Skip to content

MINIFICPP-2901 Rust route_to_failure by default, rollback explicit - #2262

Open
martinzink wants to merge 3 commits into
mainfrom
minifi_rust_impr_3
Open

martinzink wants to merge 3 commits into
mainfrom
minifi_rust_impr_3

Conversation

@martinzink

@martinzink martinzink commented Sep 15, 2026

Copy link
Copy Markdown
Member

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:

  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE file?
  • If applicable, have you updated the NOTICE file?

For documentation related changes:

  • Have you ensured that format looks appropriate for the output in which it is rendered?

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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::Rollback and ProcessErrorExt.
  • 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 failure relationship breaks DuplicateStreamText: its read/write calls use ?, but its processor definition declares only success and 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 declared failure relationship 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.

Comment thread minifi_rust/minifi_native/src/api/errors.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 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 FAILURE changes the processor schema, but the regenerated minifi_rs_playground.md still documents only success for DuplicateStreamText (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

@martinzink
martinzink marked this pull request as ready for review September 16, 2026 13:57

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants