Closed
Bug 1076650
Opened 10 years ago
Closed 10 years ago
Re-registration doesn't happen for guests after a 401
Categories
(Hello (Loop) :: Client, defect, P1)
Hello (Loop)
Client
Tracking
(firefox35- verified, firefox36- verified, firefox37 verified)
backlog | Fx35+ |
People
(Reporter: MattN, Assigned: pkerr)
References
Details
Attachments
(1 file, 3 obsolete files)
5.62 KB,
patch
|
mikedeboer
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
STR: 1) Open the loop panel to ensure you are registered. 2) Copy a call URL. 3) Modify the pref loop.hawk-session-token in about:config e.g. backspace a character - WARNING: All URLs you have previously created and shared will no longer work 4) Open the panel again. Expected result: A new call URL should be generated. A new value of loop.hawk-session-token should be generated because a new registration should occur. Actual result: The call URL box is empty. I'm pretty sure this was working around a week ago.
Flags: qe-verify+
Flags: firefox-backlog+
Reporter | ||
Comment 1•10 years ago
|
||
This wasn't working even at 2014-08-01 so I may have misremembered.
Keywords: regression
Reporter | ||
Comment 2•10 years ago
|
||
I think this will be easier to fix after my patch in bug 1047164.
Summary: Re-registration doesn't happen for guests after a 401 anymore → Re-registration doesn't happen for guests after a 401
Reporter | ||
Comment 3•10 years ago
|
||
Although supporting re-registration for a guest session while keeping the same push URL and FxA session will require some work as we currently get a new push URL when doing the guest registration.
Comment 4•10 years ago
|
||
As far as I know, we've only ever handled automatic recovery from these failures on registration. The Loop server keeps guest sessions for 60 days, and refreshes them whenever there's activity. Hence we haven't really needed this until now.
Updated•10 years ago
|
Priority: -- → P2
Updated•10 years ago
|
backlog: --- → Fx35?
Updated•10 years ago
|
Whiteboard: [fxa]
Reporter | ||
Comment 5•10 years ago
|
||
This isn't about FxA. Note that the summary says "guest".
Whiteboard: [fxa]
Updated•10 years ago
|
backlog: Fx35? → Fx35+
Comment 6•10 years ago
|
||
(Paul is working on standalone translations currently.)
Assignee: nobody → pkerr
Reporter | ||
Comment 7•10 years ago
|
||
I think this mostly requires something like calling |this.deferredRegistrations.delete(sessionType);| after a 401 but figuring out where to do that may be tricky to avoid introducing new bugs.
Comment 8•10 years ago
|
||
[Tracking Requested - why for this release]: We believe this will improve our service reliability
tracking-firefox35:
--- → ?
tracking-firefox36:
--- → ?
Comment 9•10 years ago
|
||
Hi Paul, Is this a bug you're able to do this week so we can uplift into Beta Monday for testing?
Flags: needinfo?(pkerr)
Priority: P2 → P1
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 11•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8535000 -
Flags: review?(MattN+bmo)
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8535000 [details] [diff] [review] retry registration on a 401 response Will take a review from the first person available.
Attachment #8535000 -
Flags: review?(dmose)
Assignee | ||
Comment 13•10 years ago
|
||
Reporter | ||
Comment 14•10 years ago
|
||
Comment on attachment 8535000 [details] [diff] [review] retry registration on a 401 response Review of attachment 8535000 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/loop/MozLoopService.jsm @@ +436,5 @@ > * or is rejected with an error. If the server response can be parsed > * as JSON and contains an 'error' property, the promise will be > * rejected with this JSON-parsed response. > */ > + hawkRequestInternal: function(sessionType, path, method, payloadObj, retryOn401 = true) { You should update the JSDoc comment @@ +478,4 @@ > this.clearSessionToken(sessionType); > + if (retryOn401 && error.errno && error.errno === INVALID_AUTH_TOKEN) { > + log.info("401 and INVALID_AUTH_TOKEN - retry registration"); > + return this.registerWithLoopServer(sessionType, false).then( I don't think you want to change the behaviour for FXA sessions as they require user-interaction to login again
Attachment #8535000 -
Flags: review?(MattN+bmo) → review-
Assignee | ||
Updated•10 years ago
|
Attachment #8535000 -
Flags: review?(dmose)
Assignee | ||
Updated•10 years ago
|
Attachment #8537455 -
Attachment is obsolete: true
Assignee | ||
Comment 15•10 years ago
|
||
Do you recall if the retry was managed in MozLoopService.jsm or was it triggered back from the content side using the error retry attribute?
Flags: needinfo?(MattN+bmo)
Assignee | ||
Comment 17•10 years ago
|
||
retry only if in a guest session
Assignee | ||
Updated•10 years ago
|
Attachment #8535000 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8538523 -
Flags: review?(MattN+bmo)
Assignee | ||
Comment 18•10 years ago
|
||
update comment header
Assignee | ||
Updated•10 years ago
|
Attachment #8538523 -
Attachment is obsolete: true
Attachment #8538523 -
Flags: review?(MattN+bmo)
Assignee | ||
Updated•10 years ago
|
Attachment #8538531 -
Flags: review?(MattN+bmo)
Comment 19•10 years ago
|
||
Comment on attachment 8538531 [details] [diff] [review] retry registration on a 401 response Mark -- Matt's pretty busy with the UI Tour stuff and we really want to get his landed and ideally uplifted. Can you review this? (I'll take a review from whoever gets to it first.) Thanks.
Attachment #8538531 -
Flags: review?(standard8)
Updated•10 years ago
|
Attachment #8538531 -
Flags: review?(standard8)
Attachment #8538531 -
Flags: review?(MattN+bmo)
Attachment #8538531 -
Flags: review+
Updated•10 years ago
|
Flags: needinfo?(MattN+bmo)
Comment 20•10 years ago
|
||
I pushed this so that it gets into the tree a bit earlier: https://hg.mozilla.org/integration/fx-team/rev/0f145e2cc93d
Target Milestone: --- → mozilla37
Comment 21•10 years ago
|
||
Comment on attachment 8538531 [details] [diff] [review] retry registration on a 401 response Approval Request Comment [Feature/regressing bug #]: Loop [User impact if declined]: If the server ever returns a 401, the user will be unable to use loop until they restart the browser (or perhaps login/out to fxa). [Describe test coverage new/current, TBPL]: Will be on m-c shortly. 401's from the server are not tested in automation; it has to be done by hand with a mock server. [Risks and why]: Moderately low. Leaving users in a broken state is bad (and totally confusing to them). No uplift landing until manual testing of the server error has been done. [String/UUID change made/needed]: none
Attachment #8538531 -
Flags: approval-mozilla-beta?
Attachment #8538531 -
Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/0f145e2cc93d
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Attachment #8538531 -
Flags: approval-mozilla-beta?
Attachment #8538531 -
Flags: approval-mozilla-beta+
Attachment #8538531 -
Flags: approval-mozilla-aurora?
Attachment #8538531 -
Flags: approval-mozilla-aurora+
Updated•10 years ago
|
Iteration: --- → 37.3
Updated•10 years ago
|
Updated•10 years ago
|
Updated•9 years ago
|
Iteration: 37.3 → 37.2
Comment 25•9 years ago
|
||
(In reply to Matthew N. [:MattN] (away until Jan. 7) from comment #0) > Expected result: > A new call URL should be generated. A new value of loop.hawk-session-token > should be generated because a new registration should occur. What should happen with rooms enabled? loop.hawk-session-token's invalid value is changing both when a new room is created and just clicking an old room in the list.
Flags: needinfo?(pkerr)
Assignee | ||
Comment 26•9 years ago
|
||
This is a calls-only bug. It fixes a regression that would be visible on older non-rooms clients.
Flags: needinfo?(pkerr)
Comment 27•9 years ago
|
||
Ok, thanks. Verified fixed with loop.rooms.enabled=false on FF 35 RC, 36.0a2 (2015-01-06), 37.0a1 (2015-01-06) Win 7.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•