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

Specify source of SolidQueue to ErrorReporter #446

Closed
tmimura39 opened this issue Dec 8, 2024 · 2 comments · Fixed by #447
Closed

Specify source of SolidQueue to ErrorReporter #446

tmimura39 opened this issue Dec 8, 2024 · 2 comments · Fixed by #447

Comments

@tmimura39
Copy link
Contributor

Because SolidQueue uses the Rails executor, exceptions raised in a Job are automatically handled by ActiveSupport::ErrorReporter.

This is very convenient.
I want to make this even more convenient.

Proposal

Simply put, this is what I propose.

diff --git a/lib/solid_queue/app_executor.rb b/lib/solid_queue/app_executor.rb
index da0976f..0580213 100644
--- a/lib/solid_queue/app_executor.rb
+++ b/lib/solid_queue/app_executor.rb
@@ -4,7 +4,7 @@ module SolidQueue
   module AppExecutor
     def wrap_in_app_executor(&block)
       if SolidQueue.app_executor
-        SolidQueue.app_executor.wrap(&block)
+        SolidQueue.app_executor.wrap(source: "application.solid_queue", &block)
       else
         yield
       end

Currently, the default value application.active_support is applied for source.
This does not identify it as an exception raised inside SolidQueue.

This is a quote from the rails guide.

source: a String about the source of the error. The default source is "application". Errors reported by internal libraries may set other sources; the Redis cache library may use "redis_cache_store.active_support", for instance. Your subscriber can use the source to ignore errors you aren't interested in.

Again, examples of the use of source were explained.
rails/rails#44999

What do you think?

@rosa
Copy link
Member

rosa commented Dec 8, 2024

Ohh, this is a great suggestion! I didn't know this, somehow missed it! 😳 But it makes complete sense. Would you like to open a PR with this change?

@tmimura39
Copy link
Contributor Author

Thanks for the response.

I'll do it today👍

tmimura39 pushed a commit to tmimura39/solid_queue that referenced this issue Dec 9, 2024
Currently, the default value `application.active_support` is applied for source.
This does not identify it as an exception raised inside SolidQueue.

> source: a String about the source of the error. The default source is "application". Errors reported by internal libraries may set other sources; the Redis cache library may use "redis_cache_store.active_support", for instance. Your subscriber can use the source to ignore errors you aren't interested in.

This is a quote from the [rails guide](https://github.com/rails/rails/blob/v8.0.0/guides/source/error_reporting.md?plain=1#L216-L220).

Follow this guide to specify the source so that it can be determined that the error originated from inside SolidQueue.

Fixes rails#446
tmimura39 added a commit to tmimura39/solid_queue that referenced this issue Dec 9, 2024
Currently, the default value `application.active_support` is applied for source.
This does not identify it as an exception raised inside SolidQueue.

> source: a String about the source of the error. The default source is "application". Errors reported by internal libraries may set other sources; the Redis cache library may use "redis_cache_store.active_support", for instance. Your subscriber can use the source to ignore errors you aren't interested in.

This is a quote from the [rails guide](https://github.com/rails/rails/blob/v8.0.0/guides/source/error_reporting.md?plain=1#L216-L220).

Follow this guide to specify the source so that it can be determined that the error originated from inside SolidQueue.

Fixes rails#446
@rosa rosa closed this as completed in #447 Dec 9, 2024
rosa pushed a commit that referenced this issue Dec 9, 2024
Currently, the default value `application.active_support` is applied for source.
This does not identify it as an exception raised inside SolidQueue.

> source: a String about the source of the error. The default source is "application". Errors reported by internal libraries may set other sources; the Redis cache library may use "redis_cache_store.active_support", for instance. Your subscriber can use the source to ignore errors you aren't interested in.

This is a quote from the [rails guide](https://github.com/rails/rails/blob/v8.0.0/guides/source/error_reporting.md?plain=1#L216-L220).

Follow this guide to specify the source so that it can be determined that the error originated from inside SolidQueue.

Fixes #446
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 a pull request may close this issue.

2 participants