Add callback_url/acct information for Sidekiq PuSH workers Exception(alternative)#4283
Add callback_url/acct information for Sidekiq PuSH workers Exception(alternative)#4283clworld wants to merge 3 commits intomastodon:masterfrom
Conversation
| end | ||
| end | ||
| rescue HTTP::Error, OStatus2::BadSalmonError, Mastodon::NotPermittedError | ||
| rescue HTTP::Error, OStatus2::BadSalmonError, Mastodon::NotPermittedError, Mastodon::WorkerError |
There was a problem hiding this comment.
It's for deal with rspec error. WorkerError will be raised from inside of perform_async only when test.
| @payload = payload | ||
| process_delivery unless blocked_domain? | ||
| rescue => e | ||
| raise Mastodon::WorkerError, "Delivery failed for #{subscription&.callback_url}: #{e.class}: #{e.message}" |
There was a problem hiding this comment.
Wrap exception for adding callback_url to exception message.
| logger.debug "PuSH re-subscribing to #{account.acct}" | ||
| ::SubscribeService.new.call(account) | ||
| rescue => e | ||
| raise Mastodon::WorkerError, "Subscribe failed for #{account&.acct}: #{e.class}: #{e.message}" |
There was a problem hiding this comment.
Wrap exception for adding acct to exception message.
| rescue Mastodon::WorkerError => e | ||
| e.cause | ||
| end | ||
| ).to be_an(ActiveRecord::RecordNotFound) |
There was a problem hiding this comment.
I think there is better way to write test to check e.cause... but I didn't found it..
| class NotPermittedError < Error; end | ||
| class ValidationError < Error; end | ||
| class RaceConditionError < Error; end | ||
| class WorkerError < Error; end |
There was a problem hiding this comment.
Add exception for wrap exception in Sidekiq worker. (wrapped exceptions are raised to Sidekiq)
|
I like it but I kind of want to know @Gargron thoughts on the matter since he recently changed some exception handling stuff |
|
I chose the other PR for simplicity. |
3 participants
This is alternative implementation of #4281.
