715806 - Emails can get stuck in TheSchwartz's queue because Email::Send::Sendmail doesn't deal with sendmail errors correctly

Status: RESOLVED FIXED

(Bugzilla :: Email Notifications, defect)

Description

[:glob ✱]

Email::Send::Sendmail treats all sendmail errors as transient, which results in mail jobs being "stuck" in the ts_job table.

for example:

| There was an error sending mail from '[e-mail]' to 'someone@example..com': error when closing pipe to /usr/lib/sendmail: |

in this case sendmail will have set the exit status to EX_HOST (host name unknown), which should be treated as a fatal error.

unfortunately there's no mechanism to signal to bugzilla that this message failed, and it should be removed -- currently all failures are treating as transient, and will always result in a retry.  so we'll have to tell bugzilla that the job was successful in order for it to remove it from the queue.  this isn't a massive problem, as there isn't anything in bugzilla which alerts admins of these failures anyhow.

Email::Send is deprecated project, so we won't be getting a fix into Email::Send::Sendmail.  however it would be pretty simple to write our own sendmail handler for Email::Send which identifies fatal errors from sendmail and returns success.

Comment 1

[:glob ✱]

Attachment: patch v1

i'm not even sure i like this solution, but short of a major overhaul i can't see a simpler way.

Comment 2

[Frédéric Buclin]

Comment on attachment 586359 [details] [diff] [review]
patch v1

>=== added file 'Bugzilla/Send/Sendmail.pm'

>+package Bugzilla::Send::Sendmail;

Move this line after the license block.


>+
>+# ***** BEGIN LICENSE BLOCK *****
>+# Version: MPL 1.1

Use MPL 2.0, see bug 680131 comment 22 for the exact license header to use with the Bugzilla project.


>+use File::Spec();
>+use Return::Value;
>+use Symbol qw(gensym);

We are sure these modules are available, right?


I'm really not the right reviewer for this patch. First because I never used JobQueue and I don't have the required modules to test it, and second because I don't use Sendmail. Redirecting to dkl, assuming he is a better reviewer than me about sendmail + jobqueue.

Comment 3

[Max Kanat-Alexander]

Comment on attachment 586359 [details] [diff] [review]
patch v1

Review of attachment 586359 [details] [diff] [review]:
-----------------------------------------------------------------

::: Bugzilla/Send/Sendmail.pm
@@ +1,1 @@
> +package Bugzilla::Send::Sendmail;

Could you subclass Email::Send::Sendmail instead of copying all its code?

Comment 4

[:glob ✱]

Attachment: patch v2

> Use MPL 2.0, see bug 680131 comment 22 for the exact license header to use
> with the Bugzilla project.

done

> >+use File::Spec();
> >+use Return::Value;
> >+use Symbol qw(gensym);
> 
> We are sure these modules are available, right?

yes, they are required by Email::Send

> Could you subclass Email::Send::Sendmail instead of copying all its code?

done

Comment 5

[David Lawrence [:dkl]]

Comment on attachment 586902 [details] [diff] [review]
patch v2

Review of attachment 586902 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me and works as expected. I was not able to recreate every exit code individually but they look correct to me
based on sysexits.h. r=dkl

Comment 6

[:glob ✱]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Mailer.pm
added Bugzilla/Send
added Bugzilla/Send/Sendmail.pm
Committed revision 8099.