Mails keep queued on unexpected errors

[eht16]

The default error handler (via MAILER_EMAIL_BACKEND setting) will log an info message and defer the mail if the error was one of the expected smtplib or socket errors.
This is fine.

On any other errors, the original exception is raised to make the user aware of the error.
Basically this is also fine but unfortunately breaks (probably intentionally) the loop over all queued mails in engine.send_all(). This leads to having all other queued mails to be unprocessed once an unexpected error occurs.

We actually experienced such a case recently where our code generated an invalid email which caused a ValueError in django.core.mail.message.sanitize_address and then all later queued mails were not processed anymore and the queue grew accordingly.

I see that this is basically intended behavior but I wonder if the above mentioned consequences are actually known.

For now, I overridden the default error handler to not raise an exception on unexpected errors but log them with error log level to get noticed but not break the processing of other mails in the queue.

@spookylukey are you open to PRs changing the current behavior and if so, do you have a preferred way of handling unexpected errors?

[spookylukey]

There is a difficulty with knowing what kind of errors can be produced, especially when different backends could all raise different exceptions. Have you looked at https://github.com/pinax/django-mailer/blob/master/docs/usage.rst#error-handling and is it not sufficient for your needs?

If the question is about changing the default behaviour, the answer depends on what changes you are suggesting :-)

I'm wary of changing behaviour that could cause more breakage in some other way for other users. For example, ignoring more exceptions, such as ignoring ValueError by default, which could include almost anything, could result in other undesirable behaviour, like repeatedly trying something (e.g. some external service) which is not going to work, potentially using up money or other resources.

If I was designing from the blank sheet I might do it differently, but there is value in known behaviour at this point.

[eht16]

There is a difficulty with knowing what kind of errors can be produced, especially when different backends could all raise different exceptions. Have you looked at https://github.com/pinax/django-mailer/blob/master/docs/usage.rst#error-handling and is it not sufficient for your needs?

I did and as said in the OP I already used "MAILER_ERROR_HANDLER" as workaround.

If the question is about changing the default behaviour, the answer depends on what changes you are suggesting :-) I'm wary of changing behaviour that could cause more breakage in some other way for other users. For example, ignoring more exceptions, such as ignoring `ValueError` by default, which could include almost anything, could result in other undesirable behaviour, like repeatedly trying something (e.g. some external service) which is not going to work, potentially using up money or other resources.

Agreed. I don't suggest to catch "ValueError", this is way too broad. One suggestion could be what I have done now (using a custom error handler): - use the existing list of exceptions to detect common network errors and the like and if matched, use "info" as log level. - for all other exceptions, use "error" as log level, include the full exception in the log record - in all cases, defer the mail and return normally from the error handler so the processing of subsequent mails in the queue will continue I see that this suggestion is probably not acceptable because it - requires the user to distinguish between expected and unknown errors based on the log level - obviously changes the previous behaviour. So, it is ok if the behaviour is left as is but maybe the documentation could be improved to stress that in case of other exceptions occur, any further mails in the queue will *not* be processed until the reason for the exception will be resolved. As said, we recently had such a case: due to invalid user input Django refused to generate the email message even before sending to the mail server. Since such errors will never recover, it leads to repeating the attempt to send those mail from the queue, will break, the loop to process the queue will stop and the queue will grow and grow. After thinking more about this, maybe it is enough to just .defer() all messages in the error handler, regardless of the exception to have them retried but only after "retry_deferred" has been called. So in the meantime subsequent "send_mail" invocations can process all other mails in the queue. This is less invasive to the current behaviour, I think.