Opened 20 months ago
Last modified 15 months ago
#18926 new enhancement
Provide with the ability to monitor emailing failures
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Future Release |
| Component: | Version: | 3.2.1 | |
| Severity: | minor | Keywords: | has-patch |
| Cc: | joachim.kudish@…, kpayne@… |
Description
Add an action hook in 'wp-mail' function on catching 'phpmailerException' and before returning 'false'.
This way, a plugin may track all email sending failures.
Attachments (7)
Change History (22)
Absolutely, but avoiding redeclaring this huge function for a single hook would be great.
The proposed patch is what was expected, let's just hope it will be get accepted...
- Milestone changed from Awaiting Review to Future Release
No problem here but we should probably pass the exception?
comment:5
soulseekah — 15 months ago
I have been thinking a lot about the issue of getting error messages from wp_mail lately and stumbled upon this ticket today, decided to explore the problem-solution space a bit with a small and mostly obvious article.
http://codeseekah.com/2012/02/27/getting-error-feedback-from-wp_mail/
I'm attaching two diffs that that pass a WP_Error object, instead of the exception as the internal implementation may change in the future. One using a filter, another using an action. Both are not too comfortable to use, but we can probably work something out from there + decide on a better hook name, probably specific to wp_mail , again, in case internal implementation changes.
Probably missing something, ready to discuss further.
pluggable.php.diff looks like the best approach to me. Most of the time, you can't do anything but log the error anyway.
comment:7
in reply to:
↑ description
;
follow-up:
↓ 8
npetetin — 15 months ago
Good concern and good answer. However, passing the phpMailer object allows to give more context and retrieve its attributes (from, to, subject, message, etc.) Could passing a less typed set of these attributes (associative array?) in addition be the best compromise?
Replying to npetetin:
Good concern and good answer. However, passing the phpMailer object allows to give more context and retrieve its attributes (from, to, subject, message, etc.) Could passing a less typed set of these attributes (associative array?) in addition be the best compromise?
The inner implementation may change in the future, PHPMailer could be switched for something completely different. But we could use the third optional $data parameter of the WP_Error to pass along the relevant data, might be useful indeed.
How about putting in all the original $to, $subject, $message, $headers = '', $attachments = array() set of data, that is implementation independent, isn't it? Any disadvantages?
- Cc kpayne@… added
Replying to soulseekah:
How about putting in all the original $to, $subject, $message, $headers = '', $attachments = array() set of data, that is implementation independent, isn't it? Any disadvantages?
I like this, and the pluggable.php.diff approach.
I think we should always log mail errors under WP_DEBUG, too.
comment:10
in reply to:
↑ 9
soulseekah — 15 months ago
Replying to kurtpayne:
I think we should always log mail errors under WP_DEBUG, too.
Might be useful, although WordPress doesn't seem to log errors too much, only a couple of classes (wpdb, simplePIE, POP3, deprecated functions, etc.) contain error log writes; can't see much logging going on in the core itself.
comment:11
in reply to:
↑ 8
npetetin — 15 months ago
Replying to soulseekah:
How about putting in all the original $to, $subject, $message, $headers = '', $attachments = array() set of data, that is implementation independent, isn't it? Any disadvantages?
Looks pretty good to me, best implementation with all relevant data.
+1 for pluggable.php.3.diff.
comment:12
in reply to:
↑ 9
scribu — 15 months ago
Replying to kurtpayne:
I think we should always log mail errors under WP_DEBUG, too.
Plugin territory.
comment:13
nacin — 15 months ago
error_log() is probably plugin territory, but trigger_error() for E_WARNING might not be.
comment:14
follow-up:
↓ 15
scribu — 15 months ago
First of all, the action should be called 'wp_mail_failed', to be consistent with 'wp_loaded' and others.
Most emails are sent during a POST action, which is usually followed by a redirect, so I don't see the point in calling trigger_error().
Even if we were to call trigger_error(), it should be through a callback hooked into 'wp_mail_failed', so that it's easy for plugins to disable.
comment:15
in reply to:
↑ 14
soulseekah — 15 months ago
Replying to scribu:
First of all, the action should be called 'wp_mail_failed', to be consistent with 'wp_loaded' and others.
I second this, good thinking.

Attached patch adds the hook as per the description by @npetetin.
If this patch doesn't get accepted, I should note that wp_mail() is a pluggable function (http://codex.wordpress.org/Pluggable_Functions), meaning you could do this in a plugin by redaclaring the function.