Issue status update for http://drupal.org/node/20139 Post a follow up: http://drupal.org/project/comments/add/20139 Project: Drupal Version: cvs Component: user.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: nedjo Updated by: killes@www.drop.org -Status: patch (code needs review) +Status: patch (code needs work) One hunk fails. I still consider this issue to be important to fix. killes@www.drop.org Previous comments: ------------------------------------------------------------------------ Thu, 07 Apr 2005 18:12:10 +0000 : nedjo Attachment: http://drupal.org/files/issues/user-mail-error-messages.patch (7.2 KB) 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 [1]. 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. [1] http://drupal.org/node/20026 ------------------------------------------------------------------------ Thu, 07 Apr 2005 18:16:28 +0000 : killes@www.drop.org Looks goog, +1. I also consider the lack of tests a bug. ------------------------------------------------------------------------ Thu, 07 Apr 2005 19:49:15 +0000 : Dries Maybe we can extend user_mail() to log failed mail() attempts? Like that, all such instances would automatically get logged. ------------------------------------------------------------------------ Thu, 07 Apr 2005 20:35:05 +0000 : killes@www.drop.org 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. ------------------------------------------------------------------------ Sat, 09 Apr 2005 14:36:36 +0000 : nedjo 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. ------------------------------------------------------------------------ Sun, 03 Jul 2005 13:40:13 +0000 : samo 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. ------------------------------------------------------------------------ Mon, 04 Jul 2005 20:59:43 +0000 : nedjo 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.