The user module has error handling for failed emails sent on password changing, but not yet in other cases, so that users may register and get a message that email has been sent when in fact it has not, an issue reported, e.g., here. This patch adds tests for mails sent on user registration, generating appropriate user feedback (e.g., "Unable to send mail", etc.) and logs errors to the watchdog.

CommentFileSizeAuthor
user-mail-error-messages.patch7.2 KBnedjo

Comments

killes@www.drop.org’s picture

killes@www.drop.org commented
Category: task » bug

Looks goog, +1. I also consider the lack of tests a bug.

dries’s picture

dries commented

Maybe we can extend user_mail() to log failed mail() attempts? Like that, all such instances would automatically get logged.

killes@www.drop.org’s picture

killes@www.drop.org commented

user_mail() has no idea of the context in which it is called. So error messages would be vague at best. I think the patch should go in as it is.

nedjo’s picture

nedjo
he/him/his
Primary language English
commented

There are (at least) four types of actions we sometimes want on mail attempts: logging (both success and failure) and a user response message (again, both success and failure). So, even if user_mail() handled logging (e.g., through two additional optional arguments passed in, success and failure messages), we would still have to test for success or failure of the call to know what message to return to the user.

Or else pass in four arguments, with the user message (success or failure) returned. But that seems awkward at best, and also defeats the desirable practice of returning false on failure.

Anyone see an elegant way to handle this programatically? If not, handling each call separately as I've done is probably the way to go.

samo’s picture

samo commented
Status: Needs review » Active

I don't know if this is relevant...

Modules like subscriptions, emailpage, and og all send emails out to users. With a large enough userbase, I can imagine a scenario where you would want to disable emails being sent out to a user after a certain bounce/failure limit is hit (like Mailman does). Logging a failure inside "user_mail" makes sense to me, but I am curious to hear if this is an issue for others.

I see a separate table for bounced/failed emails (time, uid) and then a cron job to disable sending emails to a user after the bounce limit is hit.

nedjo’s picture

nedjo
he/him/his
Primary language English
commented
Status: Active » Needs review

Not sure why this was reset to active. Logging is, I believe, a distinct issue. The problems addressed by this patch are (a) drupal claims mail has been sent when in fact it has not, (b) there is no way to recover a password sent out in a failed email.

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Needs review » Needs work

One hunk fails. I still consider this issue to be important to fix.

Jaza’s picture

Jaza commented
Version: x.y.z » 6.x-dev

I too consider both the lack of error-checking and the lack of error-reporting to be a bug, and one that still needs fixing. Moving to 6.x queue.

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented

Obviously still to be fixed. Critical IMHO.

dpearcefl’s picture

dpearcefl commented

Is this still an issue in current D6?

aspilicious’s picture

aspilicious commented
Status: Needs work » Closed (cannot reproduce)

A few years without any response. Feel free te reopen if needed.