Skip to content

fix: utils non-null guards + throttler max depth#607

Merged
realfishsam merged 1 commit into
mainfrom
fix/269-296-329-utils
May 24, 2026
Merged

fix: utils non-null guards + throttler max depth#607
realfishsam merged 1 commit into
mainfrom
fix/269-296-329-utils

Conversation

@realfishsam
Copy link
Copy Markdown
Contributor

Fixes #269
Fixes #296
Fixes #329

@realfishsam
Copy link
Copy Markdown
Contributor Author

PR Review: PASS (NOT VERIFIED)

What This Does

Three files changed:

  1. watcher.ts: Replaces if (!map.has(key)) map.set(key, []); map.get(key)!.push(...) with const list = map.get(key) ?? []; list.push(...); map.set(key, list). Eliminates two non-null assertions.

  2. market-utils.ts: Replaces market.yes!.label = market.title (mutation + non-null assertion) with market.yes = { ...market.yes, label: market.title } (spread-copy). Same for market.no. Also tightens the guard from market.title && yesLabel === 'yes' to market.title && market.yes && yesLabel === 'yes'.

  3. throttler.ts: Adds a maxQueueDepth config option (default 1000). When the queue exceeds this depth, the oldest entry is dropped (shifted and resolved). This prevents unbounded memory growth if producers enqueue faster than the token bucket drains.

Blast Radius

  • watcher.ts: Internal resolver-push pattern, no behavioral change.
  • market-utils.ts: addBinaryOutcomes is called from 15+ normalizers across all exchanges. The spread-copy is semantically different: the old code mutated the outcome object's label in-place, meaning market.outcomes[0].label would also change (since market.yes points to the same object). The new code creates a new outcome object for market.yes/market.no, so market.outcomes[0] retains its original label while market.yes gets the new one. This is a behavioral change.
  • throttler.ts: Used for rate limiting across exchanges. The queue-drop behavior changes how back-pressure works.

Findings

  • market-utils.ts behavioral change is significant. After this PR, market.yes.label and market.outcomes[0].label can diverge when the title-promotion logic fires. Previously they were the same object reference. Consumers that read market.outcomes[0].label instead of market.yes.label will see different values. This could affect matching, display, or serialization. Needs verification that no downstream code relies on outcomes-array labels being updated by addBinaryOutcomes.
  • throttler.ts queue-drop silently resolves the dropped request. The oldest queued request's resolve() is called, meaning the caller proceeds as if throttled successfully. This could cause a brief rate-limit burst if the caller then immediately fires a request. Consider whether reject() with a descriptive error would be safer, so callers know they were dropped rather than approved.
  • watcher.ts is clean. The ?? [] pattern is equivalent and avoids the assertion.

Semver Impact

minor -- the maxQueueDepth config addition is additive, but the market-utils.ts behavioral change around outcome object identity could be breaking for consumers that rely on reference equality between market.yes and market.outcomes[0].

Risk

Medium-High. The market-utils.ts change silently alters object identity semantics in a function used by every exchange normalizer. Needs integration testing to confirm no downstream breakage.

@realfishsam realfishsam merged commit 5cf3c01 into main May 24, 2026
8 of 11 checks passed
@realfishsam realfishsam deleted the fix/269-296-329-utils 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