Closed Bug 1076650 Opened 5 years ago Closed 5 years ago

Re-registration doesn't happen for guests after a 401

Categories

(Hello (Loop) :: Client, defect, P1)

defect

Tracking

(firefox35- verified, firefox36- verified, firefox37 verified)

VERIFIED FIXED
mozilla37
Iteration:
37.2
Tracking Status
firefox35 - verified
firefox36 - verified
firefox37 --- verified
Blocking Flags:
backlog Fx35+

People

(Reporter: MattN, Assigned: pkerr)

References

Details

Attachments

(1 file, 3 obsolete files)

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+
This wasn't working even at 2014-08-01 so I may have misremembered.
Keywords: regression
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
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.
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.
Priority: -- → P2
backlog: --- → Fx35?
Whiteboard: [fxa]
This isn't about FxA. Note that the summary says "guest".
Whiteboard: [fxa]
backlog: Fx35? → Fx35+
(Paul is working on standalone translations currently.)
Assignee: nobody → pkerr
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.
[Tracking Requested - why for this release]:
We believe this will improve our service reliability
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
Yes.
Flags: needinfo?(pkerr)
Status: NEW → ASSIGNED
Attachment #8535000 - Flags: review?(MattN+bmo)
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)
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-
Attachment #8535000 - Flags: review?(dmose)
Attachment #8537455 - Attachment is obsolete: true
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)
Not sure it is worth tracking.
retry only if in a guest session
Attachment #8535000 - Attachment is obsolete: true
Attachment #8538523 - Flags: review?(MattN+bmo)
update comment header
Attachment #8538523 - Attachment is obsolete: true
Attachment #8538523 - Flags: review?(MattN+bmo)
Attachment #8538531 - Flags: review?(MattN+bmo)
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)
Attachment #8538531 - Flags: review?(standard8)
Attachment #8538531 - Flags: review?(MattN+bmo)
Attachment #8538531 - Flags: review+
Flags: needinfo?(MattN+bmo)
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 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: 5 years ago
Resolution: --- → FIXED
Attachment #8538531 - Flags: approval-mozilla-beta?
Attachment #8538531 - Flags: approval-mozilla-beta+
Attachment #8538531 - Flags: approval-mozilla-aurora?
Attachment #8538531 - Flags: approval-mozilla-aurora+
Iteration: --- → 37.3
Iteration: 37.3 → 37.2
(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)
This is a calls-only bug. It fixes a regression that would be visible on older non-rooms clients.
Flags: needinfo?(pkerr)
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.
You need to log in before you can comment on or make changes to this bug.