Skip to content
This repository has been archived by the owner. It is now read-only.

WIP feat(email): Add an explicit error code for when email delivery fails. #1329

Closed
wants to merge 1 commit into from

Conversation

@rfk
Copy link
Member

@rfk rfk commented Jul 6, 2016

@jrgm in the morning meeting today, you mentioned a long-standing issue with the auth-server where we eat email delivery failures rather than returning an error. I couldn't find the original issue, but is this along the lines of the fix you'd hoped to see?

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Jul 18, 2016

Will errno 127 be surfaced to the content server? We have 1018 right now but that is synthesized and kinda gross. Would be nice to have the auth server just tell us.

@rfk
Copy link
Member Author

@rfk rfk commented Jul 18, 2016

Right, the idea would be that this show up to the user as an explicit "we failed to send email" error message.

@seanmonstar
Copy link
Member

@seanmonstar seanmonstar commented Nov 15, 2016

This is quite stale. The idea seems like a good one, but it'd be nice to decide one way or the other, since it's been open for almost half a year.

@rfk
Copy link
Member Author

@rfk rfk commented Nov 15, 2016

Indeed, and I still think this is worth pursuing. Any objections to the approach proposed here, with new error code and detection of errors from the mail delivery agent?

})
.catch(function (err) {
// If the email was rejected, delete the account.
if (err.errno !== error.ERRNO.EMAIL_REJECTED) {

This comment has been minimized.

@seanmonstar

seanmonstar Nov 15, 2016
Member

Oh I see, this could be a scary thing to do, considering absolutely any error in the mailer means this errno.

This comment has been minimized.

@rfk

rfk Nov 15, 2016
Author Member

Heh, yeah, the missing piece here is figuring out how to get enough error info out of the mailer, that we know for sure this was a delivery failure rather than some other transient error.

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Jan 9, 2017

cc @jrgm

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Feb 6, 2017

from mtg: todo after fxa-56 doc is merged

@rfk
Copy link
Member Author

@rfk rfk commented Mar 1, 2017

I'm going to close this in favour of #1686, now that the auth-mailer has been merged into this repo.

@rfk rfk closed this Mar 1, 2017
@rfk rfk removed the waffle:next label Mar 1, 2017
@philbooth philbooth deleted the email-rejected-errno branch Aug 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants