Bug #36103 for MailTools: Synopsis should show error checking

Thu May 22 11:43:31 2008 EDAVIS [...] cpan.org - Ticket created

Subject:

Synopsis should show error checking

The example code in the Mail::Send pod documentation should check for errors, for example $fh = $msg->open or die "could not make message"; print $fh "Body of message" or die "could not write to message body"; $fh->close or die "could not send message"; After all, surely it is possible that sending the message fails. Even if print() to the filehandle cannot ever fail with the current Mail::Send implementation, that might not always be the case, and the normal specification of print() is that it returns false on failure. So I think the example code should encourage people to check for errors rather than ignoring them. (A small section in the doc on error handling would be good too.)

Fri May 23 04:00:07 2008 MARKOV [...] cpan.org - Correspondence added

AFAIK, checking each print() does not add anything, because those errors will also be reported by close(). Not being the author of the code, I thing that I can deduct that the Mail::Send->open (is Mail::Mailer->new()) dies on errors. So also here you do not need to check the return code. If you would update the man-page, it probably should be: my $s = eval { Mail::Send->new }; die if $@; print $s "text\n"; close $s or die; Right?

Fri May 23 04:00:09 2008 The RT System itself - Status changed from 'new' to 'open'

Fri May 23 14:13:40 2008 EDAVIS [...] cpan.org - Correspondence added

Show quoted text

>AFAIK, checking each print() does not add anything, because those >errors will also be reported by close().

Fair enough - in this case there needs to be an explicit statement in the doc that you don't need to check print() but you should check close().

Fri May 23 14:39:47 2008 Mark [...] Overmeer.net - Correspondence added

Subject:	Re: [rt.cpan.org #36103] Synopsis should show error checking
Date:	Fri, 23 May 2008 20:39:10 +0200
To:	EDAVIS via RT <bug-MailTools [...] rt.cpan.org>
From:	Mark Overmeer <mark [...] overmeer.net>

* EDAVIS via RT (bug-MailTools@rt.cpan.org) [080523 18:13]: Show quoted text

> Queue: MailTools > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=36103 > >

> >AFAIK, checking each print() does not add anything, because those > >errors will also be reported by close().

> > Fair enough - in this case there needs to be an explicit statement in > the doc that you don't need to check print() but you should check close().

It seems fair to only describe what people should do (check close() in Mail::Send and Mail::Mailer), and to the describe what people do not have to do. So, I leave that part to "perldoc -f print" -- which does not explain it ;-b -- Regards, MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net

Tue May 27 04:07:52 2008 EDAVIS [...] cpan.org - Correspondence added

perlfunc(1): Show quoted text

> print Prints a string or a list of strings. Returns true if > successful.

This implies that you should always check the return value of print, since it could fail and return false. (Unless you really don't care whether it succeeds or not.) So if this is not the case for Mail::Send, you do need to say so. However, you leave yourself more wiggle room for future expansion if you document that print() can fail, and give example code that checks it.

Tue May 27 05:06:39 2008 solutions [...] overmeer.net - Correspondence added

Subject:	Re: [rt.cpan.org #36103] Synopsis should show error checking
Date:	Tue, 27 May 2008 11:06:08 +0200
To:	EDAVIS via RT <bug-MailTools [...] rt.cpan.org>
From:	Mark Overmeer <solutions [...] overmeer.net>

* EDAVIS via RT (bug-MailTools@rt.cpan.org) [080527 08:07]: Show quoted text

> Queue: MailTools > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=36103 > > > perlfunc(1): >

> > print Prints a string or a list of strings. Returns true if > > successful.

> > This implies that you should always check the return value of print, > since it could fail and return false. (Unless you really don't care > whether it succeeds or not.)

This return status indicates an error when the OS has signalled the program that something is wrong with the file-handle. It does not reflect whether the actual characters were written. Characters are written into the output buffers, which are within your program, until a buffer of 4 kB is full. Then, the stdio will transfer it to kernel space. If something goes wrong there, the last print will probably return an error. However, once in the kernel, it is still not sure that the characters will end-up on disk. The manual page of close explains it NOTES Not checking the return value of close is a common but nevertheless serious programming error. It is quite possible that errors on a pre‐ vious write(2) operation are first reported at the final close. Not checking the return value when closing the file may lead to silent loss of data. This can especially be observed with NFS and with disk quota. So: I suspect that checking the return value of print is giving you unjustified feeling of additional protection. It doesn't help much, and makes programs overly complicated. The best sign that something is wrong you probably get from Mail::Send/Mail::Mailer is a SIGPIPE from the OS. Don't ignore SIGPIPE. -- Regards, MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net

Thu May 29 07:05:19 2008 EDAVIS [...] cpan.org - Correspondence added

Thanks for the explanation. Unless $| is on, you are surely right that checking close() is important but checking print() is less so. (All that is needed is a guarantee that if there were any errors writing to the file earlier on, close() will pick them up.)

Thu May 29 07:53:14 2008 solutions [...] overmeer.net - Correspondence added

Subject:	Re: [rt.cpan.org #36103] Synopsis should show error checking
Date:	Thu, 29 May 2008 13:52:32 +0200
To:	EDAVIS via RT <bug-MailTools [...] rt.cpan.org>
From:	Mark Overmeer <solutions [...] overmeer.net>

* EDAVIS via RT (bug-MailTools@rt.cpan.org) [080529 11:05]: Show quoted text

> Thanks for the explanation. Unless $| is on, you are surely right that > checking close() is important but checking print() is less so.

Even when $|==1, the return print() will only show a very limited number of possible problems. If you use fsync(), then you will get more errors immediately. In either case, performance suffers tremendously. Both $| and fsync() should be used a little as possible. Getting speedy return codes with print is not a valid reason (in 99.9% of the cases). -- MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net

Wed Jul 09 17:18:42 2008 MARKOV [...] cpan.org - Correspondence added

no use

Wed Jul 09 17:18:44 2008 MARKOV [...] cpan.org - Status changed from 'open' to 'resolved'