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: GoodJob::AdvisoryLockable::RecordAlreadyAdvisoryLockedError (MAYBE-RAILS-DK) #1797

Closed
wants to merge 1 commit into from

Conversation

revise-dev[bot]
Copy link

@revise-dev revise-dev bot commented Feb 4, 2025

The error GoodJob::AdvisoryLockable::RecordAlreadyAdvisoryLockedError occurs when attempting to access or manipulate a GoodJob job that is already locked by another process. This is a race condition that needs to be handled gracefully to prevent application crashes and provide a better user experience.

The fix involves modifying the GoodJob::JobsController to:

  1. Add a rescue_from handler at the controller level to catch the specific advisory lock error
  2. Implement a handler method that provides a user-friendly message and redirects to a safe state
  3. Ensure the error handling integrates well with the existing authentication and authorization setup

The changes maintain the controller's original intent of managing background jobs while adding proper error handling for concurrent access scenarios. The redirect to jobs_path ensures users aren't left in a broken state when attempting to access locked jobs.

We're also adding comprehensive tests to verify:

  • The error handling works as expected
  • The user receives appropriate feedback
  • The redirect flow functions correctly
  • The error handling doesn't interfere with normal job operations

The tests follow the existing patterns seen in the codebase, using RSpec and maintaining consistency with the current testing conventions.

Error Details

Summary:

GoodJob::AdvisoryLockable::RecordAlreadyAdvisoryLockedError: GoodJob::AdvisoryLockable::RecordAlreadyAdvisoryLockedError (GoodJob::AdvisoryLockable::RecordAlreadyAdvisoryLockedError)

Stacktrace:

/home/deploy/maybe-production/shared/bundle/ruby/3.4.0/gems/good_job-4.8.2/app/models/concerns/good_job/advisory_lockable.rb:355
/home/deploy/maybe-production/shared/bundle/ruby/3.4.0/gems/good_job-4.8.2/app/models/concerns/good_job/advisory_lockable.rb:374
/home/deploy/maybe-production/shared/bundle/ruby/3.4.0/gems/good_job-4.8.2/app/models/good_job/job.rb:552
/home/deploy/maybe-production/shared/bundle/ruby/3.4.0/gems/good_job-4.8.2/app/controllers/good_job/jobs_controller.rb:93
action_controller/metal/basic_implicit_render.rb:8
abstract_controller/base.rb:226
action_controller/metal/rendering.rb:193
abstract_controller/callbacks.rb:261
active_support/callbacks.rb:121
i18n.rb:353
/home/deploy/maybe-production/shared/bundle/ruby/3.4.0/gems/good_job-4.8.2/app/controllers/good_job/application_controller.rb:37
active_support/callbacks.rb:130
turbo-rails.rb:24
/home/deploy/maybe-production/shared/bundle/ruby/3.4.0/gems/turbo-rails-2.0.11/app/controllers/concerns/turbo/request_id_tracking.rb:10
active_support/callbacks.rb:130
action_text/rendering.rb:25
action_text/engine.rb:71
active_support/callbacks.rb:130
active_support/callbacks.rb:130
sentry/rails/controller_transaction.rb:21
sentry/hub.rb:115
sentry/span.rb:237
sentry/hub.rb:113
sentry-ruby.rb:506
sentry/rails/controller_transaction.rb:18
active_support/callbacks.rb:130
active_support/callbacks.rb:141
abstract_controller/callbacks.rb:260
action_controller/metal/rescue.rb:27
action_controller/metal/instrumentation.rb:77
active_support/notifications.rb:210
active_support/notifications/instrumenter.rb:58
sentry/rails/tracing.rb:56
active_support/notifications.rb:210
action_controller/metal/instrumentation.rb:76
action_controller/metal/params_wrapper.rb:259
active_record/railties/controller_runtime.rb:39
abstract_controller/base.rb:163
action_view/rendering.rb:40
action_controller/metal.rb:252
action_controller/metal.rb:335
action_dispatch/routing/route_set.rb:67
action_dispatch/routing/route_set.rb:50
action_dispatch/journey/router.rb:53
action_dispatch/journey/router.rb:133
action_dispatch/journey/router.rb:126
action_dispatch/journey/router.rb:126
action_dispatch/journey/router.rb:34
action_dispatch/routing/route_set.rb:896
rails/engine.rb:535
rails/railtie.rb:226
rails/railtie.rb:226
action_dispatch/routing/mapper.rb:33
action_dispatch/routing/mapper.rb:62
action_dispatch/journey/router.rb:53
action_dispatch/journey/router.rb:133
action_dispatch/journey/router.rb:126
action_dispatch/journey/router.rb:126
action_dispatch/journey/router.rb:34
action_dispatch/routing/route_set.rb:896
rails-settings/middleware.rb:9
logtail-rails/error_event.rb:24
logtail-rack/http_events.rb:213
logtail-rack/user_context.rb:74
logtail-rails/session_context.rb:16
logtail-rack/http_context.rb:23
logtail/current_context.rb:120
logtail/current_context.rb:44
logtail-rack/http_context.rb:22
rack/tempfile_reaper.rb:20
rack/etag.rb:29
rack/conditional_get.rb:43
rack/head.rb:15
action_dispatch/http/permissions_policy.rb:38
action_dispatch/http/content_security_policy.rb:38
rack/session/abstract/id.rb:274
rack/session/abstract/id.rb:268
action_dispatch/middleware/cookies.rb:704
action_dispatch/middleware/callbacks.rb:31
active_support/callbacks.rb:101
action_dispatch/middleware/callbacks.rb:30
sentry/rails/rescued_exception_interceptor.rb:14
action_dispatch/middleware/debug_exceptions.rb:31
sentry/rack/capture_exceptions.rb:30
sentry/hub.rb:269
sentry-ruby.rb:419
sentry/rack/capture_exceptions.rb:21
sentry/hub.rb:59
sentry-ruby.rb:399
sentry/rack/capture_exceptions.rb:20
action_dispatch/middleware/show_exceptions.rb:32
rails/rack/logger.rb:41
rails/rack/logger.rb:29
action_dispatch/middleware/remote_ip.rb:96
action_dispatch/middleware/request_id.rb:33
rack/method_override.rb:28
rack/runtime.rb:24
action_dispatch/middleware/executor.rb:16
action_dispatch/middleware/static.rb:27
rack/sendfile.rb:114
action_dispatch/middleware/ssl.rb:82
action_dispatch/middleware/assume_ssl.rb:24
mini_profiler.rb:191
rails/engine.rb:535
puma/configuration.rb:279
puma/request.rb:99
puma/thread_pool.rb:390
puma/request.rb:98
puma/server.rb:472
puma/server.rb:254
puma/thread_pool.rb:167

Tip

You can make revisions or ask questions of Revise.dev by using /revise in any comment or review!

  • /revise Add a comment above the method to explain why we're making this change.
  • /revise Why did you choose to make this change specifically?

Important

If something doesn’t look right, click to retry this interaction.

Quick links: View in SentryView in Revise

@Shpigford Shpigford closed this Feb 4, 2025
@Shpigford Shpigford deleted the revise/fix-maybe-rails-dk-1738690400 branch February 5, 2025 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant