277437 - Use Mail::Mailer (Perl module) for mail delivery

Status: RESOLVED FIXED

(Bugzilla :: Email Notifications, enhancement)

Description

[Andrei Benea]

Using Mail::Mailer will provide at least 3 methods of delivery (sendmail, smtp, 
qmail). This will make Windows users happy (and Linux users without a working 
sendmail too).

Comment 1

[Andrei Benea]

Attachment: Mimic current mail delivery method

This patch tries to emulate the current behaviour using the Mail::Mailer
module.

Comment 2

[Gervase Markham [:gerv]]

Can you explain exactly how this works, and how it differs from what happens
now? I can't see where, for example, you configure the SMTP server to use...

Gerv

Comment 3

[Andrei Benea]

(In reply to comment #2)
> Can you explain exactly how this works, and how it differs from what happens
> now? I can't see where, for example, you configure the SMTP server to use...
> 
> Gerv

First of all this patch is a proof of concept. It doesn't add any functionality. 
It is intended to behave exactly like the current function.

To change the method used for delivery to smtp the following line

$mailer = new Mail::Mailer "sendmail", "-ODeliveryMode=deferred";

must be changed to:

$mailer = new Mail::Mailer "smtp", Server => "bugzilla.org";

Obviously, this patch will need some parameters (mail_delivery_method and 
smtp_server). It also needs to update the dependency list.

I posted this early version to get some feedback on the approach used. After so 
many failed attempts (rejected patches) in MTAConfig bugs, I wanted to be sure I 
am going in the right direction before I start dealing with the details.

Comment 4

[Gervase Markham [:gerv]]

> I wanted to be sure I 
> am going in the right direction before I start dealing with the details.

Cool :-) One question on the direction, then: are you sure Mail::Mailer is the
right module? I heard a talk at a recent Perl conference about the Email::*
modules, where their maintainer (OK, so he's biased) said that they had been
written in response to various flaws/lack of maintenance of the Mail::* modules.

Would it be wise to investigate/use the Email::* set of modules? (There's also
Mail::BulkMailer, but there was a big argument last time I suggested that.)

Gerv

Comment 5

[:glob ✱]

i've just been playing with Email::Send::SMTP and it appears to have a bug that
causes it to never delever email :(  i've notified the maintainer.

however Email::Send does look much cleaner than MailTools, especially with
respect to partially failed delivery (where only some of the recipients worked).

Comment 6

[Andrei Benea]

(In reply to comment #4)
> Cool :-) One question on the direction, then: are you sure Mail::Mailer is the
> right module? I heard a talk at a recent Perl conference about the Email::*
> modules, where their maintainer (OK, so he's biased) said that they had been
> written in response to various flaws/lack of maintenance of the Mail::* 
modules.

I must admit that this module has a very weird interface and incomplete 
documentation, but it worked perfectly with both smtp and sendmail in my tests. 

I'll give Email::Send a try, which looks better (we don't have to parse the 
headers anymore -- I am forced to do this because of the odd interface of Mail::
Mailer...)

Comment 7

[Andrei Benea]

Email::Send seems to be completely broken. It doesn't work with SMTP or 
sendmail.

Anyway, the code effort required to switch betweem Email::SMTP and Mail::Mailer 
is really small (less than 10 lines of code). I suggest we go with Mail::Mailer 
and switch later to Email::Send if needed.

Comment 8

[Gervase Markham [:gerv]]

> Email::Send seems to be completely broken. It doesn't work with SMTP or 
> sendmail.

What makes you say that? I used it in a webapp I wrote the other week and it
worked fine. It doesn't use external programs like sendmail, but does the SMTP
itself, so I don't know what you mean by "it doesn't work with sendmail".

Are you using its API correctly?

Gerv

Comment 9

[Andrei Benea]

(In reply to comment #8)
> What makes you say that? I used it in a webapp I wrote the other week and it
> worked fine. It doesn't use external programs like sendmail, but does the SMTP
> itself, so I don't know what you mean by "it doesn't work with sendmail".

I think this is a misunderstanding. I was referring to this module: http://
search.cpan.org/~cwest/Email-Send-1.45/lib/Email/Send.pm . It has multiple 
mailer backends, just like Mail::Mailer.

Comment 10

[Andrei Benea]

Attachment: Mail::Mailer v1

Tested with sendmail, SMTP and testfile.

Comment 11

[Vlad Dascalu]

Comment on attachment 170728 [details] [diff] [review]
Mail::Mailer v1

Tested the ppm installation on Windows. Works as expected, and the params do
appear in the editparams.cgi page correctly.

+	    'Mail::Mailer manual)',

manual --> documentation, and probably a dot at the end of the sentence
wouldn't hurt. (that's a nit)

It would be cool to send the whole message to Mail::Mailer, and let it parse
the headers itself instead on relying to the rather hackish nature of +    $msg
=~ /(.*?)\n\n(.*)/ms; but that's not possible as far as Andrei is concerned.
This works and is good to go.

Comment 12

[Vlad Dascalu]

The documentation patch for Net::SMTP Windows support is very difficult to
re-written with the actual MessageToMTA arhitecture. We could commit this
instead and ditch that paragraph from the documentation altogether.

It seems the need to re-write that documentation section doesn't appear nowhere
nowadays on the list of 2.18 blockers. It has a documention? flag on mkanat's
MessageToMTA patch, but that's FIXED and it's not picked up by the current queries.

Comment 13

[Shane H. W. Travis]

(In reply to comment #12)
> It seems the need to re-write that documentation section doesn't appear
> nowhere nowadays on the list of 2.18 blockers. It has a documention? flag on
> mkanat's MessageToMTA patch, but that's FIXED and it's not picked up by the
> current queries.

When I look, the "For version 2.18 documentation", query on the Documentor's 
Guide (http://www.bugzilla.org/docs/documentor.html) picks up the need for 
documentation on bug 59351 just fine.

I'm not sure what queries you're using that don't show it... yes, it's not a 
documentation bug, but that's what the documentation *flag* is for... to show 
that 'this feature needs some docs!'

Comment 14

[Max Kanat-Alexander]

You could also change the documentation to point to this patch, right?

Comment 15

[Dave Miller [:justdave]]

(In reply to comment #14)
> You could also change the documentation to point to this patch, right?

Hmm, that's a good idea for 2.18/2.20.  This is changing just enough (additional
Perl module dependency) that it shouldn't go in during the freeze.  Adding
dependencies after a release freeze is to be avoided except where necessary to
fix a security issue.  We can do this right after we branch though.

I do note that this bug is identical (exact dupe) of bug 49893.  Since we have a
reviewed patch here, though, it'll likely get duped to this one instead.  This
bug should probably inherit all of that one's dependencies in that case. 
There's some good discussion there that should probably be read, too.  I think
most of the hesitations on that bug have been separated and are now covered on
bug 249121, and this patch avoids turning that one into a regression by still
allowing Sendmail to be used (which is a good thing :)

Things for the docs:  Using SMTP allows you to send mail in an environment where
you don't have Sendmail available to use.  However, unlike Sendmail, it doesn't
perform any queuing.  This means if a temporary failure occurs when sending mail
(such as the SMTP server being down, or the SMTP server issuing a "try again
later" response), the mail will be discarded.  Thus the advantage to using
Sendmail (or another Sendmail-compatible MTA) is that Sendmail will queue the
message (up to 5 days by default - the time can be configured in your sendmail
configuration) and keep trying every so often when those types of errors occur.

Comment 16

[Vlad Dascalu]

Targeting bug to 2.20 since the 2.20 feature freeze was canceled.

Comment 17

[Dave Miller [:justdave]]

I have a feeling this will conflict with the patch on bug 277623, but the
conflict should be minor and easily resolved on checkin.

Comment 18

[Shane H. W. Travis]

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.335; previous revision: 1.334
done
Checking in defparams.pl;
/cvsroot/mozilla/webtools/bugzilla/defparams.pl,v  <--  defparams.pl
new revision: 1.144; previous revision: 1.143
done
Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v  <--  BugMail.pm
new revision: 1.25; previous revision: 1.24
done

Comment 19

[Frédéric Buclin]

Andrei, Vladd, is version 1.65 required? I have installed Mandrake Linux 10.1
Official and this distro has MailTools 1.62 only, so that checksetup.pl
complains. Is there any good reason to require such a high version?

Comment 20

[Andrei Benea]

(In reply to comment #19)
> Andrei, Vladd, is version 1.65 required? I have installed Mandrake Linux 10.1
> Official and this distro has MailTools 1.62 only, so that checksetup.pl
> complains. Is there any good reason to require such a high version?

This is the only version I tested. If it works fine with 1.62, you can write a 
trivial patch to change the version requirement.

Comment 21

[Max Kanat-Alexander]

Added to the release notes in bug 286274.

Comment 22

[Frédéric Buclin]

The documentation has been fixed as part of bug 313469, which among others mentions the new 'maildeliverymethod' parameter. I don't think we need more.