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

Status: RESOLVED FIXED

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

Description

[Adam Roach [:abr]]

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.

Comment 1

[Mark Banner (:standard8)]

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

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).

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

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?

Comment 3

[Mark Banner (:standard8)]

https://hg.mozilla.org/integration/fx-team/rev/d9241a4e3c5c

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d9241a4e3c5c