Closed Bug 1137843 Opened 7 years ago Closed 7 years ago

Loop client should not try to leave room that it fails to join

Categories

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

defect
Points:
1

Tracking

(firefox39 fixed)

RESOLVED FIXED
mozilla39
Iteration:
39.3 - 30 Mar
Tracking Status
firefox39 --- fixed

People

(Reporter: abr, Assigned: standard8)

Details

Attachments

(1 file)

In looking at call logs, I've noticed that the standalone client sends an unnecessary "leave" message any time it fails to join a room. Most commonly, this happens in a "room full" condition. I see this pattern in the logs a lot:


        Guest            Loop Server
          |                   |
          |(1) join           |
          |------------------>|
          |                   |
          |(2) 400 errno=202  |
          |<------------------|
          |                   |
          |(3) leave          |
          |------------------>|
          |                   |
          |(4) 401 errno=110  |
          |<------------------|


In this case, message 3 is unnecessary since the user isn't in the room. It also *fails* because the user isn't in the room. In addition to unnecessary traffic, this is causing our HTTP error count to be unnecessarily high.
I got bored waiting for a compile and saw this in my bugmail ;-)

This should be enough to fix it. I'm using an explicit flag rather than state tracking, to avoid the possible race condition where a user chooses to exit a room just after they've entered it (during the period where the xhr hasn't returned yet).
Attachment #8579950 - Flags: review?(dmose)
Assignee: nobody → standard8
Iteration: --- → 39.2 - 23 Mar
Points: --- → 1
Priority: -- → P3
Comment on attachment 8579950 [details] [diff] [review]
Loop client should not try to leave room that it fails to join.

Review of attachment 8579950 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, one nit noted;  r=dmose

::: browser/components/loop/content/shared/js/actions.js
@@ +361,5 @@
>       * XXX: should move to some roomActions module - refs bug 1079284
>       */
>      RoomFailure: Action.define("roomFailure", {
> +      error: Object,
> +      // If failure occurs when the join room is sent to the loop-server.

nit: shouldn't this sentence fragment be in the past tense?
Attachment #8579950 - Flags: review?(dmose) → review+
https://hg.mozilla.org/integration/fx-team/rev/d9241a4e3c5c
Iteration: 39.2 - 23 Mar → 39.3 - 30 Mar
Target Milestone: --- → mozilla39
https://hg.mozilla.org/mozilla-central/rev/d9241a4e3c5c
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.