Firefox Accounts has an onerror callback that is invoked when something goes wrong, and we're simply ignoring it. In particular, this callback would be the way to detect that an account is unverified, in which case we may want to notify the user somehow (bug 995353). Since it's still unclear how to present this error condition to the user, I'm marking this as dependent on the (rather broad and meta) bug 966033.

Here is a sketch of what onerror could look like. oncancel can probably be a no-op, but I would recommend defining it: diff --git a/apps/settings/js/findmydevice.js b/apps/settings/js/findmydevice.js index 6059e43..4938685 100644 --- a/apps/settings/js/findmydevice.js +++ b/apps/settings/js/findmydevice.js @@ -20,6 +20,7 @@ var FindMyDevice = { audience: api, onlogin: self._onChangeLoginState.bind(self, true), onlogout: self._onChangeLoginState.bind(self, false), + onerror: self._onFxAError.bind(self), onready: function fmd_fxa_onready() { var loginButton = document.getElementById('findmydevice-login'); loginButton.addEventListener('click', function() { @@ -37,6 +38,31 @@ var FindMyDevice = { checkbox.addEventListener('change', this._onCheckboxChanged.bind(this)); }, + /** + * errorBlob should have two attributes: + * "error": <string> and "details": {}. + * The value of "details" is error-specific. One example: + * {"error":"UNVERIFIED_ACCOUNT", + * "details":{"user": + * {"accountId":"<email_address>", + * "verified":false}}} + */ + _onFxAError: function fmd_on_fxa_error(errorBlob) { + + throw new Error('mid ' + errorBlob); + // throw new Error('obye: ' + JSON.parse(errorBlob)); + if (errorBlob.details && + errorBlob.details.user && + typeof(errorBlob.details.user.verified) !== 'undefined' && + (!errorBlob.details.user.verified)) { + throw new Error('Firefox Account is not verified'); + } + throw new Error(errorBlob.details + ' | ' + + errorBlob.details.user + ' | ' + + (typeof(errorBlob.details.user.verified) !== 'undefined') + ' | ' + + (!errorBlob.details.user.verified)); + }, + _setEnabled: function fmd_set_enabled(value) { var _ = navigator.mozL10n.get;

John, we need UX (for FxOS Client) in order to display this case to the user. Consult ggp for specifics.

So we don't appear to have an error string for this one in our properties list. Just to confirm, the sentiment here is something like: "Error, something's not right with your Firefox Account. Go check that out" Is that correct?

I think so, but I don't know which specific errors may be raised by FxA. You may want to check with Sam if you need that information.

Sam does this sound right?

We want the catch-all error you describe; not that it's my business but I believe we also want a specific error for unverified accounts. I do not see the bug we had for tracking error content, but I think it was assigned to you or Jared. Let me know if you want me to jump on this. The set of error strings you might possibly see is here: http://dxr.mozilla.org/mozilla-central/source/services/fxaccounts/FxAccountsCommon.js#117

Great, so perhaps what we need from John here is to figure out how to map those error conditions into user-visible error strings, right? Of course, we probably don't need one per error, but we may want to provide meaningful strings to some errors, and a generic catch-all for the rest.

After discussing it with Sam, it seems like there's only really two cases here since auth errors get handled by a reauth flow. It's probably easiest to show these errors in the standard prompt. For the unverified case: Title: Error Message: Please verify your Firefox Account to continue. On the web we offer a redirect to resend the email here, but i don't think it's possible to do this on Gaia. Mentioning Firefox Accounts in the error message is minimal but sufficient for the time being. It's my understanding that this is the only actionable error case for onError(). All other errors should throw the same generic fallback message thrown by FxA. This is: Title: Unable to Connect Message: System unavailable, try again soon. This error string can be found on lines 47-48 here: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/fxa/locales/fxa.en-US.properties

tagging with late-l10n so it's understood the above strings will also be used by FindMyDevice gaia client.

Please use gaia approvals for this one. Please help understand user impact?

I can't answer this. This is all Erin and John Gruen.

User impact is as follows For the first string: FMD relies on Firefox Accounts for authentication. In order to enable FMD the user must have a verified Firefox Account. This error message occurs if a user attempts to enable FMD an unverified Firefox Account, and provides the user with instructions about how to complete their intended action. For the second string: There may be rare cases where the Firefox Account server experiences an outage or failure and cannot provide authentication. In these cases, there is no immediate action a user can take to enable/disable FMD. This error message lets the user know that they're in such a state through no fault of their own, and that the situation will be fixed from our end shortly.

As commented in a different bug: is this bug going to add new strings or reuse existing fxa strings for Find My Device? From comment 8 we seem to be missing an error message (unverified account).

Lets uplift this when its ready which should be before June 20 as this has string changes. Overall this is nice-to-have but not blocking the release.

Hey Nikhil, I have some cycles today, happy to take this if you haven't already got it sorted. If you'd like to hand it off, just assign to me.

Thanks! All yours.

(In reply to Francesco Lodolo [:flod] from comment #13) > As commented in a different bug: is this bug going to add new strings or > reuse existing fxa strings for Find My Device? > > From comment 8 we seem to be missing an error message (unverified account). Yes, that's right: two of the error strings are reused from FxA, but two are new.

Hey Jared, is there anyway to solve this case bug 1037232 with strings we've already used (table the new strings for now?).

Hey Erin - I'll take a look Monday, we can discuss in bug 1037232.

Triage discussed - not a regression, we shipped 2.0 with this. Would be nice to have though, so once a patch has landed and settled on master, ask for patch approval to land.

[Blocking Requested - why for this release]: per bug 1150823 comment 14 this is a dupe of a 2.5+

Blocks other bugs, blocking for 2.5

So indeed, bug 1021428 introduced an onerror handler. This bug is just an old dupe.