Closed
Bug 963696
Opened 11 years ago
Closed 11 years ago
FxAccounts email verification; promise chain fails to handle rejection
Categories
(Firefox :: Sync, defect)
Firefox
Sync
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
Reporter | ||
Comment 1•11 years ago
|
||
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
Reporter | ||
Comment 2•11 years ago
|
||
Hm. Apparently the test_polling_timeout() test in test_accounts.js is not sufficient.
Comment 3•11 years ago
|
||
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.
Updated•11 years ago
|
Whiteboard: [qa+]
Comment 4•11 years ago
|
||
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.
Description
•