diff --git a/.rubocop.yml b/.rubocop.yml index 2985c1446e4..9143966b864 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -8,6 +8,7 @@ require: - rubocop-rspec AllCops: + TargetRubyVersion: 2.5 TargetRailsVersion: 5.0 Exclude: - 'vendor/**/*' @@ -184,3 +185,8 @@ Cop/InjectEnterpriseEditionModule: Style/ReturnNil: Enabled: true + +# It isn't always safe to replace `=~` with `.match?`, especially when there are +# nil values on the left hand side +Performance/RegexpMatch: + Enabled: false diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 351a662ae83..e53e2c8fc43 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -286,13 +286,11 @@ class MergeRequestDiff < ActiveRecord::Base return yield(@external_diff_file) if @external_diff_file external_diff.open do |file| - begin - @external_diff_file = file + @external_diff_file = file - yield(@external_diff_file) - ensure - @external_diff_file = nil - end + yield(@external_diff_file) + ensure + @external_diff_file = nil end end diff --git a/app/models/merge_request_diff_file.rb b/app/models/merge_request_diff_file.rb index e8d936e265c..16ec4ed470f 100644 --- a/app/models/merge_request_diff_file.rb +++ b/app/models/merge_request_diff_file.rb @@ -23,6 +23,6 @@ class MergeRequestDiffFile < ActiveRecord::Base super end - binary? ? content.unpack('m0').first : content + binary? ? content.unpack1('m0') : content end end diff --git a/app/models/project.rb b/app/models/project.rb index aba63032cdf..59b139e5986 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1209,11 +1209,9 @@ class Project < ActiveRecord::Base def repo_exists? strong_memoize(:repo_exists) do - begin - repository.exists? - rescue - false - end + repository.exists? + rescue + false end end diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index f7064d5aaea..81302c516c2 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -205,12 +205,10 @@ class JiraService < IssueTrackerService # if any transition fails it will log the error message and stop the transition sequence def transition_issue(issue) jira_issue_transition_id.scan(Gitlab::Regex.jira_transition_id_regex).each do |transition_id| - begin - issue.transitions.build.save!(transition: { id: transition_id }) - rescue => error - log_error("Issue transition failed", error: error.message, client_url: client_url) - return false - end + issue.transitions.build.save!(transition: { id: transition_id }) + rescue => error + log_error("Issue transition failed", error: error.message, client_url: client_url) + return false end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 285fce1e6dd..ff355295862 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -265,16 +265,14 @@ class Repository # to avoid unnecessary syncing. def keep_around(*shas) shas.each do |sha| - begin - next unless sha.present? && commit_by(oid: sha) + next unless sha.present? && commit_by(oid: sha) - next if kept_around?(sha) + next if kept_around?(sha) - # This will still fail if the file is corrupted (e.g. 0 bytes) - raw_repository.write_ref(keep_around_ref_name(sha), sha) - rescue Gitlab::Git::CommandError => ex - Rails.logger.error "Unable to create keep-around reference for repository #{disk_path}: #{ex}" - end + # This will still fail if the file is corrupted (e.g. 0 bytes) + raw_repository.write_ref(keep_around_ref_name(sha), sha) + rescue Gitlab::Git::CommandError => ex + Rails.logger.error "Unable to create keep-around reference for repository #{disk_path}: #{ex}" end end diff --git a/app/models/user_interacted_project.rb b/app/models/user_interacted_project.rb index ae6778e49be..5fc59b274f5 100644 --- a/app/models/user_interacted_project.rb +++ b/app/models/user_interacted_project.rb @@ -26,16 +26,14 @@ class UserInteractedProject < ActiveRecord::Base cached_exists?(attributes) do transaction(requires_new: true) do - begin - where(attributes).select(1).first || create!(attributes) - true # not caching the whole record here for now - rescue ActiveRecord::RecordNotUnique - # Note, above queries are not atomic and prone - # to race conditions (similar like #find_or_create!). - # In the case where we hit this, the record we want - # already exists - shortcut and return. - true - end + where(attributes).select(1).first || create!(attributes) + true # not caching the whole record here for now + rescue ActiveRecord::RecordNotUnique + # Note, above queries are not atomic and prone + # to race conditions (similar like #find_or_create!). + # In the case where we hit this, the record we want + # already exists - shortcut and return. + true end end end diff --git a/app/serializers/detailed_status_entity.rb b/app/serializers/detailed_status_entity.rb index da994d78286..4f23ef0ed82 100644 --- a/app/serializers/detailed_status_entity.rb +++ b/app/serializers/detailed_status_entity.rb @@ -9,16 +9,14 @@ class DetailedStatusEntity < Grape::Entity expose :details_path expose :illustration do |status| - begin - illustration = { - image: ActionController::Base.helpers.image_path(status.illustration[:image]) - } - illustration = status.illustration.merge(illustration) + illustration = { + image: ActionController::Base.helpers.image_path(status.illustration[:image]) + } + illustration = status.illustration.merge(illustration) - illustration - rescue NotImplementedError - # ignored - end + illustration + rescue NotImplementedError + # ignored end expose :favicon do |status| diff --git a/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb b/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb index 7998976b00a..a9570176e81 100644 --- a/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb +++ b/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb @@ -42,17 +42,15 @@ module Projects def parse_response_links(objects_response) objects_response.each_with_object([]) do |entry, link_list| - begin - link = entry.dig('actions', DOWNLOAD_ACTION, 'href') + link = entry.dig('actions', DOWNLOAD_ACTION, 'href') - raise DownloadLinkNotFound unless link + raise DownloadLinkNotFound unless link - link_list << LfsDownloadObject.new(oid: entry['oid'], - size: entry['size'], - link: add_credentials(link)) - rescue DownloadLinkNotFound, Addressable::URI::InvalidURIError - log_error("Link for Lfs Object with oid #{entry['oid']} not found or invalid.") - end + link_list << LfsDownloadObject.new(oid: entry['oid'], + size: entry['size'], + link: add_credentials(link)) + rescue DownloadLinkNotFound, Addressable::URI::InvalidURIError + log_error("Link for Lfs Object with oid #{entry['oid']} not found or invalid.") end end diff --git a/app/services/projects/lfs_pointers/lfs_download_service.rb b/app/services/projects/lfs_pointers/lfs_download_service.rb index 398f00a598d..a009f479d5d 100644 --- a/app/services/projects/lfs_pointers/lfs_download_service.rb +++ b/app/services/projects/lfs_pointers/lfs_download_service.rb @@ -75,17 +75,15 @@ module Projects create_tmp_storage_dir File.open(tmp_filename, 'wb') do |file| - begin - yield file - rescue StandardError => e - # If the lfs file is successfully downloaded it will be removed - # when it is added to the project's lfs files. - # Nevertheless if any excetion raises the file would remain - # in the file system. Here we ensure to remove it - File.unlink(file) if File.exist?(file) + yield file + rescue StandardError => e + # If the lfs file is successfully downloaded it will be removed + # when it is added to the project's lfs files. + # Nevertheless if any excetion raises the file would remain + # in the file system. Here we ensure to remove it + File.unlink(file) if File.exist?(file) - raise e - end + raise e end end diff --git a/app/workers/concerns/waitable_worker.rb b/app/workers/concerns/waitable_worker.rb index 27b94a82444..17946bbc5ca 100644 --- a/app/workers/concerns/waitable_worker.rb +++ b/app/workers/concerns/waitable_worker.rb @@ -25,11 +25,9 @@ module WaitableWorker failed = [] args_list.each do |args| - begin - new.perform(*args) - rescue - failed << args - end + new.perform(*args) + rescue + failed << args end bulk_perform_async(failed) if failed.present? diff --git a/app/workers/create_gpg_signature_worker.rb b/app/workers/create_gpg_signature_worker.rb index 49c7a403838..2827529cc1c 100644 --- a/app/workers/create_gpg_signature_worker.rb +++ b/app/workers/create_gpg_signature_worker.rb @@ -20,11 +20,9 @@ class CreateGpgSignatureWorker # This calculates and caches the signature in the database commits.each do |commit| - begin - Gitlab::Gpg::Commit.new(commit).signature - rescue => e - Rails.logger.error("Failed to create signature for commit #{commit.id}. Error: #{e.message}") - end + Gitlab::Gpg::Commit.new(commit).signature + rescue => e + Rails.logger.error("Failed to create signature for commit #{commit.id}. Error: #{e.message}") end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/workers/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb index 17ad1d5ab88..ed3e354e4c2 100644 --- a/app/workers/emails_on_push_worker.rb +++ b/app/workers/emails_on_push_worker.rb @@ -52,24 +52,22 @@ class EmailsOnPushWorker end valid_recipients(recipients).each do |recipient| - begin - send_email( - recipient, - project_id, - author_id: author_id, - ref: ref, - action: action, - compare: compare, - reverse_compare: reverse_compare, - diff_refs: diff_refs, - send_from_committer_email: send_from_committer_email, - disable_diffs: disable_diffs - ) + send_email( + recipient, + project_id, + author_id: author_id, + ref: ref, + action: action, + compare: compare, + reverse_compare: reverse_compare, + diff_refs: diff_refs, + send_from_committer_email: send_from_committer_email, + disable_diffs: disable_diffs + ) - # These are input errors and won't be corrected even if Sidekiq retries - rescue Net::SMTPFatalError, Net::SMTPSyntaxError => e - logger.info("Failed to send e-mail for project '#{project.full_name}' to #{recipient}: #{e}") - end + # These are input errors and won't be corrected even if Sidekiq retries + rescue Net::SMTPFatalError, Net::SMTPSyntaxError => e + logger.info("Failed to send e-mail for project '#{project.full_name}' to #{recipient}: #{e}") end ensure @email = nil diff --git a/app/workers/object_storage/migrate_uploads_worker.rb b/app/workers/object_storage/migrate_uploads_worker.rb index fe5d27b087d..206eb71b898 100644 --- a/app/workers/object_storage/migrate_uploads_worker.rb +++ b/app/workers/object_storage/migrate_uploads_worker.rb @@ -126,11 +126,9 @@ module ObjectStorage def process_uploader(uploader) MigrationResult.new(uploader.upload).tap do |result| - begin - uploader.migrate!(@to_store) - rescue => e - result.error = e - end + uploader.migrate!(@to_store) + rescue => e + result.error = e end end end diff --git a/app/workers/pipeline_schedule_worker.rb b/app/workers/pipeline_schedule_worker.rb index ac4e9710f33..02a69ea3b54 100644 --- a/app/workers/pipeline_schedule_worker.rb +++ b/app/workers/pipeline_schedule_worker.rb @@ -8,16 +8,15 @@ class PipelineScheduleWorker def perform Ci::PipelineSchedule.active.where("next_run_at < ?", Time.now) .preload(:owner, :project).find_each do |schedule| - begin - Ci::CreatePipelineService.new(schedule.project, - schedule.owner, - ref: schedule.ref) - .execute!(:schedule, ignore_skip_ci: true, save_on_errors: true, schedule: schedule) - rescue => e - error(schedule, e) - ensure - schedule.schedule_next_run! - end + + Ci::CreatePipelineService.new(schedule.project, + schedule.owner, + ref: schedule.ref) + .execute!(:schedule, ignore_skip_ci: true, save_on_errors: true, schedule: schedule) + rescue => e + error(schedule, e) + ensure + schedule.schedule_next_run! end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/workers/remove_expired_members_worker.rb b/app/workers/remove_expired_members_worker.rb index 41913900571..3497a1f9280 100644 --- a/app/workers/remove_expired_members_worker.rb +++ b/app/workers/remove_expired_members_worker.rb @@ -6,11 +6,9 @@ class RemoveExpiredMembersWorker def perform Member.expired.find_each do |member| - begin - Members::DestroyService.new.execute(member, skip_authorization: true) - rescue => ex - logger.error("Expired Member ID=#{member.id} cannot be removed - #{ex}") - end + Members::DestroyService.new.execute(member, skip_authorization: true) + rescue => ex + logger.error("Expired Member ID=#{member.id} cannot be removed - #{ex}") end end end diff --git a/config/initializers/trusted_proxies.rb b/config/initializers/trusted_proxies.rb index 7af465d8443..13896408806 100644 --- a/config/initializers/trusted_proxies.rb +++ b/config/initializers/trusted_proxies.rb @@ -14,10 +14,8 @@ module Rack end gitlab_trusted_proxies = Array(Gitlab.config.gitlab.trusted_proxies).map do |proxy| - begin - IPAddr.new(proxy) - rescue IPAddr::InvalidAddressError - end + IPAddr.new(proxy) +rescue IPAddr::InvalidAddressError end.compact Rails.application.config.action_dispatch.trusted_proxies = ( diff --git a/db/migrate/20170530130129_project_foreign_keys_with_cascading_deletes.rb b/db/migrate/20170530130129_project_foreign_keys_with_cascading_deletes.rb index d40c61f24b1..b4658bc4017 100644 --- a/db/migrate/20170530130129_project_foreign_keys_with_cascading_deletes.rb +++ b/db/migrate/20170530130129_project_foreign_keys_with_cascading_deletes.rb @@ -126,11 +126,10 @@ class ProjectForeignKeysWithCascadingDeletes < ActiveRecord::Migration[4.2] queues.each do |queue| # Stealing is racy so it's possible a pop might be called on an # already-empty queue. - begin - remove_orphans(*queue.pop(true)) - stolen = true - rescue ThreadError - end + + remove_orphans(*queue.pop(true)) + stolen = true + rescue ThreadError end break unless stolen diff --git a/lib/api/project_milestones.rb b/lib/api/project_milestones.rb index ca24742b7a3..9ecbf37b49a 100644 --- a/lib/api/project_milestones.rb +++ b/lib/api/project_milestones.rb @@ -103,17 +103,15 @@ module API detail 'This feature was introduced in GitLab 11.9' end post ':id/milestones/:milestone_id/promote' do - begin - authorize! :admin_milestone, user_project - authorize! :admin_milestone, user_project.group + authorize! :admin_milestone, user_project + authorize! :admin_milestone, user_project.group - milestone = user_project.milestones.find(params[:milestone_id]) - Milestones::PromoteService.new(user_project, current_user).execute(milestone) + milestone = user_project.milestones.find(params[:milestone_id]) + Milestones::PromoteService.new(user_project, current_user).execute(milestone) - status(200) - rescue Milestones::PromoteService::PromoteMilestoneError => error - render_api_error!(error.message, 400) - end + status(200) + rescue Milestones::PromoteService::PromoteMilestoneError => error + render_api_error!(error.message, 400) end end end diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 32e05d84491..4106a2cdf38 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -89,11 +89,9 @@ module API optional :format, type: String, desc: 'The archive format' end get ':id/repository/archive', requirements: { format: Gitlab::PathRegex.archive_formats_regex } do - begin - send_git_archive user_project.repository, ref: params[:sha], format: params[:format], append_sha: true - rescue - not_found!('File') - end + send_git_archive user_project.repository, ref: params[:sha], format: params[:format], append_sha: true + rescue + not_found!('File') end desc 'Compare two branches, tags, or commits' do @@ -118,12 +116,10 @@ module API optional :sort, type: String, values: %w[asc desc], default: 'asc', desc: 'Sort by asc (ascending) or desc (descending)' end get ':id/repository/contributors' do - begin - contributors = ::Kaminari.paginate_array(user_project.repository.contributors(order_by: params[:order_by], sort: params[:sort])) - present paginate(contributors), with: Entities::Contributor - rescue - not_found! - end + contributors = ::Kaminari.paginate_array(user_project.repository.contributors(order_by: params[:order_by], sort: params[:sort])) + present paginate(contributors), with: Entities::Contributor + rescue + not_found! end desc 'Get the common ancestor between commits' do diff --git a/lib/gitlab/background_migration/archive_legacy_traces.rb b/lib/gitlab/background_migration/archive_legacy_traces.rb index 92096e29ef1..7ee783b8489 100644 --- a/lib/gitlab/background_migration/archive_legacy_traces.rb +++ b/lib/gitlab/background_migration/archive_legacy_traces.rb @@ -11,11 +11,10 @@ module Gitlab # So we chose a way to use ::Ci::Build directly and we don't change the `archive!` method until 11.1 ::Ci::Build.finished.without_archived_trace .where(id: start_id..stop_id).find_each do |build| - begin - build.trace.archive! - rescue => e - Rails.logger.error "Failed to archive live trace. id: #{build.id} message: #{e.message}" - end + + build.trace.archive! + rescue => e + Rails.logger.error "Failed to archive live trace. id: #{build.id} message: #{e.message}" end end end diff --git a/lib/gitlab/background_migration/normalize_ldap_extern_uids_range.rb b/lib/gitlab/background_migration/normalize_ldap_extern_uids_range.rb index 698f5e46c0c..48aa369705f 100644 --- a/lib/gitlab/background_migration/normalize_ldap_extern_uids_range.rb +++ b/lib/gitlab/background_migration/normalize_ldap_extern_uids_range.rb @@ -302,14 +302,12 @@ module Gitlab ldap_identities = Identity.where("provider like 'ldap%'").where(id: start_id..end_id) ldap_identities.each do |identity| - begin - identity.extern_uid = Gitlab::Auth::LDAP::DN.new(identity.extern_uid).to_normalized_s - unless identity.save - Rails.logger.info "Unable to normalize \"#{identity.extern_uid}\". Skipping." - end - rescue Gitlab::Auth::LDAP::DN::FormatError => e - Rails.logger.info "Unable to normalize \"#{identity.extern_uid}\" due to \"#{e.message}\". Skipping." + identity.extern_uid = Gitlab::Auth::LDAP::DN.new(identity.extern_uid).to_normalized_s + unless identity.save + Rails.logger.info "Unable to normalize \"#{identity.extern_uid}\". Skipping." end + rescue Gitlab::Auth::LDAP::DN::FormatError => e + Rails.logger.info "Unable to normalize \"#{identity.extern_uid}\" due to \"#{e.message}\". Skipping." end end diff --git a/lib/gitlab/background_migration/populate_untracked_uploads.rb b/lib/gitlab/background_migration/populate_untracked_uploads.rb index a19dc9747fb..755b5ee725a 100644 --- a/lib/gitlab/background_migration/populate_untracked_uploads.rb +++ b/lib/gitlab/background_migration/populate_untracked_uploads.rb @@ -34,18 +34,16 @@ module Gitlab def filter_error_files(files) files.partition do |file| - begin - file.to_h - true - rescue => e - msg = <<~MSG + file.to_h + true + rescue => e + msg = <<~MSG Error parsing path "#{file.path}": #{e.message} #{e.backtrace.join("\n ")} MSG - Rails.logger.error(msg) - false - end + Rails.logger.error(msg) + false end end diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index 441fdec8a1e..769d3279f91 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -79,31 +79,29 @@ module Gitlab create_labels client.issues(repo).each do |issue| - begin - description = '' - description += @formatter.author_line(issue.author) unless find_user_id(issue.author) - description += issue.description + description = '' + description += @formatter.author_line(issue.author) unless find_user_id(issue.author) + description += issue.description - label_name = issue.kind - milestone = issue.milestone ? project.milestones.find_or_create_by(title: issue.milestone) : nil + label_name = issue.kind + milestone = issue.milestone ? project.milestones.find_or_create_by(title: issue.milestone) : nil - gitlab_issue = project.issues.create!( - iid: issue.iid, - title: issue.title, - description: description, - state: issue.state, - author_id: gitlab_user_id(project, issue.author), - milestone: milestone, - created_at: issue.created_at, - updated_at: issue.updated_at - ) + gitlab_issue = project.issues.create!( + iid: issue.iid, + title: issue.title, + description: description, + state: issue.state, + author_id: gitlab_user_id(project, issue.author), + milestone: milestone, + created_at: issue.created_at, + updated_at: issue.updated_at + ) - gitlab_issue.labels << @labels[label_name] + gitlab_issue.labels << @labels[label_name] - import_issue_comments(issue, gitlab_issue) if gitlab_issue.persisted? - rescue StandardError => e - errors << { type: :issue, iid: issue.iid, errors: e.message } - end + import_issue_comments(issue, gitlab_issue) if gitlab_issue.persisted? + rescue StandardError => e + errors << { type: :issue, iid: issue.iid, errors: e.message } end end # rubocop: enable CodeReuse/ActiveRecord @@ -150,37 +148,35 @@ module Gitlab pull_requests = client.pull_requests(repo) pull_requests.each do |pull_request| - begin - description = '' - description += @formatter.author_line(pull_request.author) unless find_user_id(pull_request.author) - description += pull_request.description + description = '' + description += @formatter.author_line(pull_request.author) unless find_user_id(pull_request.author) + description += pull_request.description - source_branch_sha = pull_request.source_branch_sha - target_branch_sha = pull_request.target_branch_sha - source_branch_sha = project.repository.commit(source_branch_sha)&.sha || source_branch_sha - target_branch_sha = project.repository.commit(target_branch_sha)&.sha || target_branch_sha + source_branch_sha = pull_request.source_branch_sha + target_branch_sha = pull_request.target_branch_sha + source_branch_sha = project.repository.commit(source_branch_sha)&.sha || source_branch_sha + target_branch_sha = project.repository.commit(target_branch_sha)&.sha || target_branch_sha - merge_request = project.merge_requests.create!( - iid: pull_request.iid, - title: pull_request.title, - description: description, - source_project: project, - source_branch: pull_request.source_branch_name, - source_branch_sha: source_branch_sha, - target_project: project, - target_branch: pull_request.target_branch_name, - target_branch_sha: target_branch_sha, - state: pull_request.state, - author_id: gitlab_user_id(project, pull_request.author), - assignee_id: nil, - created_at: pull_request.created_at, - updated_at: pull_request.updated_at - ) + merge_request = project.merge_requests.create!( + iid: pull_request.iid, + title: pull_request.title, + description: description, + source_project: project, + source_branch: pull_request.source_branch_name, + source_branch_sha: source_branch_sha, + target_project: project, + target_branch: pull_request.target_branch_name, + target_branch_sha: target_branch_sha, + state: pull_request.state, + author_id: gitlab_user_id(project, pull_request.author), + assignee_id: nil, + created_at: pull_request.created_at, + updated_at: pull_request.updated_at + ) - import_pull_request_comments(pull_request, merge_request) if merge_request.persisted? - rescue StandardError => e - errors << { type: :pull_request, iid: pull_request.iid, errors: e.message, trace: e.backtrace.join("\n"), raw_response: pull_request.raw } - end + import_pull_request_comments(pull_request, merge_request) if merge_request.persisted? + rescue StandardError => e + errors << { type: :pull_request, iid: pull_request.iid, errors: e.message, trace: e.backtrace.join("\n"), raw_response: pull_request.raw } end end @@ -211,23 +207,21 @@ module Gitlab end inline_comments.each do |comment| - begin - attributes = pull_request_comment_attributes(comment) - attributes[:discussion_id] = discussion_map[comment.parent_id] if comment.has_parent? + attributes = pull_request_comment_attributes(comment) + attributes[:discussion_id] = discussion_map[comment.parent_id] if comment.has_parent? - attributes.merge!( - position: position_map[comment.iid], - type: 'DiffNote') + attributes.merge!( + position: position_map[comment.iid], + type: 'DiffNote') - note = merge_request.notes.create!(attributes) + note = merge_request.notes.create!(attributes) - # We can't store a discussion ID until a note is created, so if - # replies are created before the parent the discussion ID won't be - # linked properly. - discussion_map[comment.iid] = note.discussion_id - rescue StandardError => e - errors << { type: :pull_request, iid: comment.iid, errors: e.message } - end + # We can't store a discussion ID until a note is created, so if + # replies are created before the parent the discussion ID won't be + # linked properly. + discussion_map[comment.iid] = note.discussion_id + rescue StandardError => e + errors << { type: :pull_request, iid: comment.iid, errors: e.message } end end @@ -245,11 +239,9 @@ module Gitlab def import_standalone_pr_comments(pr_comments, merge_request) pr_comments.each do |comment| - begin - merge_request.notes.create!(pull_request_comment_attributes(comment)) - rescue StandardError => e - errors << { type: :pull_request, iid: comment.iid, errors: e.message } - end + merge_request.notes.create!(pull_request_comment_attributes(comment)) + rescue StandardError => e + errors << { type: :pull_request, iid: comment.iid, errors: e.message } end end diff --git a/lib/gitlab/bitbucket_server_import/importer.rb b/lib/gitlab/bitbucket_server_import/importer.rb index 4a789ae457f..1d3ddeeb0f1 100644 --- a/lib/gitlab/bitbucket_server_import/importer.rb +++ b/lib/gitlab/bitbucket_server_import/importer.rb @@ -162,27 +162,23 @@ module Gitlab restore_branches(batch) if recover_missing_commits batch.each do |pull_request| - begin - import_bitbucket_pull_request(pull_request) - rescue StandardError => e - backtrace = Gitlab::Profiler.clean_backtrace(e.backtrace) - log_error(stage: 'import_pull_requests', iid: pull_request.iid, error: e.message, backtrace: backtrace) + import_bitbucket_pull_request(pull_request) + rescue StandardError => e + backtrace = Gitlab::Profiler.clean_backtrace(e.backtrace) + log_error(stage: 'import_pull_requests', iid: pull_request.iid, error: e.message, backtrace: backtrace) - errors << { type: :pull_request, iid: pull_request.iid, errors: e.message, backtrace: backtrace.join("\n"), raw_response: pull_request.raw } - end + errors << { type: :pull_request, iid: pull_request.iid, errors: e.message, backtrace: backtrace.join("\n"), raw_response: pull_request.raw } end end end def delete_temp_branches @temp_branches.each do |branch| - begin - client.delete_branch(project_key, repository_slug, branch.name, branch.sha) - project.repository.delete_branch(branch.name) - rescue BitbucketServer::Connection::ConnectionError => e - log_error(stage: 'delete_temp_branches', branch: branch.name, error: e.message) - @errors << { type: :delete_temp_branches, branch_name: branch.name, errors: e.message } - end + client.delete_branch(project_key, repository_slug, branch.name, branch.sha) + project.repository.delete_branch(branch.name) + rescue BitbucketServer::Connection::ConnectionError => e + log_error(stage: 'delete_temp_branches', branch: branch.name, error: e.message) + @errors << { type: :delete_temp_branches, branch_name: branch.name, errors: e.message } end end @@ -323,16 +319,14 @@ module Gitlab def import_standalone_pr_comments(pr_comments, merge_request) pr_comments.each do |comment| - begin - merge_request.notes.create!(pull_request_comment_attributes(comment)) + merge_request.notes.create!(pull_request_comment_attributes(comment)) - comment.comments.each do |replies| - merge_request.notes.create!(pull_request_comment_attributes(replies)) - end - rescue StandardError => e - log_error(stage: 'import_standalone_pr_comments', merge_request_id: merge_request.id, comment_id: comment.id, error: e.message) - errors << { type: :pull_request, comment_id: comment.id, errors: e.message } + comment.comments.each do |replies| + merge_request.notes.create!(pull_request_comment_attributes(replies)) end + rescue StandardError => e + log_error(stage: 'import_standalone_pr_comments', merge_request_id: merge_request.id, comment_id: comment.id, error: e.message) + errors << { type: :pull_request, comment_id: comment.id, errors: e.message } end end diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index d45a044686e..7011dd1aaf2 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -98,7 +98,7 @@ module Gitlab def read_uint32(gz) binary = gz.read(4) - binary.unpack('L>')[0] if binary + binary.unpack1('L>') if binary end def read_string(gz) diff --git a/lib/gitlab/database/multi_threaded_migration.rb b/lib/gitlab/database/multi_threaded_migration.rb index 1d39a3d0b57..65a6cb8e369 100644 --- a/lib/gitlab/database/multi_threaded_migration.rb +++ b/lib/gitlab/database/multi_threaded_migration.rb @@ -35,12 +35,10 @@ module Gitlab threads = Array.new(thread_count) do Thread.new do pool.with_connection do |connection| - begin - Thread.current[MULTI_THREAD_AR_CONNECTION] = connection - yield - ensure - Thread.current[MULTI_THREAD_AR_CONNECTION] = nil - end + Thread.current[MULTI_THREAD_AR_CONNECTION] = connection + yield + ensure + Thread.current[MULTI_THREAD_AR_CONNECTION] = nil end end end diff --git a/lib/gitlab/database/sha_attribute.rb b/lib/gitlab/database/sha_attribute.rb index 8d97adaff99..109ae7893da 100644 --- a/lib/gitlab/database/sha_attribute.rb +++ b/lib/gitlab/database/sha_attribute.rb @@ -22,7 +22,7 @@ module Gitlab # Casts binary data to a SHA1 in hexadecimal. def deserialize(value) value = super(value) - value ? value.unpack(PACK_FORMAT)[0] : nil + value ? value.unpack1(PACK_FORMAT) : nil end # Casts a SHA1 in hexadecimal to the proper binary format. diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 0a371889af2..e3b9a7a1a89 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -75,13 +75,11 @@ module Gitlab @certs = stub_cert_paths.flat_map do |cert_file| File.read(cert_file).scan(PEM_REGEX).map do |cert| - begin - OpenSSL::X509::Certificate.new(cert).to_pem - rescue OpenSSL::OpenSSLError => e - Rails.logger.error "Could not load certificate #{cert_file} #{e}" - Gitlab::Sentry.track_exception(e, extra: { cert_file: cert_file }) - nil - end + OpenSSL::X509::Certificate.new(cert).to_pem + rescue OpenSSL::OpenSSLError => e + Rails.logger.error "Could not load certificate #{cert_file} #{e}" + Gitlab::Sentry.track_exception(e, extra: { cert_file: cert_file }) + nil end.compact end.uniq.join("\n") end diff --git a/lib/gitlab/gitaly_client/blobs_stitcher.rb b/lib/gitlab/gitaly_client/blobs_stitcher.rb index 01bab854082..f860d8ce517 100644 --- a/lib/gitlab/gitaly_client/blobs_stitcher.rb +++ b/lib/gitlab/gitaly_client/blobs_stitcher.rb @@ -13,17 +13,15 @@ module Gitlab current_blob_data = nil @rpc_response.each do |msg| - begin - if msg.oid.blank? && msg.data.blank? - next - elsif msg.oid.present? - yield new_blob(current_blob_data) if current_blob_data + if msg.oid.blank? && msg.data.blank? + next + elsif msg.oid.present? + yield new_blob(current_blob_data) if current_blob_data - current_blob_data = msg.to_h.slice(:oid, :path, :size, :revision, :mode) - current_blob_data[:data] = msg.data.dup - else - current_blob_data[:data] << msg.data - end + current_blob_data = msg.to_h.slice(:oid, :path, :size, :revision, :mode) + current_blob_data[:data] = msg.data.dup + else + current_blob_data[:data] << msg.data end end diff --git a/lib/gitlab/legacy_github_import/importer.rb b/lib/gitlab/legacy_github_import/importer.rb index f3323c98af2..70b18221a66 100644 --- a/lib/gitlab/legacy_github_import/importer.rb +++ b/lib/gitlab/legacy_github_import/importer.rb @@ -89,12 +89,10 @@ module Gitlab def import_labels fetch_resources(:labels, repo, per_page: 100) do |labels| labels.each do |raw| - begin - gh_label = LabelFormatter.new(project, raw) - gh_label.create! - rescue => e - errors << { type: :label, url: Gitlab::UrlSanitizer.sanitize(gh_label.url), errors: e.message } - end + gh_label = LabelFormatter.new(project, raw) + gh_label.create! + rescue => e + errors << { type: :label, url: Gitlab::UrlSanitizer.sanitize(gh_label.url), errors: e.message } end end @@ -104,12 +102,10 @@ module Gitlab def import_milestones fetch_resources(:milestones, repo, state: :all, per_page: 100) do |milestones| milestones.each do |raw| - begin - gh_milestone = MilestoneFormatter.new(project, raw) - gh_milestone.create! - rescue => e - errors << { type: :milestone, url: Gitlab::UrlSanitizer.sanitize(gh_milestone.url), errors: e.message } - end + gh_milestone = MilestoneFormatter.new(project, raw) + gh_milestone.create! + rescue => e + errors << { type: :milestone, url: Gitlab::UrlSanitizer.sanitize(gh_milestone.url), errors: e.message } end end end @@ -223,24 +219,22 @@ module Gitlab def create_comments(comments) ActiveRecord::Base.no_touching do comments.each do |raw| - begin - comment = CommentFormatter.new(project, raw, client) + comment = CommentFormatter.new(project, raw, client) - # GH does not return info about comment's parent, so we guess it by checking its URL! - *_, parent, iid = URI(raw.html_url).path.split('/') + # GH does not return info about comment's parent, so we guess it by checking its URL! + *_, parent, iid = URI(raw.html_url).path.split('/') - issuable = if parent == 'issues' - Issue.find_by(project_id: project.id, iid: iid) - else - MergeRequest.find_by(target_project_id: project.id, iid: iid) - end + issuable = if parent == 'issues' + Issue.find_by(project_id: project.id, iid: iid) + else + MergeRequest.find_by(target_project_id: project.id, iid: iid) + end - next unless issuable + next unless issuable - issuable.notes.create!(comment.attributes) - rescue => e - errors << { type: :comment, url: Gitlab::UrlSanitizer.sanitize(raw.url), errors: e.message } - end + issuable.notes.create!(comment.attributes) + rescue => e + errors << { type: :comment, url: Gitlab::UrlSanitizer.sanitize(raw.url), errors: e.message } end end end @@ -281,12 +275,10 @@ module Gitlab def import_releases fetch_resources(:releases, repo, per_page: 100) do |releases| releases.each do |raw| - begin - gh_release = ReleaseFormatter.new(project, raw) - gh_release.create! if gh_release.valid? - rescue => e - errors << { type: :release, url: Gitlab::UrlSanitizer.sanitize(gh_release.url), errors: e.message } - end + gh_release = ReleaseFormatter.new(project, raw) + gh_release.create! if gh_release.valid? + rescue => e + errors << { type: :release, url: Gitlab::UrlSanitizer.sanitize(gh_release.url), errors: e.message } end end end diff --git a/lib/gitlab/metrics/influx_db.rb b/lib/gitlab/metrics/influx_db.rb index 0b04340fbb5..269d90fa971 100644 --- a/lib/gitlab/metrics/influx_db.rb +++ b/lib/gitlab/metrics/influx_db.rb @@ -52,10 +52,8 @@ module Gitlab pool&.with do |connection| prepared.each_slice(settings[:packet_size]) do |slice| - begin - connection.write_points(slice) - rescue StandardError - end + connection.write_points(slice) + rescue StandardError end end rescue Errno::EADDRNOTAVAIL, SocketError => ex diff --git a/lib/tasks/gitlab/artifacts/migrate.rake b/lib/tasks/gitlab/artifacts/migrate.rake index e7634d2ed4f..9012e55a70c 100644 --- a/lib/tasks/gitlab/artifacts/migrate.rake +++ b/lib/tasks/gitlab/artifacts/migrate.rake @@ -11,14 +11,13 @@ namespace :gitlab do Ci::Build.joins(:project) .with_artifacts_stored_locally .find_each(batch_size: 10) do |build| - begin - build.artifacts_file.migrate!(ObjectStorage::Store::REMOTE) - build.artifacts_metadata.migrate!(ObjectStorage::Store::REMOTE) - logger.info("Transferred artifact ID #{build.id} with size #{build.artifacts_size} to object storage") - rescue => e - logger.error("Failed to transfer artifacts of #{build.id} with error: #{e.message}") - end + build.artifacts_file.migrate!(ObjectStorage::Store::REMOTE) + build.artifacts_metadata.migrate!(ObjectStorage::Store::REMOTE) + + logger.info("Transferred artifact ID #{build.id} with size #{build.artifacts_size} to object storage") + rescue => e + logger.error("Failed to transfer artifacts of #{build.id} with error: #{e.message}") end end end diff --git a/lib/tasks/gitlab/lfs/migrate.rake b/lib/tasks/gitlab/lfs/migrate.rake index a45e5ca91e0..97c15175a23 100644 --- a/lib/tasks/gitlab/lfs/migrate.rake +++ b/lib/tasks/gitlab/lfs/migrate.rake @@ -9,13 +9,12 @@ namespace :gitlab do LfsObject.with_files_stored_locally .find_each(batch_size: 10) do |lfs_object| - begin - lfs_object.file.migrate!(LfsObjectUploader::Store::REMOTE) - logger.info("Transferred LFS object #{lfs_object.oid} of size #{lfs_object.size.to_i.bytes} to object storage") - rescue => e - logger.error("Failed to transfer LFS object #{lfs_object.oid} with error: #{e.message}") - end + lfs_object.file.migrate!(LfsObjectUploader::Store::REMOTE) + + logger.info("Transferred LFS object #{lfs_object.oid} of size #{lfs_object.size.to_i.bytes} to object storage") + rescue => e + logger.error("Failed to transfer LFS object #{lfs_object.oid} with error: #{e.message}") end end end diff --git a/lib/tasks/gitlab/traces.rake b/lib/tasks/gitlab/traces.rake index 5a232091a7e..5e1ec481ece 100644 --- a/lib/tasks/gitlab/traces.rake +++ b/lib/tasks/gitlab/traces.rake @@ -26,13 +26,12 @@ namespace :gitlab do Ci::Build.joins(:project) .with_archived_trace_stored_locally .find_each(batch_size: 10) do |build| - begin - build.job_artifacts_trace.file.migrate!(ObjectStorage::Store::REMOTE) - logger.info("Transferred job trace of #{build.id} to object storage") - rescue => e - logger.error("Failed to transfer artifacts of #{build.id} with error: #{e.message}") - end + build.job_artifacts_trace.file.migrate!(ObjectStorage::Store::REMOTE) + + logger.info("Transferred job trace of #{build.id} to object storage") + rescue => e + logger.error("Failed to transfer artifacts of #{build.id} with error: #{e.message}") end end end diff --git a/lib/tasks/lint.rake b/lib/tasks/lint.rake index 5d673a1a285..c5d0f2c292f 100644 --- a/lib/tasks/lint.rake +++ b/lib/tasks/lint.rake @@ -19,11 +19,9 @@ unless Rails.env.production? desc "GitLab | lint | Lint HAML files" task :haml do - begin - Rake::Task['haml_lint'].invoke - rescue RuntimeError # The haml_lint tasks raise a RuntimeError - exit(1) - end + Rake::Task['haml_lint'].invoke + rescue RuntimeError # The haml_lint tasks raise a RuntimeError + exit(1) end desc "GitLab | lint | Run several lint checks" diff --git a/lib/tasks/migrate/migrate_iids.rake b/lib/tasks/migrate/migrate_iids.rake index aa2d01730d7..cb7c496c31c 100644 --- a/lib/tasks/migrate/migrate_iids.rake +++ b/lib/tasks/migrate/migrate_iids.rake @@ -2,49 +2,43 @@ desc "GitLab | Build internal ids for issues and merge requests" task migrate_iids: :environment do puts 'Issues'.color(:yellow) Issue.where(iid: nil).find_each(batch_size: 100) do |issue| - begin - issue.set_iid + issue.set_iid - if issue.update_attribute(:iid, issue.iid) - print '.' - else - print 'F' - end - rescue + if issue.update_attribute(:iid, issue.iid) + print '.' + else print 'F' end + rescue + print 'F' end puts 'done' puts 'Merge Requests'.color(:yellow) MergeRequest.where(iid: nil).find_each(batch_size: 100) do |mr| - begin - mr.set_iid + mr.set_iid - if mr.update_attribute(:iid, mr.iid) - print '.' - else - print 'F' - end - rescue + if mr.update_attribute(:iid, mr.iid) + print '.' + else print 'F' end + rescue + print 'F' end puts 'done' puts 'Milestones'.color(:yellow) Milestone.where(iid: nil).find_each(batch_size: 100) do |m| - begin - m.set_iid + m.set_iid - if m.update_attribute(:iid, m.iid) - print '.' - else - print 'F' - end - rescue + if m.update_attribute(:iid, m.iid) + print '.' + else print 'F' end + rescue + print 'F' end puts 'done' diff --git a/qa/qa/page/project/web_ide/edit.rb b/qa/qa/page/project/web_ide/edit.rb index 2b6c01888d5..ff7cc04e352 100644 --- a/qa/qa/page/project/web_ide/edit.rb +++ b/qa/qa/page/project/web_ide/edit.rb @@ -43,11 +43,9 @@ module QA def create_new_file_from_template(file_name, template) click_element :new_file within_element(:template_list) do - begin - click_on file_name - rescue Capybara::ElementNotFound - raise ElementNotFound, %Q(Couldn't find file template named "#{file_name}". Please confirm that it is a valid option.) - end + click_on file_name + rescue Capybara::ElementNotFound + raise ElementNotFound, %Q(Couldn't find file template named "#{file_name}". Please confirm that it is a valid option.) end wait(reload: false) do diff --git a/scripts/gitaly_test.rb b/scripts/gitaly_test.rb index dee4c2eba7e..b5d3facd18a 100644 --- a/scripts/gitaly_test.rb +++ b/scripts/gitaly_test.rb @@ -79,15 +79,13 @@ module GitalyTest socket = read_socket_path Integer(timeout / delay).times do - begin - UNIXSocket.new(socket) - puts ' OK' + UNIXSocket.new(socket) + puts ' OK' - return - rescue Errno::ENOENT, Errno::ECONNREFUSED - print '.' - sleep delay - end + return + rescue Errno::ENOENT, Errno::ECONNREFUSED + print '.' + sleep delay end puts ' FAILED' diff --git a/scripts/merge-simplecov b/scripts/merge-simplecov index 65f93f8830b..746be3317a7 100755 --- a/scripts/merge-simplecov +++ b/scripts/merge-simplecov @@ -12,11 +12,9 @@ module SimpleCov def resultset_hashes resultset_files.map do |path| - begin - JSON.parse(File.read(path)) - rescue - {} - end + JSON.parse(File.read(path)) + rescue + {} end end diff --git a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb index bc71a90605a..e7ff9169f1b 100644 --- a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb +++ b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb @@ -172,10 +172,8 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :m let(:exception) { ActiveRecord::RecordNotFound } let(:perform_ignoring_exceptions) do - begin - subject.perform(start_id, stop_id) - rescue described_class::Error - end + subject.perform(start_id, stop_id) + rescue described_class::Error end before do diff --git a/spec/lib/gitlab/git/repository_cleaner_spec.rb b/spec/lib/gitlab/git/repository_cleaner_spec.rb index 7f9cc2bc9ec..6602f22843f 100644 --- a/spec/lib/gitlab/git/repository_cleaner_spec.rb +++ b/spec/lib/gitlab/git/repository_cleaner_spec.rb @@ -37,14 +37,12 @@ describe Gitlab::Git::RepositoryCleaner do let(:object_map) { Gitlab::HttpIO.new(url, object_map_data.size) } around do |example| - begin - tempfile.write(object_map_data) - tempfile.close + tempfile.write(object_map_data) + tempfile.close - example.run - ensure - tempfile.unlink - end + example.run + ensure + tempfile.unlink end it 'removes internal references' do diff --git a/spec/rack_servers/puma_spec.rb b/spec/rack_servers/puma_spec.rb index 431fab87857..891df4f1a66 100644 --- a/spec/rack_servers/puma_spec.rb +++ b/spec/rack_servers/puma_spec.rb @@ -44,11 +44,9 @@ describe 'Puma' do end after(:all) do - begin - WebMock.disable_net_connect!(allow_localhost: true) - Process.kill('TERM', @puma_master_pid) - rescue Errno::ESRCH - end + WebMock.disable_net_connect!(allow_localhost: true) + Process.kill('TERM', @puma_master_pid) + rescue Errno::ESRCH end def wait_puma_boot!(master_pid, ready_file) diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index dfbdfa2ab69..d3a8ee46f85 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -128,10 +128,8 @@ describe Projects::DestroyService do it 'keeps project team intact upon an error' do perform_enqueued_jobs do - begin - destroy_project(project, user, {}) - rescue ::Redis::CannotConnectError - end + destroy_project(project, user, {}) + rescue ::Redis::CannotConnectError end expect(project.team.members.count).to eq 2 diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index 6cdc19ac2e5..ca28325eab9 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -18,12 +18,10 @@ module GraphqlHelpers # Runs a block inside a BatchLoader::Executor wrapper def batch(max_queries: nil, &blk) wrapper = proc do - begin - BatchLoader::Executor.ensure_current - yield - ensure - BatchLoader::Executor.clear_current - end + BatchLoader::Executor.ensure_current + yield + ensure + BatchLoader::Executor.clear_current end if max_queries diff --git a/spec/support/helpers/stub_object_storage.rb b/spec/support/helpers/stub_object_storage.rb index e0c50e533a6..30c8477f16a 100644 --- a/spec/support/helpers/stub_object_storage.rb +++ b/spec/support/helpers/stub_object_storage.rb @@ -23,15 +23,13 @@ module StubObjectStorage Fog.mock! ::Fog::Storage.new(connection_params).tap do |connection| - begin - connection.directories.create(key: remote_directory) + connection.directories.create(key: remote_directory) - # Cleanup remaining files - connection.directories.each do |directory| - directory.files.map(&:destroy) - end - rescue Excon::Error::Conflict + # Cleanup remaining files + connection.directories.each do |directory| + directory.files.map(&:destroy) end + rescue Excon::Error::Conflict end end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index 80a22134021..dc902d373b8 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -202,12 +202,10 @@ module TestEnv socket = Gitlab::GitalyClient.address('default').sub('unix:', '') Integer(sleep_time / sleep_interval).times do - begin - Socket.unix(socket) - return - rescue - sleep sleep_interval - end + Socket.unix(socket) + return + rescue + sleep sleep_interval end raise "could not connect to gitaly at #{socket.inspect} after #{sleep_time} seconds"