MINIFICPP-2901 Rust route_to_failure by default, rollback explicit - #2262
martinzink wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Existing complex processors and a stream transformer are not fully migrated to the new error semantics.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Refactors Rust processor error handling so ordinary errors route to failure while rollback is explicit, and improves Rust test execution.
Changes:
- Introduces
ProcessError::RollbackandProcessErrorExt. - Updates wrappers and playground processors to use the new semantics.
- Uses cargo-nextest when available.
File summaries
| File | Description |
|---|---|
minifi_rust/CMakeLists.txt |
Adds nextest support and doctests. |
minifi_native/src/lib.rs |
Re-exports the renamed error extension trait. |
minifi_native/src/api/errors.rs |
Defines default routing and explicit rollback conversion. |
minifi_native/src/c_ffi/c_ffi_processor_definition.rs |
Maps rollback errors to native statuses. |
minifi_native/src/api/processor_wrappers/complex_processor.rs |
Makes unscheduled errors explicit rollbacks. |
minifi_native/src/api/processor_wrappers/flow_file_source.rs |
Rolls back session failures. |
minifi_native/src/api/processor_wrappers/flow_file_stream_transform.rs |
Separates routing from rollback behavior. |
minifi_native/src/api/processor_wrappers/flow_file_transform.rs |
Implements and tests routing versus rollback. |
minifi_rs_playground/src/processors/asciify_german.rs |
Uses default failure routing. |
minifi_rs_playground/src/processors/kamikaze_processor.rs |
Returns an explicit rollback error. |
minifi_rs_playground/src/processors/kamikaze_processor/tests.rs |
Updates rollback expectations. |
minifi_rs_playground/src/processors/lorem_ipsum_cs_user.rs |
Rolls back controller-service errors. |
minifi_rs_playground/src/processors/put_file.rs |
Uses default failure routing. |
Review details
Suppressed comments (1)
minifi_rust/minifi_native/src/api/errors.rs:89
- Routing every raw I/O error to the literal
failurerelationship breaksDuplicateStreamText: itsread/writecalls use?, but its processor definition declares onlysuccessand does not support dynamic relationships. On an I/O error the stream wrapper therefore transfers to an undeclared, non-auto-terminated relationship, causing commit to fail instead of producing the intended failure route. Add a declaredfailurerelationship for that processor or make those I/O errors explicit rollbacks; also audit other consumers before making this the default conversion.
ProcessError::Route(RouteError {
relationship: "failure",
source: Box::new(err),
log_level: LogLevel::Warn,
})
- Files reviewed: 13/13 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
8e0aeb0 to
6249729
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Mock cancellation can retain partial output, and generated processor documentation is stale.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 2
- Review effort level: Balanced
6249729 to
69b2db2
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The generated processor documentation omits DuplicateStreamText’s newly added failure relationship.
Review details
Suppressed comments (1)
minifi_rust/extensions/minifi_rs_playground/src/processors/duplicate_text.rs:76
- Adding
FAILUREchanges the processor schema, but the regeneratedminifi_rs_playground.mdstill documents onlysuccessforDuplicateStreamText(lines 91–96). Please regenerate and commit the extension documentation so users can discover and connect the new failure relationship.
const RELATIONSHIPS: &'static [Relationship] = &[SUCCESS, FAILURE];
- Files reviewed: 23/23 changed files
- Comments generated: 0 new
- Review effort level: Balanced
I've changed how err routing works in transformer processors
Removed the processerr (complex and source processors now return Result<T, MinifiErr> any error 'bubbled' via ? is rollback error (we cant route the error to failure because there maybe multiple flowfiles in use and if a source has some problems its usually dont want to create flowfiles rather than to route them to fialure.
So for complex and source processors the processor must be explicit what to do in case of errors if they dont want them to cause rollback.
For trasnformer type processors (FlowFileTransform FlowFileStreamTransform) , since they work on a single flow file its simpler so I've introduced an ErrorRelationship which the processor has to explicitly set. Now every 'bubbled' (via ?) error will simply route the flowfile it is working on to this relationship and will WARN log the error in question.
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically main)?
Is your initial contribution a single, squashed commit?
For code changes:
For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.