Skip to content

Fix incorrect handling of data responses in msgbus#3310

Merged
cjdsellers merged 2 commits into
developfrom
msgbus-data-response-fix
Dec 17, 2025
Merged

Fix incorrect handling of data responses in msgbus#3310
cjdsellers merged 2 commits into
developfrom
msgbus-data-response-fix

Conversation

@filipmacek

Copy link
Copy Markdown
Member

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

Fix Message Bus TypeId Mismatch Bug

2025-12-17T17:25:56.857102000Z [ERROR] TypedMessageHandler downcast failed:
expected nautilus_common::messages::data::response::InstrumentResponse
got TypeId(0x396bc1ca65527c35903f52d8288fb4ba)

All data request/response flows were incorrect - request_instrument(), request_quotes(), request_trades(), etc. failed across all adapters.

Root Cause

send_response() was passing &DataResponse (enum wrapper) to handlers expecting &InstrumentResponse (inner type):

// BEFORE (BUGGY):
pub fn send_response(correlation_id: &UUID4, message: &DataResponse) {
    if let Some(handler) = handler {
        handler.0.handle(message);  // Passing &DataResponse to handler expecting &InstrumentResponse
    }
}

When handler tried to downcast:

message.downcast_ref::<InstrumentResponse>()  // Fails - message is &DataResponse, not &InstrumentResponse

The Fix

Unwrap the enum before passing to handler:

// AFTER (FIXED):
pub fn send_response(correlation_id: &UUID4, message: &DataResponse) {
    if let Some(handler) = handler {
        match message {
            DataResponse::Data(resp) => handler.0.handle(resp),
            DataResponse::Instrument(resp) => handler.0.handle(resp.as_ref()),
            DataResponse::Instruments(resp) => handler.0.handle(resp),
            DataResponse::Book(resp) => handler.0.handle(resp),
            DataResponse::Quotes(resp) => handler.0.handle(resp),
            DataResponse::Trades(resp) => handler.0.handle(resp),
            DataResponse::Bars(resp) => handler.0.handle(resp),
        }
    }
}

Why Tests Didn't Catch It

Tests used a different function that bypassed send_response():

// Tests used this (bypassed the bug):
msgbus::response(&request_id, response.as_any());  // Takes &dyn Any, goes directly to handler

// Production used this (contained the bug):
msgbus::send_response(&request_id, &data_response);  // Enum wasn't unwrapped

How Data engine code actually works

The data engine shows the correct production usage pattern:

crates/data/src/engine/mod.rs:720-737

The data engine receives a DataResponse and calls send_response(&resp).

Solution: Deleted response() function and refactored tests to use send_response() with wrapped DataResponse:

// Before:
msgbus::response(&request_id, response.as_any());

// After(matches data engine pattern):
let data_response = DataResponse::Instrument(Box::new(response));
msgbus::send_response(&request_id, &data_response);

Impact

  • ✅ Data request/response flow now works
  • ✅ Tests now exercise production code paths
  • ✅ Future bugs of this type will be caught by tests

Type of change

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

Testing

Ensure new or changed logic is covered by tests.

  • Affected code paths are already covered by the test suite
  • I added/updated tests to cover new or changed logic

@filipmacek filipmacek self-assigned this Dec 17, 2025
@filipmacek filipmacek added bug Something isn't working rust Relating to the Rust core labels Dec 17, 2025
@cjdsellers cjdsellers merged commit 048a659 into develop Dec 17, 2025
19 checks passed
@cjdsellers cjdsellers deleted the msgbus-data-response-fix branch December 17, 2025 20:16
Martingale42 pushed a commit to Martingale42/nautilus_trader that referenced this pull request Mar 20, 2026
arif-b-khan pushed a commit to arif-b-khan/nautilus_trader that referenced this pull request May 29, 2026
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.

2 participants