Closed Bug 963696 Opened 11 years ago Closed 11 years ago

FxAccounts email verification; promise chain fails to handle rejection

Categories

(Firefox :: Sync, defect)

defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: jedp, Unassigned)

References

Details

(Whiteboard: [qa+])

Gavin reported on the sync-dev list [1]: The verification email took a while to arrive, and so the build I created an account with gave me: A promise chain failed to handle a rejection. Date: Fri Jan 24 2014 10:31:43 GMT-0800 (PST) Full Message: Error: User email verification timed out. Full Stack: pollEmailStatus/<@resource://gre/modules/FxAccounts.jsm:412 Handler.prototype.process@resource://gre/modules/Promise.jsm:767 this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm:531 FxAccounts.jsm:412 [1] https://mail.mozilla.org/pipermail/sync-dev/2014-January/000757.html
For context, this is the line in question from the build gavin would have had: https://hg.mozilla.org/mozilla-central/file/bfe4ed6d47ce/services/fxaccounts/FxAccounts.jsm#l412
Hm. Apparently the test_polling_timeout() test in test_accounts.js is not sufficient.
Hm, the email-polling process is supposed to run for 5 minutes, then stop (silently), and resume whenever the browser is restarted or the UI is provoked. (imagine a worker who gets bored and falls asleep after 5 minutes, but tries to hide it by suddenly doing their job whenever somebody checks up on them). (this is not necessarily the best approach, but polling forever is painful, and so is having Sync not start until you look at it. Some sort of pubsub/long-poll might reduce the costs somewhat, but I think we still need to find a reasonable way to remove the timeout and effectively wait forever for verification to finish) I see two places that drop the Promise, which could then cause this unhandled-rejection log to occur: * FxAccounts.setSignedInUser() drops the Promise returned by internal.startVerifiedCheck() -> internal.whenVerified() . The intention is that setSignedInUser() waits for durability (the credentials have finished being saved to the profile), but not for verification to complete. The about:accounts page (aboutaccounts.js:104) waits for setSignedInUser()'s promise before updating the UI. * BrowserIDManager.initializeWithCurrentIdentity() does some error-handling on the promise returned by _fetchSyncKeyBundle -> _refreshTokenForLoggedInUser -> _fetchTokenForUser -> FxAccounts.whenVerified() , at browserid_identity.js:105, but then it re-throws the error, and the containing then() call doesn't return the Promise, which means errors here will show up as unhandled rejections. I'm not sure if this was intentional or not. But really, the error message is the shallow problem.. the deeper one is whether or not touching the UI correctly causes the poller to restart. I haven't tested that yet. One thought: maybe we could enhance the setSignedInUser() API to return more information than "credentials saved", by adding a second event/response/promise for "verification is complete", and maybe a third for "polling timed out". That might let the about:accounts UI present a better status message.
Whiteboard: [qa+]
I no longer see this - on a timeout there is a log message: 1393572965091 Sync.BrowserIDManager ERROR Background fetch for key bundle failed: User email verification timed out. But no "promise chain fails to handle rejection" message. It could be argued that log message isn't an "error", but shouldn't be seen in general anyway - so let's tackle that in a followup if anyone complains.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.