Skip to content

fix(exchange): harden Trading212 order watcher and fee mapping#1184

Open
bennycode wants to merge 2 commits into
test/trading212-and-broker-contractfrom
fix/trading212-watcher-hardening
Open

fix(exchange): harden Trading212 order watcher and fee mapping#1184
bennycode wants to merge 2 commits into
test/trading212-and-broker-contractfrom
fix/trading212-watcher-hardening

Conversation

@bennycode

Copy link
Copy Markdown
Owner

Stacked on #1181; merge that first, GitHub will retarget this to main.

Summary

Fixes four latent Trading212 bugs documented during the #1181 test-suite work (packages/exchange/src/broker/trading212/).

1. Unhandled 'error' emission can crash the process

Scenario: watchOrders' polling tick emits 'error' on the broker EventEmitter when a poll fails. 'error' is Node's reserved event — emitting it with zero listeners throws synchronously, so the first transient API failure inside the timer callback kills the whole process (which also hosts other sessions and the Telegram bot).

Fix: The tick emits 'error' only when listenerCount('error') > 0 and falls back to a console.warn otherwise. Polling always survives to the next tick. Listener-attached behavior is unchanged (backward compatible).

2. Empty-baseline full-history paging

Scenario: The baseline cursor (lastSeenId) was seeded from FILLED orders only. On an account whose history holds no fill yet, it stayed at 0, so every poll tick paged the ENTIRE order history — compounded by Trading212's 1 req/60s rate limit on /equity/history/orders.

Fix: The cursor is seeded from the newest order of ANY status — it exists to bound paging, not to mark fills. Ticks also advance it past non-FILLED entries (cancellations/rejections) so they aren't re-scanned forever. No-replay guarantee preserved: everything in the baseline snapshot predates watchOrders and is never emitted.

3. cancel/list asymmetry wipes manual protective stops

Scenario: getOpenOrders deliberately filters out STOP/STOP_LIMIT/VALUE orders (they don't fit the neutral LIMIT/MARKET model), but cancelOpenOrders cancelled them anyway — a session's cancel-before-place silently wiped a user's manually placed stop-loss it never showed.

Fix: Both methods now share one predicate (#fitsNeutralOrderModel) with the invariant documented at its definition and referenced at the cancel site: a session must never cancel an order it cannot list.

4. Fee summation uses Math.abs on tax lines

Scenario: The fill mapper summed Math.abs of all walletImpact.taxes[].quantity lines. Tax lines are signed (debits negative, credits/rebates positive), so a rebate line would inflate the reported fee instead of reducing it.

Fix: Lines are netted with big.js (matching the surrounding arithmetic). Since neutral Fill.fee is a cost that downstream P&L accumulates additively (totalFees.plus(fill.fee) in BacktestExecutor), the rare net credit is clamped to zero rather than reported as a negative fee — documented at the site.

Test plan

  • Updated the test(exchange): cover the Trading212 stack and add a cross-broker contract suite #1181 suites that pinned old behavior and added regression tests (fake-timer pattern for the watcher):
    • watcher survives a poll failure with no 'error' listener attached and keeps polling (console.warn fallback asserted)
    • fill-free baseline (CANCELLED-only history) stops after the first page instead of following nextPagePath; later fills still emitted
    • cursor advances past non-FILLED orders seen while polling
    • cancelOpenOrders spares STOP/STOP_LIMIT/VALUE orders and still cancels neutral-model orders of the requested ticker
    • mapper nets debit tax lines, subtracts rebates, clamps a net credit to '0'
  • npm test in packages/exchange: 12 files, 153 tests passed
  • npm run lint in packages/exchange: 0 errors
  • npm run dist (tsc) passes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the Trading212 broker integration in packages/exchange by fixing several latent correctness and reliability issues discovered during the #1181 test-suite effort, focusing on order watching stability, cursor paging behavior, order-cancel safety, and fee/tax aggregation correctness.

Changes:

  • Prevents watchOrders from crashing the process by guarding 'error' emissions and falling back to console.warn when no listeners exist.
  • Fixes watchOrders paging/cursor behavior so fill-free or cancellation-heavy histories don’t trigger full-history re-scans and re-paging.
  • Corrects filled-order fee mapping by netting signed tax lines (subtracting rebates) and clamping rare net credits to zero, with regression tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
packages/exchange/src/broker/trading212/Trading212BrokerMapper.ts Net signed tax lines using big.js and clamp net credits to zero for neutral Fill.fee.
packages/exchange/src/broker/trading212/Trading212BrokerMapper.test.ts Adds/updates tests asserting rebate subtraction and net-credit clamping behavior.
packages/exchange/src/broker/trading212/Trading212Broker.ts Hardens watchOrders error emission, improves cursor seeding/advancement, and aligns cancel filtering with list visibility via a shared predicate.
packages/exchange/src/broker/trading212/Trading212Broker.test.ts Adds regression tests for cursor seeding/advancement, cancel/list symmetry, and no-listener 'error' handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants