Release: develop -> main#4347
Merged
Merged
Conversation
…ams (#4310) (#4335) * fix(scrypt): catch up state on reconnect and cancel leaked fetch streams (#4310) Completes the receive-path resilience epic: - Leak-fix: fetch/fetchAll opened a server-side stream via `type: subscribe` and never stopped it. They now cancel it by request id (the venue's cancel-by-reqid) after collecting the response — best-effort, never affecting the fetch result or triggering a retry. - Catch-up: a bare re-subscribe is not documented to replay a BalanceTransaction missed during an outage, so after a reconnect the connection fires an onReconnect hook and ScryptService re-fetches balance transactions + execution reports (as the constructor warm-up does), recovering the missed event within the reconnect instead of never. The balance-transaction cache write is terminal-aware, so a fetched non-terminal record can never clobber a fresher completed one. Stacked on the WS reconnect PR; the onReconnect callbacks are fire-and-forget (never awaited in establishConnection) so the catch-up cannot block or delay a reconnect. * fix(scrypt): apply the terminal cache guard universally, cancel on every fetch exit, coalesce catch-ups (#4310) Address review findings on the reconnect catch-up + leak-fix: - The permanent live BalanceTransaction subscriber and the constructor warm-up now write through cacheBalanceTransaction too, so the terminal guard is universal — a stale non-terminal push (e.g. a resubscribe snapshot on reconnect) can no longer clobber a completed withdrawal in the cache that getWithdrawalStatus reads. - fetch/fetchAll cancel their ad-hoc stream in a finally, so a malformed-initial throw, a request timeout, or a mid-pagination failure still stops the server-side stream instead of leaking it on the non-happy path. - catchUpAfterReconnect coalesces via an in-progress flag, so a re-drop during its own fetch cannot spawn a second concurrent catch-up (which could regress an execution report, as those carry no terminal guard). - fireReconnectCallbacks now also swallows async rejections from a callback, not just synchronous throws. * fix(scrypt): coalesce overlapping reconnect catch-ups instead of dropping them (#4310) The catchUpInProgress guard hard-skipped an overlapping reconnect, so on a double reconnect the already-resolved execution-report leg was never re-fetched and executionReports could be left stale for an order whose final transition landed in the second reconnect's downtime (getOrderStatus reads the cache with no staleness check). Replace the skip with a catchUpPending flag: an overlapping trigger makes the in-flight catch-up run once more (do/while), coalescing multiple overlaps into a single rerun that covers their downtime. * fix(scrypt): guard execution reports too, DRY the cache-apply, parallelize catch-up (#4310) pr-ready review: executionReports had no terminal guard while balanceTransactions did, and the reconnect catch-up now writes execution reports — so a live fill could be clobbered by a staler catch-up fetch, regressing an order (possible re-cancel/duplicate). Add isTerminalExecutionReport + cacheExecutionReport and route all three execution-report write sites (warm-up, live subscriber, catch-up) through it, mirroring balanceTransactions. Extract applyExecutionReports/applyBalanceTransactions helpers; run the catch-up's two independent fetches via Promise.allSettled; sendCancel logs at error severity. * test(scrypt): cover reconnect catch-up allSettled isolation and cancel-frame throw path (#4310) pr-ready round 2: add two coverage tests. (1) catchUpAfterReconnect applies a fulfilled stream even when the other stream's fetchAll rejects, and logs the rejection once (Promise.allSettled isolation). (2) fetch still resolves with its collected data when the best-effort CANCEL frame's ws.send throws synchronously, logging the failure. Test-only; no production change. * fix(scrypt): cache live stream updates directly so they are never age-filtered (#4310) pr-ready round 3: the DRY refactor routed the live EXECUTION_REPORT/BALANCE_TRANSACTION subscription callbacks through the same age-bounded helpers as the bulk warm-up/catch-up, so a live BalanceTransaction without a Timestamp (new Date(undefined) >= cutoff is false) was silently dropped. Live callbacks now cache each item directly via cacheExecutionReport/cacheBalanceTransaction (terminal guard, no age cutoff); applyExecutionReports/applyBalanceTransactions stay on the bulk paths only. Pinned by mutation-checked tests plus a bulk-path age-filter regression test. * fix(scrypt): route getOrderStatus fallback through the terminal guard and key-guard balance-tx cache (#4310) pr-ready round 4: getOrderStatus's cache-miss fallback wrote executionReports directly, bypassing the terminal-aware guard the other write paths use — a live terminal push landing during the fetch await could be clobbered by a stale non-terminal snapshot and, since a terminal order emits no further update, persist until restart. Route it through cacheExecutionReport and re-read the cache. Also guard cacheBalanceTransaction's terminal check on a defined ClReqID so it no longer suppresses distinct keyless records (ClReqID is optional). Both pinned by mutation-checked tests.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
July 23, 2026 18:41
Author
ℹ️ New TODOs/FIXMEs (1)+ address: yapealEUR.bic.padEnd(11, 'XXX'), |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist