Skip to content

fix: limitless non-null assertions + resolver leak cleanup#605

Merged
realfishsam merged 1 commit into
mainfrom
fix/257-limitless
May 24, 2026
Merged

fix: limitless non-null assertions + resolver leak cleanup#605
realfishsam merged 1 commit into
mainfrom
fix/257-limitless

Conversation

@realfishsam

Copy link
Copy Markdown
Contributor

Fixes #257
Fixes #290
Fixes #303
Fixes #372

@realfishsam

Copy link
Copy Markdown
Contributor Author

PR Review: PASS (NOT VERIFIED)

What This Does

Three files changed across the Limitless exchange:

  1. client.ts: Replaces three this.orderClient! / this.signer! non-null assertions with explicit if (!x) throw guards in cancelOrder, cancelAllOrders, and getBalance. Error messages are clear and actionable.

  2. normalizer.ts: Replaces two params.start! / params.end! non-null assertions inside their own if (params.start) / if (params.end) blocks with local const bindings. TypeScript narrows correctly when the value is captured before the closure.

  3. websocket.ts: Multiple changes:

Blast Radius

Three files in core/src/exchanges/limitless/. The client and normalizer changes are cosmetic safety improvements. The websocket resolver-leak fix and close-cleanup are behavioral changes that affect long-running connections.

Findings

  • Resolver leak fix is correct. The resolverEntry object is created outside the Promise constructor so it can be referenced by both the wsUpdatePromise and the timeout cleanup. indexOf identity comparison works because it is the same object reference.
  • close() now rejects pending resolvers -- this is good. Without this, callers awaiting watchOrderBook would hang forever after close().
  • Mutation concern in websocket.ts: resolvers.splice(idx, 1) mutates the array in-place. This is acceptable here because the array is owned by the Map and only accessed from this module, but it is worth noting as a deviation from immutable patterns.
  • The buffer.shift() guard (line ~120): After the if (entry) return entry; check, if shift() returned undefined (empty array that passed length > 0 check -- impossible), execution falls through to the rest of the method. This is fine.
  • Not verified because the websocket resolver-leak fix involves a race condition that requires runtime testing to confirm correctness under concurrent watchOrderBook calls. The logic reads correctly, but races are hard to verify statically.

Semver Impact

patch -- bug fix (resolver leak, close cleanup), no API surface change.

Risk

Medium. The resolver-leak fix changes race semantics in watchOrderBook. Incorrect cleanup could cause a resolver to be resolved after removal, or a timeout to fire on a cleaned-up resolver. The code looks correct, but integration testing is recommended.

@realfishsam realfishsam merged commit b608660 into main May 24, 2026
9 of 12 checks passed
@realfishsam realfishsam deleted the fix/257-limitless branch May 24, 2026 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment