Closed Bug 1137843 Opened 10 years ago Closed 10 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+
Iteration: 39.2 - 23 Mar → 39.3 - 30 Mar
Target Milestone: --- → mozilla39
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: