[wp-trac] [WordPress Trac] #23291: wp_mail should handle phpmailer exceptions instead of ignoring them

[WordPress Trac]

#23291: wp_mail should handle phpmailer exceptions instead of ignoring them
-------------------------+------------------------------
 Reporter:  mark-k       |       Owner:
     Type:  enhancement  |      Status:  new
 Priority:  normal       |   Milestone:  Awaiting Review
Component:  Mail         |     Version:  3.5
 Severity:  minor        |  Resolution:
 Keywords:  has-patch    |
-------------------------+------------------------------

Comment (by rmccue):

 Replying to [comment:19 iandunn]:
 >   a. Because of the structure of the function, though, the
 trigger_error() calls are still necessary. The function only returns false
 | WP_Error if sending fails. There are cases where errors occur and
 sending is still successful. For example, if $to is valid, but there's an
 error in the CC header. If we remove the trigger_error() calls, those
 errors will continue to be ignored and undetectable.

 Can we return the error anyway? The calling code can always check the
 error code and decide what to do with it.

 >   b. $return = 'bool' | 'wp_error' isn't consistent with
 wp_insert_post()'s $wp_error = false | true signature, but it conforms to
 the coding guidelines.

 I personally think it should be a boolean. Which specific guideline are
 you referencing here?

 > 2. Updated the call to wp_mail() in retrieve_password() to use the new
 param. It was the only function in core that was actually checking the
 return value.

 I feel like that should be fixed up where functions are already returning
 `WP_Error`s, but that's probably out-of-scope for this ticket. (I'll defer
 to your judgement here.)

 > 3. Removed the error suppression operator from 4 calls to wp_mail(). It
 doesn't seem like there's a valid case for using it, and it could
 potentially hide errors and make debugging/troubleshooting unnecessarily
 difficult. See [http://stackoverflow.com/questions/136899/suppress-error-
 with-operator-in-php 1] and
 [http://programmers.stackexchange.com/questions/120391/is-error-
 suppression-a-valid-technique-for-testing-for-an-optional-array-key 2].

 We do use the error suppression in a few places, but where possible it
 makes sense to avoid it. This is one of those places, so that looks good.
 :)

 > I don't think there's a way to translate the error messages thrown by
 PHPMailer, since they're in a variable rather than a string. Is that
 correct?

 It looks like PHPMailer reads the strings from a `$language` property (see
 `PHPMailer::SetLanguage`) that we could use (although there are a couple
 of missed strings there that could use an upstream patch). Again, I'd say
 that's out-of-scope for this ticket.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/23291#comment:21>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software