Skip to content

Commit d0747de

Browse files
committed
Skip done trades in error() to prevent late-error mutation
Mainly a pre-defensive update here since it _looks like it could happen_ but I've never seen any proof "delayed replay updates" happen live. More of a logical protection fix against other condtions until we get a better refactor of some of the long state-tracking-logic in place. `Wrapper.error` looks up `self.trades[(clientId, reqId)]` whenever the incoming reqId is non-negative, but `self.trades` never evicts finished orders. A late or replayed error for a long-completed order would silently: - flip its `OrderStatus.status` to `ValidationError` on the warning branch (corrupting the audit log of a Filled/Cancelled order); or - set `advancedError` on the trade in the error branch (`if not isDone()` only guards the cancel-mutation, not the `advancedError` assignment). Treat done trades as absent at the lookup site so the rest of `error` only acts on live orders. Live-order behavior is unchanged.
1 parent c679af6 commit d0747de

2 files changed

Lines changed: 70 additions & 0 deletions

File tree

ib_async/wrapper.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1678,6 +1678,14 @@ def error(
16781678
# reqId is a local orderId, but is delivered as -1 if this is a non-order-related error
16791679
if reqId != -1:
16801680
trade = self.trades.get((self.clientId, reqId))
1681+
# Trades are never evicted from `self.trades`, so a late or
1682+
# replayed error for an already-finished order would otherwise
1683+
# corrupt its status (warning branch sets ValidationError) or
1684+
# set `advancedError` (error branch) on a completed trade.
1685+
# Treat done trades as absent here so the rest of the function
1686+
# only acts on live orders.
1687+
if trade and trade.isDone():
1688+
trade = None
16811689

16821690
# Warnings are currently:
16831691
# 105 - Order being modified does not match the original order. (?)

tests/test_error_done_trade.py

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
"""Late/replayed errors for already-finished orders must not mutate them.
2+
3+
`self.trades` is never evicted, so an error arriving for a long-finished
4+
orderId would otherwise:
5+
- flip its status to ValidationError on the warning branch
6+
- set its `advancedError` on the error branch
7+
8+
Both are wrong: a Filled or Cancelled order is final.
9+
"""
10+
11+
import ib_async as ibi
12+
from ib_async.order import Order, OrderState, OrderStatus, Trade
13+
14+
15+
def _doneTrade(ib, *, orderId=1, permId=42, status=OrderStatus.Filled):
16+
contract = ibi.Stock("ABC", "SMART", "USD")
17+
contract.conId = 12345
18+
order = Order(orderId=orderId, clientId=ib.wrapper.clientId, permId=permId)
19+
orderStatus = OrderStatus(orderId=orderId, status=status)
20+
trade = Trade(contract, order, orderStatus, [], [])
21+
ib.wrapper.trades[(ib.wrapper.clientId, orderId)] = trade
22+
return trade
23+
24+
25+
def test_warning_error_does_not_mutate_filled_trade():
26+
ib = ibi.IB()
27+
ib.wrapper.clientId = 0
28+
trade = _doneTrade(ib, orderId=1, status=OrderStatus.Filled)
29+
30+
# 105 is in the warning code set
31+
ib.wrapper.error(reqId=1, errorCode=105, errorString="late warning",
32+
advancedOrderRejectJson="")
33+
34+
assert trade.orderStatus.status == OrderStatus.Filled
35+
assert trade.log == []
36+
37+
38+
def test_error_does_not_set_advancedError_on_cancelled_trade():
39+
ib = ibi.IB()
40+
ib.wrapper.clientId = 0
41+
trade = _doneTrade(ib, orderId=2, status=OrderStatus.Cancelled)
42+
assert trade.advancedError == ""
43+
44+
# 201 is a non-warning order error code
45+
ib.wrapper.error(reqId=2, errorCode=201, errorString="late reject",
46+
advancedOrderRejectJson='{"foo":"bar"}')
47+
48+
assert trade.advancedError == ""
49+
assert trade.orderStatus.status == OrderStatus.Cancelled
50+
51+
52+
def test_warning_error_still_mutates_live_trade():
53+
"""Sanity: the live-trade path must remain intact."""
54+
ib = ibi.IB()
55+
ib.wrapper.clientId = 0
56+
trade = _doneTrade(ib, orderId=3, status=OrderStatus.Submitted)
57+
58+
ib.wrapper.error(reqId=3, errorCode=105, errorString="live warning",
59+
advancedOrderRejectJson="")
60+
61+
assert trade.orderStatus.status == OrderStatus.ValidationError
62+
assert len(trade.log) == 1

0 commit comments

Comments
 (0)