Class: RuboCop::Cop::Heatwave::NoBareJobPathRedirect

Inherits:
Base
  • Object
show all
Defined in:
lib/rubocop/cop/heatwave/no_bare_job_path_redirect.rb

Overview

Flags redirect_to job_path(jid) where the JID is not proven non-nil.

Every Sidekiq job inherits lock: :until_and_while_executing with
client on_conflict: :log (see Sidekiq.default_job_options), so
perform_async returns nil whenever the uniqueness lock rejects a
duplicate push — a double-clicked button, a double-submitted form. The
route has no nil-able segment, so job_path(nil) raises
ActionController::UrlGenerationError and the user gets a 500.

This has now bitten production three times (AppSignal #4231, #6312,
#6497). Each time the fix was a per-call-site guard, and each time new
call sites reintroduced it — hence this cop.

CrmController#redirect_to_job_or_fallback handles all three cases:
the job queued, a duplicate was already running (via duplicate_lookup:),
or the push genuinely failed (falls back with an explanatory flash).

A call is accepted when the JID is guarded — the cop walks up the AST
looking for an enclosing if/unless/&& condition that tests the same
variable, which covers the if job_id and if job_id.present? shapes
already used across the controllers, plus the trailing-modifier form.

Examples:

# bad
job_id = SomeWorker.perform_async(opts)
redirect_to job_path(job_id)

# good — the helper
job_id = SomeWorker.perform_async(opts)
redirect_to_job_or_fallback(job_id, thing_path(@thing))

# good — explicitly guarded
if job_id.present?
  redirect_to job_path(job_id)
else
  flash[:error] = 'Could not queue the job.'
  redirect_to thing_path(@thing)
end

Constant Summary collapse

MSG =

Names the nil-JID failure mode and points at the two sanctioned fixes:
the shared helper, or an explicit guard.

'`perform_async` returns nil when the uniqueness lock rejects a duplicate, and ' \
'`job_path(nil)` raises UrlGenerationError (AppSignal #4231/#6312/#6497). Use ' \
'`redirect_to_job_or_fallback(jid, fallback_path)` or guard the jid explicitly.'
RESTRICT_ON_SEND =

Only redirect_to sends can match, so skip the rest outright.

%i[redirect_to].freeze
VARIABLE_TYPES =

Node types that prove a variable is non-nil: a plain read, or the
assign-and-test idiom if (jid = Something.find).

%i[lvar ivar lvasgn ivasgn].freeze
EXIT_TYPES =

Node types that leave the action, making a preceding conditional a real
guard rather than just an unrelated branch.

%i[return next break].freeze

Instance Method Summary collapse

Instance Method Details

#on_send(node) ⇒ Object 



67
68
69
70
71
72
73
74
75
76
 
# File 'lib/rubocop/cop/heatwave/no_bare_job_path_redirect.rb', line 67

def on_send(node)
  jid = redirect_to_job_path(node)
  return unless jid
  # A literal (or anything that isn't a plain local/ivar read) can't be
  # the nil-from-perform_async case.
  return unless (name = jid_name(jid))
  return if guarded?(node, name)

  add_offense(node)
end

#redirect_to_job_path(node) ⇒ Object 



63
64
65
 
# File 'lib/rubocop/cop/heatwave/no_bare_job_path_redirect.rb', line 63

def_node_matcher :redirect_to_job_path, <<~PATTERN
  (send nil? :redirect_to (send nil? :job_path $_ ...) ...)
PATTERN