diff --git a/src/project_x_py/order_manager/core.py b/src/project_x_py/order_manager/core.py index bfe12a8..d78ffbf 100644 --- a/src/project_x_py/order_manager/core.py +++ b/src/project_x_py/order_manager/core.py @@ -58,7 +58,7 @@ async def main(): import asyncio import time -from datetime import datetime +from datetime import UTC, datetime, timedelta from decimal import Decimal from typing import TYPE_CHECKING, Any, Optional @@ -731,6 +731,83 @@ async def search_open_orders( return open_orders + @handle_errors("search orders") + async def search_orders( + self, + start_timestamp: datetime | None = None, + end_timestamp: datetime | None = None, + contract_id: str | None = None, + account_id: int | None = None, + ) -> list[Order]: + """ + Search historical orders with optional contract and time filters. + + Unlike search_open_orders, this method returns terminal orders such as + filled, cancelled, expired, and rejected orders. It is useful for + reconciling race conditions where an order changes state while a cancel + request is in flight. + + Args: + start_timestamp: Start of the search window. Defaults to 30 days ago. + end_timestamp: End of the search window. Defaults to now. + contract_id: Filter by instrument (optional). + account_id: Account ID. Uses default account if None. + + Returns: + List of Order objects. + """ + if account_id is None: + if not self.project_x.account_info: + await self.project_x.authenticate() + if not self.project_x.account_info: + raise ProjectXOrderError(ErrorMessages.ORDER_NO_ACCOUNT) + account_id = self.project_x.account_info.id + + if end_timestamp is None: + end_timestamp = datetime.now(UTC) + if start_timestamp is None: + start_timestamp = end_timestamp - timedelta(days=30) + + payload: dict[str, Any] = { + "accountId": account_id, + "startTimestamp": start_timestamp.isoformat(), + "endTimestamp": end_timestamp.isoformat(), + } + + if contract_id: + resolved = await resolve_contract_id(contract_id, self.project_x) + if resolved and resolved.get("id"): + payload["contractId"] = resolved["id"] + + response = await self.project_x._make_request( + "POST", "/Order/search", data=payload + ) + + if not isinstance(response, dict): + raise ProjectXOrderError("Invalid response format") + + if not response.get("success", False): + error_msg = response.get("errorMessage", ErrorMessages.ORDER_SEARCH_FAILED) + raise ProjectXOrderError(error_msg) + + orders = [] + for order_data in response.get("orders", []): + try: + order = Order(**order_data) + orders.append(order) + + async with self.order_lock: + self.tracked_orders[str(order.id)] = order_data + self.order_status_cache[str(order.id)] = order.status + except Exception as e: + self.logger.warning( + "Failed to parse order", + extra={"error": str(e), "order_data": order_data}, + ) + continue + + return orders + async def _check_circuit_breaker(self) -> bool: """ Check if circuit breaker allows execution. @@ -916,15 +993,37 @@ async def get_order_by_id(self, order_id: int) -> Order | None: except Exception as e: self.logger.debug(f"Failed to parse cached order data: {e}") - # Fallback to API search + # Fallback to open order search try: orders = await self.search_open_orders() for order in orders: if order.id == order_id: return order - return None + except Exception as e: + self.logger.debug(f"Failed to search open orders for {order_id}: {e}") + + # Fallback to historical order search so terminal orders can be reconciled. + try: + orders = await self.search_orders() + for order in orders: + if order.id == order_id: + return order except Exception as e: self.logger.error(f"Failed to get order {order_id}: {e}") + + return None + + async def _cache_order_status( + self, order_id: int, status: int, order_data: dict[str, Any] | None = None + ) -> None: + """Update local order caches for a known order status.""" + async with self.order_lock: + order_id_str = str(order_id) + if order_data is not None: + self.tracked_orders[order_id_str] = order_data + elif order_id_str in self.tracked_orders: + self.tracked_orders[order_id_str]["status"] = status + self.order_status_cache[order_id_str] = status return None @handle_errors("cancel order") @@ -941,15 +1040,17 @@ async def cancel_order(self, order_id: int, account_id: int | None = None) -> bo """ self.logger.info(LogMessages.ORDER_CANCEL, extra={"order_id": order_id}) + # Check if order is already known to be terminal before submitting a + # cancel. Keep the lock scope small so reconciliation can perform API + # lookups without deadlocking on cache updates. + order_id_str = str(order_id) async with self.order_lock: - # Check if order is already filled - order_id_str = str(order_id) if order_id_str in self.order_status_cache: status = self.order_status_cache[order_id_str] if status == OrderStatus.FILLED or status == 2: # 2 is FILLED - raise ProjectXOrderError( - f"Cannot cancel order {order_id}: already filled" - ) + return False + if status == OrderStatus.CANCELLED or status == 3: + return True # Also check tracked orders if order_id_str in self.tracked_orders: @@ -958,58 +1059,71 @@ async def cancel_order(self, order_id: int, account_id: int | None = None) -> bo tracked.get("status") == OrderStatus.FILLED or tracked.get("status") == 2 ): - raise ProjectXOrderError( - f"Cannot cancel order {order_id}: already filled" - ) + return False + if ( + tracked.get("status") == OrderStatus.CANCELLED + or tracked.get("status") == 3 + ): + return True + + # Get account ID if not provided + if account_id is None: + if not self.project_x.account_info: + await self.project_x.authenticate() + if not self.project_x.account_info: + raise ProjectXOrderError(ErrorMessages.ORDER_NO_ACCOUNT) + account_id = self.project_x.account_info.id + + # Use correct endpoint and payload structure + payload = { + "accountId": account_id, + "orderId": order_id, + } - # Get account ID if not provided - if account_id is None: - if not self.project_x.account_info: - await self.project_x.authenticate() - if not self.project_x.account_info: - raise ProjectXOrderError(ErrorMessages.ORDER_NO_ACCOUNT) - account_id = self.project_x.account_info.id + response = await self.project_x._make_request( + "POST", "/Order/cancel", data=payload + ) - # Use correct endpoint and payload structure - payload = { - "accountId": account_id, - "orderId": order_id, - } + # Response should be a dict + if not isinstance(response, dict): + raise ProjectXOrderError("Invalid response format") - response = await self.project_x._make_request( - "POST", "/Order/cancel", data=payload - ) + success = response.get("success", False) if response else False - # Response should be a dict - if not isinstance(response, dict): - raise ProjectXOrderError("Invalid response format") + if success: + await self._cache_order_status(order_id, OrderStatus.CANCELLED) - success = response.get("success", False) if response else False - - if success: - # Update cache - if str(order_id) in self.tracked_orders: - self.tracked_orders[str(order_id)]["status"] = OrderStatus.CANCELLED - self.order_status_cache[str(order_id)] = OrderStatus.CANCELLED + # Update statistics + await self.increment("orders_cancelled") + self.stats["orders_cancelled"] += 1 + self.logger.info( + LogMessages.ORDER_CANCELLED, extra={"order_id": order_id} + ) + return True - # Update statistics - await self.increment("orders_cancelled") - self.stats["orders_cancelled"] += 1 - self.logger.info( - LogMessages.ORDER_CANCELLED, extra={"order_id": order_id} - ) + reconciled_order = await self.get_order_by_id(order_id) + if reconciled_order is not None and reconciled_order.is_terminal: + await self._cache_order_status(reconciled_order.id, reconciled_order.status) + if reconciled_order.is_cancelled: return True - else: - error_msg = response.get( - "errorMessage", ErrorMessages.ORDER_CANCEL_FAILED - ) - raise ProjectXOrderError( - format_error_message( - ErrorMessages.ORDER_CANCEL_FAILED, - order_id=order_id, - reason=error_msg, - ) - ) + self.logger.info( + "Order already terminal before cancel completed", + extra={ + "order_id": order_id, + "status": reconciled_order.status, + "status_str": reconciled_order.status_str, + }, + ) + return False + + error_msg = response.get("errorMessage", ErrorMessages.ORDER_CANCEL_FAILED) + raise ProjectXOrderError( + format_error_message( + ErrorMessages.ORDER_CANCEL_FAILED, + order_id=order_id, + reason=error_msg, + ) + ) @handle_errors("modify order") async def modify_order( diff --git a/tests/order_manager/test_core_advanced.py b/tests/order_manager/test_core_advanced.py index 8324470..a16b1bf 100644 --- a/tests/order_manager/test_core_advanced.py +++ b/tests/order_manager/test_core_advanced.py @@ -533,9 +533,8 @@ async def test_cancel_already_filled_order(self, order_manager): order_manager.tracked_orders["999"] = {"status": 2} # Already filled order_manager.order_status_cache["999"] = 2 - with pytest.raises(ProjectXOrderError) as exc_info: - await order_manager.cancel_order(999) - assert "already filled" in str(exc_info.value).lower() + result = await order_manager.cancel_order(999) + assert result is False @pytest.mark.asyncio async def test_get_order_by_id_with_invalid_cache_data(self, order_manager): diff --git a/tests/order_manager/test_order_core.py b/tests/order_manager/test_order_core.py index 2d46bc2..78aa5dc 100644 --- a/tests/order_manager/test_order_core.py +++ b/tests/order_manager/test_order_core.py @@ -109,6 +109,8 @@ async def test_cancel_order_success_and_failure(self, order_manager): assert order_manager.order_status_cache["888"] == 3 assert order_manager.stats["orders_cancelled"] == start + 1 + order_manager.tracked_orders.pop("888", None) + order_manager.order_status_cache.pop("888", None) order_manager.project_x._make_request = AsyncMock( return_value={"success": False, "errorMessage": "fail"} ) @@ -116,6 +118,60 @@ async def test_cancel_order_success_and_failure(self, order_manager): await order_manager.cancel_order(888) assert "Failed to cancel order 888: fail" in str(exc_info.value) + @pytest.mark.asyncio + async def test_cancel_order_reconciles_filled_race(self, order_manager): + """cancel_order returns False when a failed cancel reconciles to filled.""" + order_data = { + "id": 888, + "accountId": 12345, + "contractId": "MNQ", + "creationTimestamp": "2024-01-01T01:00:00Z", + "updateTimestamp": "2024-01-01T01:01:00Z", + "status": 2, + "type": 1, + "side": 1, + "size": 1, + "fillVolume": 1, + "filledPrice": 30120.0, + } + order_manager.project_x.account_info.id = 12345 + order_manager.project_x._make_request = AsyncMock( + side_effect=[ + {"success": False, "errorMessage": None}, + {"success": True, "orders": []}, + {"success": True, "orders": [order_data]}, + ] + ) + + assert await order_manager.cancel_order(888) is False + assert order_manager.order_status_cache["888"] == 2 + + @pytest.mark.asyncio + async def test_cancel_order_reconciles_cancelled_race(self, order_manager): + """cancel_order returns True when a failed cancel reconciles to cancelled.""" + order_data = { + "id": 777, + "accountId": 12345, + "contractId": "MNQ", + "creationTimestamp": "2024-01-01T01:00:00Z", + "updateTimestamp": "2024-01-01T01:01:00Z", + "status": 3, + "type": 1, + "side": 1, + "size": 1, + } + order_manager.project_x.account_info.id = 12345 + order_manager.project_x._make_request = AsyncMock( + side_effect=[ + {"success": False, "errorMessage": None}, + {"success": True, "orders": []}, + {"success": True, "orders": [order_data]}, + ] + ) + + assert await order_manager.cancel_order(777) is True + assert order_manager.order_status_cache["777"] == 3 + @pytest.mark.asyncio async def test_modify_order_success_and_aligns(self, order_manager): """modify_order aligns prices, makes API call, returns True on success.""" @@ -219,7 +275,7 @@ async def test_get_order_by_id_success(self, order_manager): "size": 1, } - # Mock search_open_orders which get_order_by_id uses internally + # Mock search_open_orders which get_order_by_id uses first. order_manager.project_x._make_request = AsyncMock( return_value={"success": True, "orders": [order_data]} ) @@ -234,6 +290,40 @@ async def test_get_order_by_id_success(self, order_manager): assert order_manager.tracked_orders["123"] == order_data assert order_manager.order_status_cache["123"] == 1 + @pytest.mark.asyncio + async def test_get_order_by_id_searches_order_history(self, order_manager): + """get_order_by_id falls back to historical search for terminal orders.""" + order_data = { + "id": 123, + "accountId": 12345, + "contractId": "MNQ", + "creationTimestamp": "2024-01-01T01:00:00Z", + "updateTimestamp": "2024-01-01T01:01:00Z", + "status": 2, + "type": 1, + "side": 0, + "size": 1, + "fillVolume": 1, + "filledPrice": 17000.0, + } + order_manager.project_x.account_info.id = 12345 + order_manager.project_x._make_request = AsyncMock( + side_effect=[ + {"success": True, "orders": []}, + {"success": True, "orders": [order_data]}, + ] + ) + + order = await order_manager.get_order_by_id(123) + + assert isinstance(order, Order) + assert order.id == 123 + assert order.status == 2 + assert order_manager.project_x._make_request.call_args_list[1].args[:2] == ( + "POST", + "/Order/search", + ) + @pytest.mark.asyncio async def test_get_order_by_id_not_found(self, order_manager): """get_order_by_id returns None when order not found."""