[DF-21594]update the ws reconnect logic and adding explicit logging#547
[DF-21594]update the ws reconnect logic and adding explicit logging#547
Conversation
|
👋 guandali, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
NPM Publishing labels 🏷️🟢 This PR has valid version labels and will cause a |
src/transports/websocket.ts
Outdated
| lastMessageReceivedAt = 0 | ||
| connectionOpenedAt = 0 | ||
|
|
||
src/transports/websocket.ts
Outdated
| const timeSinceLastMessage = Math.max(0, now - this.lastMessageReceivedAt) | ||
| const timeSinceConnectionOpened = Math.max(0, now - this.connectionOpenedAt) | ||
| // Explicit initial grace (1s) after open; after that, rely on "time since last message" | ||
| const INITIAL_GRACE_MS = 1000 |
src/transports/websocket.ts
Outdated
| await new Promise<void>((resolve) => { | ||
| this.wsConnection!.once('close', () => resolve()) | ||
| try { | ||
| this.wsConnection!.close(1000) |
There was a problem hiding this comment.
Do you want to use INITIAL_GRACE_MS here as well?
There was a problem hiding this comment.
its a standard code for normal closing of ws
|
Need more details on what the changes are |
guandali
changed the title
guandali
force-pushed
the
chore/lli/harden-ws-reconnect
branch
from
4b7f1ce to
d722bcb
Compare
|
@mxiao-cll hey Michael, thank you for the review and i have added more context and address ed your comment |
|
It seems reasonable to me. We should make sure this is tested on Staging before it goes to Production. |
|
🚀 Successfully created version bump PR: #525 |
This reverts commit 6d5d449.
3 participants
related to reported bug here: DF-21594 icap EAs appears hung over the weekend
Slack discussion
Some identified issue:
lastMessageReceivedAtis only updated when there a write to cache (if the source sends in heartbeat, thelastMessageReceivedAt) and that would lead to periodic closure of web socket over the non-market hour (every 120 seconds).closeon ws right away and then start a new ws; if the previous instance is not properly closed, we could end up with two instances of ws for same data source and it is known issue for ICAP/TP has two concurrent connections Context: TP and ICAP EAs use the same credentials, and often there are issues with the set of credentials not having concurrent (ie: 2+) connections enabled.What changed in this PR
lastMessageReceivedAtwhenever receives an update