Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Violations of and Reenable Style/RescueModifier #57847

Merged
merged 4 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 0 additions & 5 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -365,11 +365,6 @@ Style/RedundantSelfAssignment:
Style/RegexpLiteral:
Enabled: false

# Offense count: 29
# This cop supports safe auto-correction (--auto-correct).
Style/RescueModifier:
Enabled: false

# Offense count: 172
# This cop supports safe auto-correction (--auto-correct).
# Configuration parameters: EnforcedStyle.
Expand Down
6 changes: 5 additions & 1 deletion aws/cloudformation/update_certs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ begin
# raise 'new cert'

CDO.log.info "Checking for existing certificate for #{common_name}..."
Acmesmith::Command.start ['current', common_name] rescue raise 'cert not found'
begin
Acmesmith::Command.start ['current', common_name]
rescue
raise 'cert not found'
end

CDO.log.info "Existing certificate found, checking expiration..."
expiration_date = get_certificate_expiration(common_name)
Expand Down
6 changes: 5 additions & 1 deletion bin/cron/deliver_poste_messages
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ def main
deliverer = Deliverer.new SMTP_OPTIONS

until queue.empty?
next unless delivery = queue.pop(true) rescue nil
next unless delivery = begin
queue.pop(true)
rescue
nil
end

sent_at = DateTime.now

Expand Down
6 changes: 5 additions & 1 deletion bin/cron/update_hoc_map
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ def main

DB_READONLY[:forms].where(kind: CURRENT_HOC_SIGNUP).paged_each(rows_per_fetch: 10_000) do |form|
data = JSON.parse(form[:data])
processed_data = JSON.parse(form[:processed_data]) rescue {}
processed_data = begin
JSON.parse(form[:processed_data])
rescue
{}
end
# ***
# If these rules are changed and we wish to display a broader set of events,
# update our JS code (hoc_events_map.js) that visualizes these events
Expand Down
6 changes: 5 additions & 1 deletion bin/i18n/utils/malformed_i18n_reporter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ def worksheet_data
def process_file(i18n_file_path)
return unless File.exist?(i18n_file_path)

translations = I18nScriptUtils.parse_file(i18n_file_path) rescue nil
translations = begin
I18nScriptUtils.parse_file(i18n_file_path)
rescue
nil
end
return unless translations

collect_malformed_translations(locale, i18n_file_path, translations)
Expand Down
6 changes: 5 additions & 1 deletion cookbooks/cdo-apps/ohai/sudo_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@
provides 'user', 'home', 'current_user'

collect_data(:default) do
user = Etc.getpwuid(Process.euid).name rescue nil
user = begin
Etc.getpwuid(Process.euid).name
rescue
nil
end
user = ENV['SUDO_USER'] if [nil, 'root'].include?(user) && ENV['SUDO_USER']
user ||= ENV['USER']

Expand Down
4 changes: 3 additions & 1 deletion dashboard/app/helpers/locale_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ def data_t_suffix(dotted_path, key, suffix, options = {})

# Tries to access translation, returning nil if not found
def try_t(dotted_path, params = {})
I18n.t(dotted_path, **({raise: true}.merge(params))) rescue nil
I18n.t(dotted_path, **({raise: true}.merge(params)))
rescue
nil
end

def i18n_dropdown
Expand Down
6 changes: 5 additions & 1 deletion dashboard/app/models/datablock_storage_table.rb
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,11 @@ def coerce_column(column_name, column_type)
value.to_s
end
when 'number'
Float(value) rescue raise StudentFacingError.new(:CANNOT_CONVERT_COLUMN_TYPE), "Couldn't convert #{value.inspect} to number"
begin
Float(value)
rescue
raise StudentFacingError.new(:CANNOT_CONVERT_COLUMN_TYPE), "Couldn't convert #{value.inspect} to number"
end
when 'boolean'
if [true, 'true'].include? value
true
Expand Down
6 changes: 5 additions & 1 deletion dashboard/app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1569,7 +1569,11 @@ def confirmation_required?

def age=(val)
@age = val
val = val.to_i rescue 0 # sometimes we get age: {"Pr" => nil}
val = begin
val.to_i
rescue
0 # sometimes we get age: {"Pr" => nil}
end
return unless val > 0
return unless val < 200
return if birthday && val == age # don't change birthday if we want to stay the same age
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ def up
rescue
# If an exception occurs, back out of this migration, but ignore any
# exceptions generated there. Do the best you can.
down rescue nil
begin
down
rescue
nil
end

# Re-raise this exception for diagnostic purposes.
raise
Expand Down
6 changes: 5 additions & 1 deletion dashboard/legacy/middleware/files_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,11 @@ def record_event(quota_event_type, quota_type, encrypted_channel_id)
#
def get_file(endpoint, encrypted_channel_id, filename, code_projects_domain_root_route = false, cache_duration: nil)
# We occasionally serve HTML files through theses APIs - we don't want NewRelic JS inserted...
NewRelic::Agent.ignore_enduser rescue nil
begin
NewRelic::Agent.ignore_enduser
rescue
nil
end

buckets = get_bucket_impl(endpoint).new
cache_duration ||= buckets.cache_duration_seconds
Expand Down
4 changes: 3 additions & 1 deletion dashboard/legacy/middleware/helpers/firebase_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ def table_as_csv(table_name)
end

def number?(num)
!Float(num).nil? rescue false
!Float(num).nil?
rescue
false
end

def csv_as_table(csv_data)
Expand Down
4 changes: 3 additions & 1 deletion dashboard/legacy/middleware/net_sim_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,9 @@ def parse_table_map_from_query_string(query_string)
def parse_ids_from_query_string(query_string)
[].tap do |ids|
CGI.parse(query_string)['id[]'].each do |id|
ids << Integer(id, 10) rescue ArgumentError
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to say, this looks like a potential misuse of the "modifier" syntax -- it looks like the caller wanted to rescue this type of error, rather than insert this error class into the list of returned ids. in fact it looks like this error was introduced specifically when switching to the modifier syntax: d04d412

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, great catch! Definitely agree with that analysis; I'm tempted to manually update this fix to reflect that presumed intent rather than the current functionality.

ids << Integer(id, 10)
rescue
ArgumentError
end
end
end
8 changes: 6 additions & 2 deletions dashboard/lib/json_value.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
# Utility for formatting JSON values from string input
class JSONValue
def self.numeric?(val)
!Float(val).nil? rescue false
!Float(val).nil?
rescue
false
end

def self.integral?(val)
!Integer(val).nil? rescue false
!Integer(val).nil?
rescue
false
end

def self.boolean?(val)
Expand Down
6 changes: 5 additions & 1 deletion dashboard/scripts/archive/count_lines
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ require 'json'

DIR = '/mnt/backups/'.freeze
FILE = (DIR + 'milestone_cache').freeze
sum_cache = (File.open(FILE, 'r') {|f| Marshal.load(f.read)}) rescue {}
sum_cache = begin
(File.open(FILE, 'r') {|f| Marshal.load(f.read)})
rescue
{}
end
today_sum = archive_sum = 0
results = {}

Expand Down
12 changes: 10 additions & 2 deletions dashboard/scripts/import_course_facilitators.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,17 @@ def [](header)
next if user_id&.start_with?('-')
facilitator =
if user_id.nil?
User.find_by!(email: email) rescue raise "Unable to find user email #{email}"
begin
User.find_by!(email: email)
rescue
raise "Unable to find user email #{email}"
end
else
User.find(user_id) rescue raise "Unable to find user id #{user_id}"
begin
User.find(user_id)
rescue
raise "Unable to find user id #{user_id}"
end
end

courses = row[COL_COURSES].split(',').map(&:strip)
Expand Down
6 changes: 5 additions & 1 deletion dashboard/test/ui/utils/selenium_browser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ def create_response(code, body, content_type)
rescue Selenium::WebDriver::Error::WebDriverError => exception
if (msg = exception.message.match(/unexpected response, code=(?<code>\d+).*\n(?<error>.*)/))
error = msg[:error]
error = JSON.parse(error)['value']['error'] rescue error
error = begin
JSON.parse(error)['value']['error']
rescue
error
end
raise exception, "Error #{msg[:code]}: #{error}", exception.backtrace
end
raise
Expand Down
18 changes: 15 additions & 3 deletions lib/cdo/aws/cloud_formation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,11 @@ def delete

private def wait_for_stack(action)
log.info "Stack #{action} requested, waiting for provisioning to complete..."
yield rescue nil
begin
yield
rescue
nil
end
begin
cfn.wait_until("stack_#{action}_complete".to_sym, stack_name: @stack_id) do |w|
w.delay = 5 # seconds
Expand All @@ -311,13 +315,21 @@ def delete
end
end
rescue Aws::Waiters::Errors::FailureStateError
yield rescue nil
begin
yield
rescue
nil
end
if action == :create
log.info 'Stack will remain in its half-created state for debugging. To delete, run `rake adhoc:stop`.'
end
raise "\nError on #{action}."
end
yield rescue nil
begin
yield
rescue
nil
end
unless options[:quiet]
log.info "\nStack #{action} complete."
unless action == 'delete'
Expand Down
12 changes: 10 additions & 2 deletions lib/cdo/cloud_formation/lambda.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ def zip_directory(relative_directory, key_prefix: 'lambda')
code_zip = `zip -qr - .`
key = "#{key_prefix}-#{hash}.zip"
s3_client = Aws::S3::Client.new(http_read_timeout: 30)
object_exists = s3_client.head_object(bucket: S3_LAMBDA_BUCKET, key: key) rescue nil
object_exists = begin
s3_client.head_object(bucket: S3_LAMBDA_BUCKET, key: key)
rescue
nil
end
unless object_exists
CDO.log.info("Uploading Lambda zip package to S3 (#{code_zip.length} bytes)...")
s3_client.put_object({bucket: S3_LAMBDA_BUCKET, key: key, body: code_zip})
Expand Down Expand Up @@ -106,7 +110,11 @@ def lambda_zip(*files, key_prefix: 'lambda')
code_zip = `zip -qr - #{files.join(' ')}`
key = "#{key_prefix}-#{hash}.zip"
s3_client = Aws::S3::Client.new(http_read_timeout: 30)
object_exists = s3_client.head_object(bucket: S3_LAMBDA_BUCKET, key: key) rescue nil
object_exists = begin
s3_client.head_object(bucket: S3_LAMBDA_BUCKET, key: key)
rescue
nil
end
unless object_exists
CDO.log.info("Uploading Lambda zip package to S3 (#{code_zip.length} bytes)...")
s3_client.put_object({bucket: S3_LAMBDA_BUCKET, key: key, body: code_zip})
Expand Down
6 changes: 5 additions & 1 deletion lib/cdo/cloud_formation/tail_logs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ def tail_events(start)
log_config[:start_time] = start
end
# Return silently if we can't get the log events for any reason.
resp = cw_logs.get_log_events(log_config) rescue return
resp = begin
cw_logs.get_log_events(log_config)
rescue
return
end
resp.events.map(&:message).each {|message| log.info(message)}
@log_token = resp.next_forward_token
end
Expand Down
18 changes: 15 additions & 3 deletions lib/cdo/geocoder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,11 @@ def self.find_potential_street_address(text)

first_number_to_end = number_to_end_search.first.first

return nil if Float(first_number_to_end) rescue false # is a number
begin
return nil if Float(first_number_to_end)
rescue
false # is a number
end
return nil if first_number_to_end.length < MIN_ADDRESS_LENGTH # too short to be an address
return nil if first_number_to_end.count(' ') < 2 # too few words to be an address

Expand Down Expand Up @@ -129,7 +133,11 @@ module SauceLabsOverride
]

def search(query, options = {})
ip = IPAddr.new(query) rescue nil
ip = begin
IPAddr.new(query)
rescue
nil
end
if SAUCELABS_CIDR.any? {|cidr| cidr.include?(ip)}
[OpenStruct.new(country_code: 'US', country: 'United States')]
else
Expand All @@ -145,7 +153,11 @@ def search(query, options = {})
# https://github.com/alexreisner/geocoder/blob/350cf0cc6a158d510aec3d91594d9b5718f877a9/lib/geocoder/lookups/freegeoip.rb#L41-L54
module LocahostOverride
def search(query, options = {})
ip = IPAddr.new(query) rescue nil
ip = begin
IPAddr.new(query)
rescue
nil
end
if ip&.loopback?
[OpenStruct.new(
ip: ip.to_s,
Expand Down
6 changes: 5 additions & 1 deletion lib/cdo/i18n_string_url_tracker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,11 @@ def self.string_exists?(string_key, scope = nil, locale = I18n.default_locale)
# raise error if translation is missing
raise: true
}
source_string = I18n.t(string_key, **options) rescue nil
source_string = begin
I18n.t(string_key, **options)
rescue
nil
end
!source_string.nil?
end
end
Expand Down
6 changes: 5 additions & 1 deletion lib/cdo/optimizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ def self.optimize(data, content_type)
# Optimizes image content.
def self.optimize_image(data)
# Skip image optimization if image is too big.
pixels = ImageSize.new(data).size.inject(&:*) rescue 0
pixels = begin
ImageSize.new(data).size.inject(&:*)
rescue
0
end
if pixels > DCDO.get('image_optim_pixel_max', IMAGE_OPTIM_PIXEL_MAX)
return data
end
Expand Down
6 changes: 5 additions & 1 deletion lib/cdo/rack/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ module RequestExtension
end

def trusted_proxy?(ip)
super(ip) || TRUSTED_PROXIES.any? {|proxy| proxy.include?(ip) rescue false}
super(ip) || TRUSTED_PROXIES.any? do |proxy|
proxy.include?(ip)
rescue
false
end
end

def json_body
Expand Down
6 changes: 5 additions & 1 deletion lib/cdo/rake_utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,11 @@ def self.threaded_each(array, thread_count = 2)

threads = create_threads(thread_count) do
until queue.empty?
next unless item = queue.pop(true) rescue nil
next unless item = begin
queue.pop(true)
rescue
nil
end
yield item if block_given?
end
end
Expand Down