Closed
Bug 1097862
Opened 10 years ago
Closed 9 years ago
[rooms] Sometimes exiting and re-joining a room gives "the room is full" message from the server
Categories
(Hello (Loop) :: Client, defect, P1)
Hello (Loop)
Client
Tracking
(firefox35 fixed, firefox36 fixed, firefox37 fixed)
backlog | Fx35+ |
People
(Reporter: standard8, Assigned: standard8)
References
Details
(Whiteboard: [standalone])
Attachments
(1 file)
6.05 KB,
patch
|
mikedeboer
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
We've been playing around with rooms, and have both the standalone & desktop UI hooked up. Sometimes we've been getting "The room is full" from the server on re-joining: { code: 400, errno: 202, error: "The room is full." } Its a bit hard to say what the STR are, except exit and re-join the room lots of times. I've just had this happen, and although I can't confirm the exact content of the messages, I believe the timings where as follows: 1) Initial POST to join room at 18:55:18 2) POST refresh to room at 18:55:37 3) Leave room at 18:55:55 4) Re-join room at 18:56:03 This was using a localhost server with participantTTL of 20 seconds, using revision 325574c46d75b2fb81dc8eb907dd81e6908e1c26 of loop-server.
Comment 1•10 years ago
|
||
When you left the room did you use POST {action: leave} on the room?
Updated•10 years ago
|
Flags: needinfo?(standard8)
Assignee | ||
Comment 2•10 years ago
|
||
(In reply to Rémy Hubscher (:natim) from comment #1) > When you left the room did you use POST {action: leave} on the room? In theory, item 3 - 18:55:55 should have been the POST {action: leave} - unfortunately I couldn't get the data of the messages because the logging I needed for that wasn't turned on at the time. I'll see if I can reproduce it later.
Flags: needinfo?(standard8)
Assignee | ||
Comment 4•10 years ago
|
||
I've not seen this recently, lets call it WFM for now.
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: needinfo?(standard8)
Resolution: --- → WORKSFORME
Comment 5•9 years ago
|
||
This happens when reloading the standalone page while the call is active, without pressing the "Leave" button first. 37.0a1 (2014-12-08) Win 7
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Comment 6•9 years ago
|
||
(In reply to Paul Silaghi, QA [:pauly] from comment #5) > This happens when reloading the standalone page while the call is active, > without pressing the "Leave" button first. > 37.0a1 (2014-12-08) Win 7 I also managed to reproduce this issue: - closing the tab with the call and loading the url in a new tab - crashing the browser and loading again the url on Firefox 35 Beta 2 (20141208150535) using Windows 7 x64.
Assignee | ||
Comment 7•9 years ago
|
||
Crashing the browser is an expected case - there's no chance it could have told the loop server that it was leaving. After about 5 minutes, the user should be able to enter the room again. I've just managed to reproduce the issue with closing the tab. It looks like our requests to the server to leave the room are a two-step async process, which is enough to make "onbeforeunload" not complete. So we need to rethink how we manage onbeforeunload - if that's possible to handle in an async situation.
backlog: --- → Fx35?
Component: Server → Client
QA Contact: anthony.s.hughes
Comment 8•9 years ago
|
||
This is standalone side, so we don't need to uplift this, but we want to fix this soon.
backlog: Fx35? → Fx35+
Whiteboard: [standalone]
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → standard8
Iteration: --- → 37.2
Points: --- → 2
Assignee | ||
Comment 9•9 years ago
|
||
This relies on the patch in bug 1111560 to make the tests pass. In this patch, I've made all "Leave" requests for the standalone to be sync. I'm a bit dubious about this for the leave button, but I think a delay when leaving the room isn't too bad. Additionally, to indicate to the mozLoop api (fake or otherwise) that this is a window close versus a button press would likely unnecessarily complicate the api. I've tested it locally by opening the browser console, joining a room, and then closing the tab - and ensuring the POST still completes successfully (repeated a few times). Before, it was frequently only doing the OPTIONS part of the request - not the full POST as well.
Attachment #8536529 -
Flags: review?(mdeboer)
Comment 10•9 years ago
|
||
Comment on attachment 8536529 [details] [diff] [review] Perform the leave notification to the loop-server in a synchronous fashion to give the notification more change of succeeding. Review of attachment 8536529 [details] [diff] [review]: ----------------------------------------------------------------- Looks good! I think there's no harm in the leave call being sync always, since it's a blocking action anyway. I also think this'll get us much more reliable signalling. Thanks! ::: browser/components/loop/standalone/content/js/standaloneMozLoop.js @@ +133,5 @@ > } > + }, > + async: async, > + success: function(responseData) { > + console.log("done"); please remove this line when landing this patch.
Attachment #8536529 -
Flags: review?(mdeboer) → review+
Updated•9 years ago
|
Status: REOPENED → ASSIGNED
Comment 11•9 years ago
|
||
I think this somewhere between a P1 and P2. Since we have an r+'d patch, I'm calling this a P1.
Priority: -- → P1
Assignee | ||
Comment 12•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/240000a647f1
Target Milestone: --- → mozilla37
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/240000a647f1
Status: ASSIGNED → RESOLVED
Closed: 10 years ago → 9 years ago
Resolution: --- → FIXED
Comment 14•9 years ago
|
||
Paul, can you please do some exploratory testing around this?
status-firefox35:
--- → affected
status-firefox36:
--- → affected
status-firefox37:
--- → fixed
Flags: qe-verify+
Flags: needinfo?(paul.silaghi)
Comment 15•9 years ago
|
||
Comment on attachment 8536529 [details] [diff] [review] Perform the leave notification to the loop-server in a synchronous fashion to give the notification more change of succeeding. Approval Request Comment [Feature/regressing bug #]: N/A [User impact if declined]: merge issues at most (standalone) [Describe test coverage new/current, TBPL]: N/A (standalone only) [Risks and why]: no risk; standalone [String/UUID change made/needed]: none
Attachment #8536529 -
Flags: approval-mozilla-beta?
Attachment #8536529 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Attachment #8536529 -
Flags: approval-mozilla-beta?
Attachment #8536529 -
Flags: approval-mozilla-beta+
Attachment #8536529 -
Flags: approval-mozilla-aurora?
Attachment #8536529 -
Flags: approval-mozilla-aurora+
Comment 16•9 years ago
|
||
Verified fixed on the standalone page in Chrome and Nightly.
Status: RESOLVED → VERIFIED
Flags: needinfo?(paul.silaghi)
Updated•9 years ago
|
Updated•9 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•