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

Feature request: Provide a hook to handle/report exceptions in processes forked by Supervisor::ForkSupervisor #297

Closed
floehopper opened this issue Aug 23, 2024 · 8 comments

Comments

@floehopper
Copy link

Recently I had an issue when running rake solid_queue:start where I was seeing the following exception, because I had neglected to generate & run the migrations in the upgrading instructions:

ruby/3.3.0/lib/ruby/gems/3.3.0/gems/activerecord-7.1.3.4/lib/active_record/insert_all.rb:161:in `find_unique_index_for': No unique index found for key (ArgumentError)

          raise ArgumentError, "No unique index found for #{name_or_columns}"
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/activerecord-7.1.3.4/lib/active_record/insert_all.rb:35:in `initialize'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/activerecord-7.1.3.4/lib/active_record/persistence.rb:363:in `new'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/activerecord-7.1.3.4/lib/active_record/persistence.rb:363:in `upsert_all'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/app/models/solid_queue/recurring_task.rb:28:in `create_or_update_all'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/dispatcher/recurring_schedule.rb:44:in `persist_tasks'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/dispatcher/recurring_schedule.rb:20:in `block in schedule_tasks'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/activesupport-7.1.3.4/lib/active_support/execution_wrapper.rb:92:in `wrap'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/app_executor.rb:7:in `wrap_in_app_executor'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/dispatcher/recurring_schedule.rb:19:in `schedule_tasks'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/dispatcher.rb:42:in `schedule_recurring_tasks'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/activesupport-7.1.3.4/lib/active_support/callbacks.rb:403:in `block in make_lambda'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/activesupport-7.1.3.4/lib/active_support/callbacks.rb:239:in `block in halting_and_conditional'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/activesupport-7.1.3.4/lib/active_support/callbacks.rb:602:in `block in invoke_after'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/activesupport-7.1.3.4/lib/active_support/callbacks.rb:602:in `each'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/activesupport-7.1.3.4/lib/active_support/callbacks.rb:602:in `invoke_after'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/activesupport-7.1.3.4/lib/active_support/callbacks.rb:111:in `run_callbacks'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/processes/runnable.rb:13:in `block in start'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/activesupport-7.1.3.4/lib/active_support/notifications.rb:206:in `block in instrument'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/activesupport-7.1.3.4/lib/active_support/notifications/instrumenter.rb:58:in `instrument'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/activesupport-7.1.3.4/lib/active_support/notifications.rb:206:in `instrument'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue.rb:59:in `instrument'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/processes/runnable.rb:12:in `start'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/supervisor/fork_supervisor.rb:40:in `block in start_process'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/supervisor/fork_supervisor.rb:39:in `fork'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/supervisor/fork_supervisor.rb:39:in `start_process'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/supervisor.rb:47:in `block in start_processes'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/supervisor.rb:47:in `each'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/supervisor.rb:47:in `start_processes'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/supervisor.rb:24:in `start'
  from <internal:kernel>:90:in `tap'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/supervisor.rb:13:in `start'
  from ruby/3.3.0/lib/ruby/gems/3.3.0/gems/solid_queue-0.5.0/lib/solid_queue/tasks.rb:4:in `block (2 levels) in <top (required)>'
  ...

These exceptions were not being reported to our exception monitoring tool (Rollbar).

While I have now resolved the ArgumentError problem, I wanted to avoid missing exceptions like this in the future. I tried setting the on_thread_error configuration option, but this didn't seem to handle exceptions like the ArgumentError above. In the end the only way that worked was to add a begin ... rescue ... end construct inside the fork block in Supervisor::ForkSupervisor#start_process. Is there a better way to do this or is it worth providing a configuration hook to report exceptions like this...?

@rosa
Copy link
Member

rosa commented Aug 23, 2024

Oh, interesting! It seems like Rollbar doesn't provide a subscriber for Rails's error reporting? Exceptions like this one are handled by Rails's error reporting automatically so you shouldn't need to configure anything... but it's possible Rollbar is outdated?

@floehopper
Copy link
Author

floehopper commented Aug 23, 2024

Oh, interesting! It seems like Rollbar doesn't provide a subscriber for Rails's error reporting? Exceptions like this one are handled by Rails's error reporting automatically so you shouldn't need to configure anything... but it's possible Rollbar is outdated?

Ah, I wondered whether that was the case. Is there somewhere in solid_queue other than in on_thread_error that reports exceptions to Rails' error reporting? I tried printing something to stdout from within that on_thread_error block and it didn't seem to print anything when the exception occurred, but perhaps that stdout wasn't connected to the parent process stdout...? I guess my question is: are you confident that the exception above would have been caught by the on_thread_error block...?

If so, I'll close this and work on getting Rollbar to play nicely with Rails error reporting...

PS Thanks for the great library! ❤️

@rosa
Copy link
Member

rosa commented Aug 24, 2024

Thanks a lot @floehopper! 🙏

I guess my question is: are you confident that the exception above would have been caught by the on_thread_error block...?

No, it wouldn't have been captured by that, but I think it should have been handled by Rails's error reporter regardless, being run within executor.wrap. How does Rollbar hook into Rails errors? Sentry, for example, registers an error subscriber during initialization, and it definitely got this kind of error 🤔

In any case, I realise the way on_thread_error works is quite confusing as it is now. It was something I added in the very beginning, before even having a supervisor that forked processes, so I'll revisit it to make it clearer.

@floehopper
Copy link
Author

No, it wouldn't have been captured by that, but I think it should have been handled by Rails's error reporter regardless, being run within executor.wrap.

Ah, I missed that - thanks so much for explaining.

I have just verified that the ArgumentError exception I was seeing is indeed reported successfully via the Rails error reporter, so I'm going to close this and focus on sorting out the Rollbar integration with the Rails error reporter.

@floehopper
Copy link
Author

Just in case anyone else comes across this - Rollbar added support for the Rails error reporter in v3.6.0.

@rosa
Copy link
Member

rosa commented Sep 5, 2024

Oh, nice! 🙏 Thanks a lot for posting that information.

@npezza93
Copy link

npezza93 commented Oct 5, 2024

In case someone comes along this, I dont think active job reports errors to the error reporter. I've opened rails/rails#53201 to start doing this. Tools like sentry monkey patch active job to send jobs to the reporter: https://github.com/getsentry/sentry-ruby/blob/master/sentry-rails/lib/sentry/rails/active_job.rb#L4.

@rosa
Copy link
Member

rosa commented Oct 5, 2024

@npezza93 indeed! Thanks a lot for doing that 🙏

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

No branches or pull requests

3 participants