Skip to content

Conversation

@filipmacek
Copy link
Member

@filipmacek filipmacek commented Jan 7, 2026

Pull Request

NautilusTrader prioritizes correctness and reliability, please follow existing patterns for validation and testing.

  • I have reviewed the CONTRIBUTING.md and followed the established practices

Summary

The OKX execution client had a bug where order operation failures (submit, modify, cancel) were only logged but never propagated as rejection events to the execution engine. This caused orders to appear in incorrect states with no feedback to traders when WebSocket or HTTP operations failed.

Impact:

  • Submit Operations: Orders appeared "submitted" even when WebSocket submission failed
  • Modify Operations: Order modifications appeared "pending" even when WebSocket updates failed
  • Cancel Operations: Order cancellations appeared "pending" even when WebSocket cancels failed
  • Silent Failures: Traders saw orders stuck in incorrect states with no error feedback

Root Cause

All order operation methods (submit_regular_order, submit_conditional_order, modify_order, cancel_ws_order) followed this pattern:

fn submit_regular_order(&self, cmd: &SubmitOrder) -> anyhow::Result<()> {
    self.spawn_task("submit_order", async move {
        ws_private.submit_order(...).await?;  // Error only logged by spawn_task
        Ok(())
    });
    Ok(())  // Always returns Ok immediately
}
  1. The method spawns an async task and immediately returns Ok(())
  2. WebSocket/HTTP operations happen asynchronously inside the spawned task
  3. When these operations fail, errors are caught by spawn_task's wrapper which only logs them
  4. No OrderRejected, OrderModifyRejected, or OrderCancelRejected events were sent

Additionally, lines 659-682 in submit_order attempted error handling but were unreachable dead code since both submit_conditional_order and submit_regular_order always returned Ok(()):

// REMOVED - This code was unreachable (lines 659-682)
if let Err(e) = result {
    let rejected_event = OrderRejected::new(
        self.core.trader_id,
        order.strategy_id(),
        order.instrument_id(),
        order.client_order_id(),
        self.core.account_id,
        format!("submit-order-error: {e}").into(),
        UUID4::new(),
        cmd.ts_init,
        get_atomic_clock_realtime().get_time_ns(),
        false,
        false,
    );
    if let Some(sender) = &self.exec_event_sender {
        if let Err(e) = sender.send(ExecutionEvent::Order(OrderEventAny::Rejected(
            rejected_event,
        ))) {
            log::warn!("Failed to send OrderRejected event: {e}");
        }
    } else {
        log::warn!("Cannot send OrderRejected: exec_event_sender not initialized");
    }
    return Err(e);
}

Solution

Move error handling inside the spawned async tasks where WebSocket/HTTP errors actually occur, and send rejection events when operations fail.

self.spawn_task("submit_order", async move {
    let result = ws_private.submit_order(...).await
        .map_err(|e| anyhow::anyhow!("Submit order failed: {e}"));

    if let Err(e) = result {
        let rejected_event = OrderRejected::new(...);
        if let Some(sender) = &exec_event_sender {
            sender.send(ExecutionEvent::Order(OrderEventAny::Rejected(rejected_event)))?;
        }
        return Err(e);
    }
    Ok(())
});

Type of change

  • Bug fix (non-breaking)
  • New feature (non-breaking)
  • Improvement (non-breaking)
  • Breaking change (impacts existing behavior)
  • Documentation update
  • Maintenance / chore

@filipmacek filipmacek requested a review from cjdsellers January 7, 2026 15:25
@filipmacek filipmacek self-assigned this Jan 7, 2026
@filipmacek filipmacek added bug Something isn't working rust Relating to the Rust core labels Jan 7, 2026
@cjdsellers cjdsellers merged commit c137fd0 into develop Jan 7, 2026
19 checks passed
@cjdsellers cjdsellers deleted the fix-okx-order-events-sending branch January 7, 2026 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working rust Relating to the Rust core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants