Skip to content

fix: add 30s connection timeout to feed WebSockets#606

Merged
realfishsam merged 1 commit into
mainfrom
fix/252-253-feed-timeouts
May 24, 2026
Merged

fix: add 30s connection timeout to feed WebSockets#606
realfishsam merged 1 commit into
mainfrom
fix/252-253-feed-timeouts

Conversation

@realfishsam
Copy link
Copy Markdown
Contributor

Fixes #252
Fixes #253

@realfishsam
Copy link
Copy Markdown
Contributor Author

PR Review: VERIFIED

What This Does

Adds a 30-second connection timeout to the establishConnection() method in both BinanceFeed and ChainlinkFeed. If the WebSocket does not emit open, error, or close within 30s, the timeout fires, closes the socket, nulls out state, and rejects the connection promise.

Blast Radius

Two files: core/src/feeds/binance/binance-feed.ts and core/src/feeds/chainlink/chainlink-feed.ts. Changes are structurally identical. Only the establishConnection() method is affected.

Findings

  • Timeout is cleared on all three settlement paths (open, close, error). No double-settlement risk.
  • ws.close() is used (graceful close) rather than ws.terminate() (hard close). This is appropriate since the connection attempt may have partially completed a TCP handshake.
  • The timeout rejects the promise, which will propagate to ensureConnected() -> connect(). Callers should handle this rejection. Looking at both feeds, connect() catches and calls scheduleReconnect(), so the rejection is handled.
  • The close handler also clears the timeout. This is correct -- if the connection closes before the timeout fires (e.g., server RST), the timer is cleaned up.
  • No settled flag unlike PR fix: opinion fetcher perf + websocket safety #604's Opinion websocket. This is acceptable because ws.close() in the timeout handler will trigger the close event, but by that point the promise is already rejected (the close handler only nulls state and schedules reconnect, it does not resolve/reject). There is no double-rejection risk.

Semver Impact

patch -- bug fix (prevents indefinite hang on feed connection), no API change.

Risk

Low. The pattern is straightforward and mirrors standard WebSocket timeout implementations.

@realfishsam realfishsam merged commit d0496ac into main May 24, 2026
8 of 11 checks passed
@realfishsam realfishsam deleted the fix/252-253-feed-timeouts branch May 24, 2026 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant