Repository navigation
fix: prevent worker crashes after WebSocket connection timeouts - #1346
Conversation
commit: |
✅ Testplane E2E run succeed
|
✅ Testplane browser-env run succeed
|
1d9693e to
f5b521c
Compare
|
|
||
| const isClosing = status === WsConnectionStatus.CLOSED; | ||
|
|
||
| this._wsConnectionStatus = status; |
There was a problem hiding this comment.
codex rightfully complains that when forceReconnect is called multiple times while a connection is still opening, previous connections aren't cancelled and only newest attempt will be tracked. Previous ones will be left hanging.
Before, we had a guard if !ws, but now we don't have it
There was a problem hiding this comment.
yes, you're right. Thanks
DudaGod
changed the title
f5b521c to
41bc36c
Compare
| // Keep a handler until close because the attempt has already removed its listeners. | ||
| const onError = (): void => {}; | ||
|
|
||
| ws.on("error", onError); |
There was a problem hiding this comment.
the main fix for the problem
| } else { | ||
| ws.once("open", () => { | ||
| ws.close(); | ||
| if (ws.readyState === ws.CONNECTING) { |
There was a problem hiding this comment.
Method close() aborts a CONNECTING handshake and emits an error on the next tick.
Keep a handler until close because the attempt has already removed its listeners.
| closeWsConnection(ws); | ||
|
|
||
| const onUnexpectedResponse = async (_ws: WebSocket, res: IncomingMessage): Promise<void> => { | ||
| const reason = await consumeText(res).catch(() => "Unknown reason"); |
There was a problem hiding this comment.
Read the HTTP error before aborting, otherwise cancellation can replace its status and body.
|
|
||
| const isClosing = status === WsConnectionStatus.CLOSED; | ||
|
|
||
| this._wsConnectionStatus = status; |
There was a problem hiding this comment.
yes, you're right. Thanks
What is done
Fixed a worker crash caused by late WebSocket errors after a connection timeout. Previously, the old connection could remain active without an error handler and crash the worker even after a retry succeeded.
Now cancel timed-out connections and handle errors until the socket closes. Also fixed
close()so it cancels an ongoing connection attempt and prevents new attempts afterward.More info in ticket