Rate-limit keystroke events to the backend

[jdeanwallace]

Resolves https://github.com/tiny-pilot/tinypilot/issues/1026

This PR rate-limits the speed at which the fontend sends keystroke WebSocket messages to the backend (currently at a fixed rate of 20 messages per second). This change is necessary to avoid clogging up SocketIO's message queue with custom keystroke messages that previously starved the server of PONG messages (needed to keep the WebSocket connection alive) before the ping_timeout (i.e., 5s) was reached.

This change seems to fix the large paste bug without the client being disconnected 🙌 . As for the fast typers, I doubt anyone can (comfortably) type at 20 characters per second so no one should even notice this change.

Notes

• One interesting side effect is that the browser seems to rate limit setInterval events at one per minute when the browser tab is inactive. So if someone does paste a large amount of text into TinyPilot and continues in another tab, the keystokes only get sent to the backend at 1/min.
  • To avoid browser rate-limiting, someone suggests using web workers
    • https://stackoverflow.com/a/6032591/3769045

• Testing steps: Paste a large amount of text (>2k characters) onto the target machine via the TinyPilot web-UI:

  1. Install this branch's bundle onto a device
  2. On your machine, copy a large amount of text (pick a chapter from Moby Dick)
  3. In your browser, navigate to the TinyPilot web-UI
  4. On the target machine, open a text editor
  5. In the TinyPilot Web-UI, click "Actions" > "Paste"
  6. On your keyboard, press cmd-v or ctrl-v

  All the text should be pasted, uninterrupted (i.e., without the client being disconnected from the server).

[jdeanwallace]

@mtlynch - Can you give me a early review on this PR, before I continue with proper documentation/docstrings?

[mtlynch]

Yep, this looks like the right direction to me. Glad to hear it's working!

[jdeanwallace]

Automated comment from CodeApprove ➜

⏳ @mtlynch please review this Pull Request

[mtlynch]

@jotaen4tinypilot - Do you have time for a first-round review before your vacation?

[jotaen4tinypilot]

Do you have time for a first-round review before your vacation?

I might not manage to do a “proper”, thorough review, but I can take a look later today and leave some feedback, if that’s any helpful.

[mtlynch]

@jotaen4tinypilot - Sure, that would be helpful. The highest value stuff would be to focus on JS-specific stuff, as you can review that stuff better than I can. I can cover stuff like naming and code comments after you do a first pass.

[jdeanwallace]

My other thought is that I’m wondering whether a client-side fix is the optimal approach for this problem. Would it e.g. still be an option for us to transmit the keystrokes via an HTTP call, and to implement a server-side mechanism that would prevent the socket endpoint from freezing?

We tried an implementation of that, but it causes the same problem. Flask's main thread gets stuck processing the long set of keystrokes, which prevents Flask from doing the WebSockets PING/PONG sequence.

But actually, one thing we didn't try is executing the paste async on the backend (e.g, in start_background_task). We lose the result of each keystroke, but we don't strictly need that for paste.

@mtlynch - In a previous comment, you showed some concern about running into this lockup issue when a user types too quickly:

We're only fixing the lockup issue in the case of paste, but couldn't the same thing happen if the user types too quickly?

Do we still consider that an issue or can we assume that fast continual typing by a user is unlikely to trigger a lockup?

If we can ignore fast typing, then maybe we can revisit the async paste idea?

[mtlynch]

We're only fixing the lockup issue in the case of paste, but couldn't the same thing happen if the user types too quickly?

Do we still consider that an issue or can we assume that fast continual typing by a user is unlikely to trigger a lockup?

If we can ignore fast typing, then maybe we can revisit the async paste idea?

No, that's not a big deal to fix. Nobody's reported it so far, so it's unlikely that anyone is typing faster than the backend can consume keystrokes.

We should try the async paste in a background thread since we're pretty close to having an implementation. If that doesn't work how we expect, we can still explore smarter queueing from the client.