Skip to content

fix: surface TWS errors on subscription channels (closes #434)#490

Merged
wboayue merged 2 commits into
mainfrom
fix/issue-434-decoder-error-handling
May 3, 2026
Merged

fix: surface TWS errors on subscription channels (closes #434)#490
wboayue merged 2 commits into
mainfrom
fix/issue-434-decoder-error-handling

Conversation

@wboayue
Copy link
Copy Markdown
Owner

@wboayue wboayue commented May 2, 2026

Summary

Closes #434. The original symptom (realtime_bars decoder treating an error as a Bar, producing ParseIntError "invalid digit found in string") was fixed in e1333df but had no regression test. This PR locks that down and audits every other StreamDecoder for the same gap.

  • 15 StreamDecoders now match on IncomingMessages::Error and return Err(Error::Message(code, msg)) instead of producing a parse error, losing the IB error code via Error::Simple("unexpected message: …"), or silently skipping via UnexpectedResponse. Affects: OptionComputation, OptionChain, AccountSummaryResult, PnL, PnLSingle, PositionUpdate, PositionUpdateMulti, AccountUpdate, AccountUpdateMulti, Vec<ScannerData>, NewsBulletin, NewsArticle, WshMetadata, WshEventData, DisplayGroupUpdate.
  • Regression test for realtime_bars Error handling (sync + async) — locks down the prior fix.
  • Sync Subscription log line at subscriptions/sync.rs now branches on Error::Message: TWS-side errors log warn!("subscription terminated by TWS error [code] msg"), genuine decode failures keep error!("error decoding message: ...").
  • Per-decoder unit test added for each newly-handled decoder verifying a wire error decodes to Error::Message(10089, _). One existing test (AccountSummaryResult::test_decode_unexpected_message) updated since its old assertion no longer holds.

Behavior change

Five decoders (OptionChain, NewsBulletin, NewsArticle, WshMetadata, DisplayGroupUpdate) previously fell through to Error::UnexpectedResponse for type-4 messages, which process_decode_result translates to Skip — i.e. the subscription kept running silently after a TWS error. They now terminate with Error::Message(code, msg), surfacing the IB error to the caller. Warning codes (2100-2169) are filtered at the dispatcher (is_warning_error), so only real errors reach these decoders; terminating is the correct UX, but it is a visible behavior shift beyond what #434 literally requested.

The Group B accounts/contracts decoders also see a small side effect: their wildcard _ arm now returns UnexpectedResponse (skip) instead of Error::Simple("unexpected message: …") (terminate). Unrelated message types routed to these channels — if any — will now be skipped instead of killing the subscription, matching the rest of the codebase.

Test plan

  • cargo fmt --check clean
  • cargo clippy --all-targets -- -D warnings clean
  • cargo clippy --all-targets --features sync -- -D warnings clean
  • cargo clippy --all-features clean
  • cargo test --lib (default async): 895 pass
  • cargo test --lib --features sync: 1079 pass
  • New test_realtime_bars_error_handling (sync + async) verifies Error::Message(10089, _) surfaces correctly.

wboayue added 2 commits May 2, 2026 16:26
Audit StreamDecoders for IncomingMessages::Error handling. When TWS
returns an error tied to a subscription's request_id, decoders that
didn't match on the message type either reproduced the original
"invalid digit found in string" parse failure (PnL, PnLSingle,
WshEventData, ScannerData), terminated with Error::Simple
("unexpected message: Error") losing the IB code/text (option
computations, account summary/position/update variants), or silently
skipped via UnexpectedResponse (option chain, news, wsh metadata,
display groups). 15 decoders now return Err(Error::Message(code, msg))
for type-4 messages.

Also: regression test for realtime_bars Error handling (sync + async)
locks down the prior fix (e1333df). Sync Subscription log line
distinguishes TWS errors (warn!) from real decode failures (error!).
Replace 15 copies of the Error::Message match-and-panic pattern with a
shared helper in common::test_utils. Tightens the new decoder tests
without changing semantics.
@wboayue wboayue merged commit 8ba0e39 into main May 3, 2026
3 checks passed
@wboayue wboayue deleted the fix/issue-434-decoder-error-handling branch May 3, 2026 01:53
wboayue added a commit that referenced this pull request May 3, 2026
- test-builders.md: PRs #495-#501 merged.
- issue-441-order-status-option-doubles.md: issue closed; OrderStatus
  fields are now Option<f64>.
- issue-434-stream-decoder-error-handling.md: issue closed; main fixed
  via #490 (v2-stable mirror is a separate concern if it surfaces).
- improve-encoder-test-coverage.md: approach superseded by the
  test-builders pattern (assert_request<B>(builder) at the integration
  layer in place of field-level encoder unit tests).

Merged commits preserve the history.
wboayue added a commit that referenced this pull request May 12, 2026
#490) (#567)

* fix: surface TWS errors on subscription channels (v2-stable backport of #490)

Backports the issue #434 fix from main (#490) to v2-stable.

15 StreamDecoders now match on IncomingMessages::Error and return
Err(Error::Message(code, msg)) instead of producing a parse error, dropping
the IB error code via Error::Simple("unexpected message: ..."), or silently
skipping via UnexpectedResponse. Affects: OptionComputation, OptionChain,
AccountSummaryResult, PnL, PnLSingle, PositionUpdate, PositionUpdateMulti,
AccountUpdate, AccountUpdateMulti, Vec<ScannerData>, NewsBulletin,
NewsArticle, WshMetadata, WshEventData, DisplayGroupUpdate.

Adds regression test for realtime_bars (sync + async) locking down the
prior decoder fix, plus per-decoder unit tests verifying a wire error
decodes to Error::Message(10089, _).

Sync Subscription log line in subscriptions/sync.rs now branches on
Error::Message: TWS-side errors log warn! "subscription terminated by
TWS error [code] msg"; decode failures keep error!.

Layout differences vs main: v2-stable predates the
news/scanner/common/stream_decoders.rs split, so news/{sync,async}.rs
and scanner/{sync,async}.rs are edited directly with matching cfg gates.
Tests live inline. Sync news/scanner gain explicit RESPONSE_MESSAGE_IDS
arrays for symmetry with async (no behavior change).

* simplify: collapse decoder error tests behind a generic helper

Adds `assert_decode_surfaces_tws_error<T: StreamDecoder<T>>(code, msg)` and
`decoder_test_context()` to `common::test_utils::helpers`. The 15 per-decoder
error tests collapse from a 5-line `from(wire) -> decode -> unwrap_err -> assert`
body to a single typed call.

Also drops per-task narration comments (#434 references, "previously this was
called blindly", "Issue #434: when TWS returns ..."), and merges 4 sibling
`mod decoder_error_tests` blocks into the host `mod tests` for news/scanner
(sync + async).

Branch diff: 414 / 40 → 308 / 40 (-106 LoC of additions).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error messages decoded as bar data cause cryptic parse failures

1 participant