AO3-4037: Automatically retry failing emails #3397
Conversation
| else | ||
| raise error | ||
| end | ||
| } |
There was a problem hiding this comment.
Layout/TrailingBlankLines: Final newline missing.
| @@ -1 +1,10 @@ | |||
| Resque::Mailer.excluded_environments = [:test, :cucumber] | |||
| Resque::Mailer.error_handler = lambda { |mailer, message, error, action, args| | |||
There was a problem hiding this comment.
Lint/UnusedBlockArgument: Unused block argument - message. If it's necessary, use _ or _message as an argument name to indicate that it won't be used.
| layout 'mailer' | ||
| helper :mailer | ||
| default from: "Archive of Our Own " + "<#{ArchiveConfig.RETURN_ADDRESS}>" | ||
| class KudoMailer < ArchiveMailer |
There was a problem hiding this comment.
Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
| layout 'mailer' | ||
| helper :mailer | ||
| default from: "Archive of Our Own " + "<#{ArchiveConfig.RETURN_ADDRESS}>" | ||
| class CommentMailer < ArchiveMailer |
There was a problem hiding this comment.
Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
| @@ -1,14 +1,9 @@ | |||
| class CollectionMailer < ActionMailer::Base | |||
| include Resque::Mailer # see README in this directory | |||
| class CollectionMailer < ArchiveMailer | |||
There was a problem hiding this comment.
Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
| helper :mailer | ||
| default from: "Archive of Our Own " + "<#{ArchiveConfig.RETURN_ADDRESS}>" | ||
|
|
||
| end |
There was a problem hiding this comment.
Layout/TrailingBlankLines: Final newline missing.
| layout 'mailer' | ||
| helper :mailer | ||
| default from: "Archive of Our Own " + "<#{ArchiveConfig.RETURN_ADDRESS}>" | ||
|
|
There was a problem hiding this comment.
Layout/EmptyLinesAroundClassBody: Extra empty line detected at class body end.
| @@ -0,0 +1,8 @@ | |||
| class ArchiveMailer < ActionMailer::Base | |||
There was a problem hiding this comment.
Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
| helper :mailer | ||
| default from: "Archive of Our Own " + "<#{ArchiveConfig.RETURN_ADDRESS}>" | ||
|
|
||
| class AdminMailer < ArchiveMailer |
There was a problem hiding this comment.
Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
elzj
changed the title
|
I think we should send emails from |
| if error.is_a?(Resque::TermException) | ||
| Resque.enqueue(mailer, action, *args) | ||
| elsif error.is_a?(ActiveRecord::RecordNotFound) && !args.include?(:retried) | ||
| Resque.enqueue_in(5.minutes, mailer, action, :retried, *args) |
There was a problem hiding this comment.
This looks like it prepends the symbol :retried to the arguments passed to the mailer. Doesn't that mean all the arguments will be shifted over by 1, and the first argument (which is frequently supposed to be an ID of some sort) will be :retried? Or is there some code that strips the :retried symbol?
Also, I poked around the mailer code and I think Resque::Mailer.perform expects the arguments to be serialized:
https://github.com/zapnap/resque_mailer/blob/35c2a7c7ff9038b9bff95c50190d162f95afcd5f/lib/resque_mailer.rb#L52-L54
|
After some more poking around in the Resque Mailer code, I don't think it's going to be possible to use the built-in error handling to accomplish this, at least not without modifying some of the mailer code. For any mailers that take advantage of I'd suggest either modifying (Alternatively, it might be possible to report this bug and get the Resque Mailer to handle errors in the serializer more sensibly.) |
|
Just doing some housekeeping! We're going to close this since it doesn't sound like the approach will work. The issue will still be open and assigned to you if you want to take another crack at it, though! <3 |
Issue
https://otwarchive.atlassian.net/browse/AO3-4037
Purpose
Rescues and retries resque email failures that occur when redis is ahead of mysql, which should hopefully keep those out of the failed jobs list, making it easier to track. Also cleans up a bit of duplication in the mailers.
Testing
It could be hard to simulate this on staging, so we may need a database admin to create some fake failures.