Closed Bug 1148273 Opened 5 years ago Closed 5 years ago

Readinglist scheduler doesn't handle FxA error states correctly.

Categories

(Firefox Graveyard :: Reading List, defect)

defect
Not set

Tracking

(firefox38+ verified, firefox39+ verified)

VERIFIED FIXED
Firefox 39
Tracking Status
firefox38 + verified
firefox39 + verified

People

(Reporter: markh, Assigned: markh)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

2 problems:
* If there is no FxA account, or the account is not verified, the scheduler reports this as a "normal" error.

* If there is an FxA authentication issue (eg, password changed on other device) then the scheduler doesn't correctly report it as such.

and for an added bonus, this patch also ensures the scheduler logs are captured correctly.

This patch fixes both.  The fix for the first is more of a work-around as the comments mention, but I think it is a less risky fix than the "correct" one at this point in the cycle.

Requesting review from Drew, but I'm going to try and find a sucker^h^h^h^h^h^h kind person to steal it in the meantime.

For QA:
* Ensure that when you disconnect your FxA account from the browser, you don't see error-readinglist-*.txt files start to appear in about:sync-log.

* Ensure that if you change your FxA password, you *do* see such logs appear and at the end up the log you see something like:
> 1427426644400	FirefoxAccounts	ERROR	FxA rejecting with error AUTH_ERROR, details: {"code":401,"errno":110,"error":"Unauthorized","message":"Invalid authentication token in request signature","info":"https://github.com/mozilla/fxa-auth-server/blob/master/docs/api.md#response-format","log":[]}
> 1427426644402	readinglist.scheduler	ERROR	Sync failed, now in state authentication error: Error: AUTH_ERROR (resource://gre/modules/FxAccounts.jsm:1000:17) JS Stack trace: ...

and that the regular Sync "You need to sign-in to continue syncing" error bar appears, and that when you do re-sign in the reading-list continues syncing as normal.

Note for the last step it might be necessary to create a boolean preference "services.sync.debug.ignoreCachedAuthCredentials" with value true to see the error state appear in a timely fashion.
Attachment #8584265 - Flags: review?(adw)
Flags: qe-verify+
Flags: firefox-backlog+
[Tracking Requested - why for this release]:

Account errors in reading list aren't handled correctly.  I expect this to land soon, but requesting tracking as we should make sure it makes it.
One additional tweak - once the user re-logs on (or when a new user initially logs in) we sync "now" rather than waiting for the regular schedule.
Attachment #8584265 - Attachment is obsolete: true
Attachment #8584265 - Flags: review?(adw)
Attachment #8584283 - Flags: review?(adw)
Depends on: 1148301
Comment on attachment 8584283 [details] [diff] [review]
0007-Bug-1148273-have-the-readinglist-scheduler-handle-Fx.patch

This looks like it makes things no worse, so r+. Drew, please feel free to do a follow-up pass, see if I missed anything.
Attachment #8584283 - Flags: review?(adw)
Attachment #8584283 - Flags: review+
FTR, this patch works fine on 38 but doesn't have the desired effect on 39 due to bug 1148301 (and it works on 39 with that applied).  Thanks to rnewman, I'm going to go ahead and land this - not having bug 1148301 doesn't make the status quo any worse on 39.

https://hg.mozilla.org/integration/fx-team/rev/bf843cab375a
https://hg.mozilla.org/releases/mozilla-aurora/rev/d8ffff967a4d
https://hg.mozilla.org/mozilla-central/rev/bf843cab375a
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Iteration: --- → 40.1 - 13 Apr
Iteration: 40.1 - 13 Apr → 39.3 - 30 Mar
Blocks: 1132074
Attachment #8584283 - Flags: review?(adw) → review+
QA Contact: andrei.vaida
Mark, I don't see any uplift request here?!
Could you at least fill the form for posterity?
Thanks
Flags: needinfo?(mhammond)
(In reply to Sylvestre Ledru [:sylvestre] from comment #6)

Clearing NI as per IRC chat.
Flags: needinfo?(mhammond)
Couldn't reproduce this issue with Nightly from 2015-03-24 & from 2015-03-26 under Windows 7 64-bit, OS X 10.9.5 and Ubuntu 14.04 32-bit: the expected results (as mentioned in comment 0/*For QA) are displayed every single time; any idea if there's another way to reproduce this issue in order to reliably verify the fix?
Flags: needinfo?(mhammond)
Hmm - I'm having trouble now too, but what should work is to go through the account creation process but *do not* verify the user.  You should see the same general symptoms.
Flags: needinfo?(mhammond)
Thanks, Mark!
I was able to reproduce: error-reading-.txt files start to appear in about:sync-log 
Verified as fixed with Fx 38 beta 4 and latest Developer Edition 39.0a2 (2015-04-13) on Windows 7 64 bit, Ubuntu 14.04 32 bit and OS X 10.9.5
- in about:sync-log -> via success-readinglist-.txt file I get :
1429013474390	FirefoxAccounts	ERROR	FxA rejecting with error UNVERIFIED_ACCOUNT, details: undefined
1429013474391	readinglist.scheduler	INFO	Can't sync due to FxA account state UNVERIFIED_ACCOUNT
- "Sync encountered an error while syncing: Incorrect account name or password. Sync will automatically retry this acction" error notification is displayed at the bottom of the page.
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.