Skip to content

Conversation

@a00012025
Copy link

Problem

In SingleInstructionPipeline::handle(), when a handler returns an error while processing instruction N, the method immediately returns, preventing subsequent instructions (N+1, N+2, ...) in the same transaction from being processed.

Root Cause: Early return in error handling path at runtime/src/instruction.rs:113

Err(e) => {
    let handled = e.handle::<InstructionUpdate>(&pipe.id());
    return Err(PipelineErrors::AlreadyHandled(handled));  // ← Stops loop
}

This causes data loss when handlers encounter recoverable errors (channel backpressure, validation errors, etc.).

Solution

Match the behavior of InstructionPipeline::handle(): store errors but continue processing all instructions.

let mut err = None;
for insn in ixs.iter().flat_map(|i| i.visit_all()) {
    match res {
        Ok(()) => (),
        Err(PipelineErrors::AlreadyHandled(h)) => h.as_unit(),
        Err(e) => err = Some(e.handle::<InstructionUpdate>(&pipe.id())),  // Store, continue
    }
}
if let Some(h) = err {
    Err(PipelineErrors::AlreadyHandled(h))
} else {
    Ok(())
}

Impact

  • Before: Error on instruction N skips all remaining instructions in transaction
  • After: All instructions processed; errors still logged and returned

This only affects pipelines created via Runtime::builder().instruction(). Manual InstructionPipeline creation was unaffected.

Copy link
Collaborator

@kespinola kespinola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for thoughtful pull request. Let's discuss some.

Comment on lines -110 to -114
Err(e) => {
let handled = e.handle::<InstructionUpdate>(&pipe.id());

return Err(PipelineErrors::AlreadyHandled(handled));
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is any instruction error then the whole transaction is aborted. This error handling is designed to mimic the same transaction handling as the svm runtime. Is this not the cases?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it happens when one handler returned an error when parsing an instruction, even that instruction succeeds. I notice this issue because it didn't parse the second jupiter route instruction in this tx: https://solscan.io/tx/3YEYmqJeRJHDNX4B7yY5mmjeGbT5scMw977LXQfC4CBTihX7qoSW3AQiZnFD8Us5jz2LXoTSu5HPF5XrfqzBcWUZ

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because I returned yellowstone_vixen_core::ParseError::Filtered in some of my parsers which will be treated as error here

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@a00012025 I haven't gotten back to this but I am hesitant. What if you handle the error in your parser and don't return an error unless its something you want to fail?

I do feel as an error in parsing any instruction in the transaction should early exit but lets keep talking about it.

@a00012025 a00012025 requested a review from kespinola October 7, 2025 09:10
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.

2 participants