smtp.SMTPSender reports Unhandled error in Deferred:

[twisted-trac]

	atsuoi reported
Trac ID	trac#3642
Type	enhancement
Created	2009-02-02 12:32:08Z
Branch	https://github.com/twisted/twisted/tree/smtpsender-error-3642

If connection was lost while sending mail data to the server, following errors are printed.

Traceback (most recent call last): Failure: exceptions.Exception: Consumer asked us to stop producing

I'll attach a file contains fix and test. I guess the test can be more simple, but I don't know how to make it simple.

Attachments:

• smtperr.diff (2850 bytes) - added by atsuoi on 2009-02-02 12:33:58Z -

Searchable metadata

``` trac-id__3642 3642 type__enhancement enhancement reporter__atsuoi atsuoi priority__normal normal milestone__ branch__branches_smtpsender_error_3642 branches/smtpsender-error-3642 branch_author__exarkun exarkun status__closed closed resolution__fixed fixed component__mail mail keywords__ time__1233577928000000 1233577928000000 changetime__1234642724000000 1234642724000000 version__None None owner__ ```

[twisted-trac]

	atsuoi removed owner

[twisted-trac]

	@exarkun set owner to @exarkun @exarkun set status to assigned

[twisted-trac]

	@exarkun removed owner @exarkun set status to new

Please review.

[twisted-trac]

	@therve set owner to @exarkun

+        If L{smtp.SMTPClientError.sendError} is called with an
+        L{SMTPClientError} which is not fatal, it sends C{"QUIT"} and waits for

+        If L{smtp.SMTPClientError.sendError} is called with an exception which
+        is not an L{SMTPClientError}, it disconnects its transport without

I think you meant smtp.SMTPClient.sendError here.

That's it, +1!

[twisted-trac]

	Automation removed owner

[twisted-trac]

	@exarkun set status to closed

(In [26218]) Merge smtpsender-error-3642

Author: exarkun Reviewer: therve Fixes: #3642

Handle failures from the FileSender.beginFileTransfer Deferred in SMTPClient. This lets errors which occur while writing out a message body be reported to the higher level API (such as sendmail) which is necessary to recognize that the message was not actually sent.

[twisted-trac]

	@exarkun commented

I'll skip the review comments which would be duplicates of those I posted on #3638.

1. Calling loseConnection with a reason isn't allowed. If you look up the interface definition (you can find it in the API docs) for ITransport (what a protocol's transport attribute typically provides), you'll see that loseConnection doesn't take an arguments. It's a bit unfortunate that some implementations actually do accept an argument, but this is an implementation detail. Following the interface will ensure that the code actually continues to work, and works against other implementations of the interface.
2. Silencing the error completely isn't a good way to get rid of it. Probably using smtpTransferFailed is a good way to get the error to some code which will be interested in it. This is important so that a retry can happen, if appropriate, or so that the Deferred returned by sendmail or one of the other dependent APIs will actually errback. If this doesn't happen, it may appear as though the mail was sent when it was not.

[twisted-trac]

	@exarkun commented

(In [26178]) Branching to 'smtpsender-error-3642'

[twisted-trac]

	@exarkun commented

(In [26217]) Fix mistakes in test docstrings

refs #3642