TextsHQ / platform-discord

MIT License
1 stars 0 forks source link

completely reimplement resumption and reconnection & remove polling #18

Closed slice closed 9 months ago

slice commented 9 months ago

Remove behavior where we poll the HTTP API when we think we're offline. This is incompatible with PersistentWS's connection lifecycle behavior, and can cause an explosion of reconnection spam that Cloudflare-bans the user.

Implement proper RESUME support. I'm not sure if we were doing this correctly before. Overall, WSClient has become much more behaved. I did some preliminary smoke testing and it responds correctly to having Wi-Fi shut off and the machine going to sleep.

After landing patches that add further customizability to how PersistentWS behaves, we now rely on it to perform low-level WebSocket reconnections whenever appropriate. We track the necessary state to tell it whether to reconnect or not, whether we should RESUME or IDENTIFY, and which gateway URL to use (when resuming, it's different).

To help facilitate debugging, this code introduces a texts.log wrapper that also outputs a timestamp, which is useful when debugging. This should really be integrated into PAS/desktop instead, though.

lmk if i should split out the logging changes, it's a bit noisy i'll admit (and p much unrelated to the meat of this change)

fixes PLT-1068

linear[bot] commented 9 months ago
PLT-1068 Discord: have polling respect ratelimits, or remove altogether

ran into this while my mac was asleep: [2024_01_10-04-26-07-PM.png](https://uploads.linear.app/82933d82-5911-4a8b-83e1-3a835a378825/148429c6-a289-4c91-8ca4-d71f8a43d145/7dc872c4-92c9-45fa-9955-7b587bac7981) 429ing is dangerous when connecting to discord unofficially. this should really be removed or improved in some way. the platform still ended up connecting successfully in the end, but it's hard to tell what happened with a lack of timestamps

slice commented 9 months ago

2024_01_17-02-21-49-AM

making reconnectRealtime a no-op for now since that avoids connection lifecycle conflicts w/ PWS auto retry (not super problematic, but yeah) - i'll test overnight and see how it goes

slice commented 9 months ago

ran into this cryptic error overnight, will investigate before merging

uncaughtException TypeError: First argument must be a valid error code number
    at Sender.close (/Users/slice/src/work/a8c/platform-discord/node_modules/ws/lib/sender.js:161:13)
    at WebSocket.close (/Users/slice/src/work/a8c/platform-discord/node_modules/ws/lib/websocket.js:308:18)
    at PersistentWS.disconnect (/Users/slice/src/work/a8c/platform-discord/node_modules/@textshq/platform-sdk/dist/PersistentWS.js:123:17)
    at WSClient.disconnect (/Users/slice/src/work/a8c/platform-discord/dist/websocket/wsclient.js:120:17)
    at WSClient.reconnect (/Users/slice/src/work/a8c/platform-discord/dist/websocket/wsclient.js:125:14)
    at Timeout.sendHeartbeat (/Users/slice/src/work/a8c/platform-discord/dist/websocket/wsclient.js:301:18)
    at listOnTimeout (node:internal/timers:569:17)
    at process.processTimers (node:internal/timers:512:7)
[2024-01-17 14:43:59.643] [discord/WSClient] connection zombified! tried to send heartbeat, but the last one wasn't acked
[2024-01-17 14:43:59.643] [discord/WSClient] disconnecting with code 1006
uncaughtException TypeError: First argument must be a valid error code number
    at Sender.close (/Users/slice/src/work/a8c/platform-discord/node_modules/ws/lib/sender.js:161:13)
    at WebSocket.close (/Users/slice/src/work/a8c/platform-discord/node_modules/ws/lib/websocket.js:308:18)
    at PersistentWS.disconnect (/Users/slice/src/work/a8c/platform-discord/node_modules/@textshq/platform-sdk/dist/PersistentWS.js:123:17)
    at WSClient.disconnect (/Users/slice/src/work/a8c/platform-discord/dist/websocket/wsclient.js:120:17)
    at WSClient.reconnect (/Users/slice/src/work/a8c/platform-discord/dist/websocket/wsclient.js:125:14)
    at Timeout.sendHeartbeat (/Users/slice/src/work/a8c/platform-discord/dist/websocket/wsclient.js:301:18)
    at listOnTimeout (node:internal/timers:569:17)
    at process.processTimers (node:internal/timers:512:7)
slice commented 9 months ago

merging now, will continually dogfood and feed as we go - p important to ship this asap i think