Closed Bug 963696 Opened 9 years ago Closed 9 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: 9 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.