Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

Send a special WebSocket close code for clients that ping too frequently #103

Closed
3 tasks
ghost opened this issue Jun 21, 2015 · 6 comments · Fixed by #130
Closed
3 tasks

Send a special WebSocket close code for clients that ping too frequently #103

ghost opened this issue Jun 21, 2015 · 6 comments · Fixed by #130
Assignees
Labels
Milestone

Comments

@ghost
Copy link

ghost commented Jun 21, 2015

For context: https://bugzilla.mozilla.org/show_bug.cgi?id=1152264

In #78, we introduced an adaptive response delay for clients that ping too frequently. Unfortunately, folks are still reporting high battery and data usage, even with the fix in place. This is caused by a bug in the client's adaptive ping logic, and affects all FxOS 2.x releases (1.x is unaffected because it didn't ship with adaptive pings).

Any client can potentially enter this state (especially those on unreliable networks), and there's no recovery apart from manually resetting the prefs. The client patch is in place, but has not yet been uplifted.

On our end, we can detect when clients enter a ping loop, and send a special WebSocket close code (4774). This is normally used for UDP wake-up: if the client detects this code, it won't reconnect, as it expects the server to wake it up for incoming notifications.

The trade-off is that phones on non-TEF networks won't receive any push notifications until their network status changes—either they lose reception and reconnect, or their phone switches between cellular and Wi-Fi. (TEF has their own UDP wake-up platform, so we can actually make this work for them). But it's a small price to pay for battery life and reasonable data usage.

A vague plan:

  • Remove the adaptive response delay, and disconnect clients that ping too frequently.
  • On disconnect, store the connection lifespan in DynamoDB. I think this calls for a weighted moving average, to minimize the impact on well-behaved clients that happen to be on spotty networks.
  • When the client reconnects, look up its average connection lifespan. If it drops below a threshold (15 seconds?), flush any pending messages, then close its connection via self.sendClose(code=4774).
@ghost ghost added the enhancement label Jun 21, 2015
@bbangert
Copy link
Member

Only thing I'd change, don't drop adaptive response delay, since that may still help some clients. But do check the user-agent, and send that special close code to known buggy clients.

@ghost
Copy link
Author

ghost commented Jun 29, 2015

Only thing I'd change, don't drop adaptive response delay, since that may still help some clients.

👍

But do check the user-agent, and send that special close code to known buggy clients.

Do you mean the device ID, or User-Agent header?

@bbangert
Copy link
Member

The User-Agent header, afaik we know the possible values for FxOS to only send them this close code.

@ghost
Copy link
Author

ghost commented Jun 29, 2015

Sounds good. It's a bit unnerving that "the possible values for FxOS to only send them this close code" means "everything released in the past year, including the current one." 😉 But looks like the client patch has been uplifted. Now, how many phones will receive that update...

@bbangert
Copy link
Member

Well yea..... there is that. 😬 But I mean to avoid sending it to new unreleased clients.... or desktop, TV, etc. until we know they need it.

@ghost ghost added bug and removed enhancement labels Aug 3, 2015
@bbangert bbangert added this to the 1.4.0 milestone Aug 6, 2015
@bbangert bbangert self-assigned this Aug 6, 2015
@ghost
Copy link
Author

ghost commented Aug 11, 2015

Remove the adaptive response delay, and disconnect clients that ping too frequently. [...] I think this calls for a weighted moving average, to minimize the impact on well-behaved clients that happen to be on spotty networks.

Actually, this is silly. I don't know why I suggested reinstating the disconnect, or storing the connection lifespan, since the problem is specifically with pings. Let's just keep the connection open, and send the close code if we detect a consistently low ping interval.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant