Skip to content

CAMEL-25527: camel-mybatis, camel-sql - do not run onConsume for a failed or rollback only exchange - #27677

Merged
davsclaus merged 2 commits into
apache:mainfrom
allthingssecurity:camel-mybatis-sql-on-consume-failed
Oct 11, 2026
Merged

davsclaus merged 2 commits into
apache:mainfrom
allthingssecurity:camel-mybatis-sql-on-consume-failed

Conversation

@allthingssecurity

Copy link
Copy Markdown
Contributor

Description

CAMEL-25527

MyBatisConsumer.processBatch ran the onConsume statements after every exchange, also when the exchange failed (a failed route leaves the exception on the exchange instead of throwing it) or was marked rollback only. The row was then marked as consumed (in the documented example update ACCOUNT set PROCESSED = true) and was not polled again, although the javadoc says the statements run "after successful processing". SqlConsumer.processBatch picked onConsumeFailed from exchange.isFailed() only, so a rollback only exchange (no exception) got onConsume. This is the sibling of the camel-jooq fix CAMEL-25511.

Now:

  • camel-mybatis runs onConsume only when the exchange is neither failed nor rollback only. The row of a failed exchange is consumed again by the next poll, so a row that always fails is processed at every poll; onException(...).handled(true) consumes it anyway (docs and catalog copy say so).
  • camel-sql runs onConsumeFailed for a rollback only exchange, and no statement when onConsumeFailed is not set (as for a failed exchange), so the row is consumed again by the next poll.
  • With transacted=true, both consumers now break out of the batch at a rollback only exchange, as they do at a failed one. That code path created RollbackExchangeException with a null exchange, which throws a NullPointerException (it had never run, since isFailed() implies an exception); it is now created with the exchange, before the exchange is released.

Both consumers already create their exchanges with createExchange(false) and call releaseExchange(exchange, false) after reading the outcome, so the change also holds with the pooled exchange factory; the tests run with it too. No extra per-message cost (one more flag read). An upgrade guide note is added for 4.23 under its own heading next to the existing camel-mybatis entry.

Tests:

  • New MyBatisOnConsumeFailedTest: one poll (scheduler not started) where the route fails for one account, marks one exchange rollback only, or (with transacted=true) marks the second of three rollback only.
  • New SqlConsumerRollbackOnlyTest: one poll over the three projects of createAndPopulateDatabase.sql where the AMQ exchange is marked rollback only: with onConsumeFailed, without it, and with transacted=true.
  • MyBatisOnConsumeFailedPooledExchangeTest and SqlConsumerRollbackOnlyPooledExchangeTest run the same tests with PooledExchangeFactory.
  • Without the change all 12 fail, e.g. expected: <[2]> but was: <[]>, expected: <[DONE, BAD, DONE]> but was: <[DONE, DONE, DONE]>, expected: <[DONE, ASF, DONE]> but was: <[DONE, DONE, DONE]> and Expected org.apache.camel.RollbackExchangeException to be thrown, but nothing was thrown.
  • With the change: camel-mybatis 52 tests, camel-sql 323 tests (3 skipped), 0 failures. SqlFunctionDataSourceTest was excluded locally: it needs MariaDB4j, which does not install on my machine, and fails the same way without the change.

Not changed here: JpaConsumer runs the @Consumed delete handler when exchange.getException() == null, so a rollback only exchange is consumed there as well; that can be a follow-up if wanted.

Target

  • I checked that the commit is targeting the correct branch (Camel 4 uses the main branch)

Tracking

  • If this is a large change, bug fix, or code improvement, I checked there is a JIRA issue filed for the change (usually before you start working on it).

Apache Camel coding standards and style

  • I checked that each commit in the pull request has a meaningful subject line and body.
  • I have run mvn clean install -DskipTests locally from root folder and I have committed all auto-generated changes.
    (I built and tested camel-mybatis and camel-sql with install, including the formatter and import-sort plugins. I did not run the full root build.)

AI-assisted contributions

  • If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., Co-authored-by trailers) and the PR description identifies the AI tool used.
    This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a Co-Authored-By trailer.

Claude Code on behalf of allthingssecurity

🤖 Generated with Claude Code

…iled or rollback only exchange

The camel-mybatis consumer ran the onConsume statements after every
exchange, also when the exchange failed (a failed route sets the exception
on the exchange instead of throwing it) or was marked rollback only, so the
row was marked as consumed and was not polled again. It now runs them only
for an exchange that completed successfully; the row of a failed or
rollback only exchange is consumed again by the next poll. Handle the
exception in the route, for example with onException(...).handled(true),
to consume it anyway.

The camel-sql consumer picked onConsumeFailed only for an exchange with an
exception, so a rollback only exchange got onConsume. It now gets
onConsumeFailed.

With transacted=true both consumers now also break out of the batch at a
rollback only exchange, as they do at a failed one. That path created
RollbackExchangeException without the exchange, which threw a
NullPointerException; it is now created with the exchange, before the
exchange is released.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run
  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
  • You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
  • Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@davsclaus davsclaus 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.

Thanks! This matches the documented contract ("after successful processing") and lines up with the merged camel-jooq fix (CAMEL-25511), including the handled(true) escape hatch. Good tests, including the pooled exchange factory variants.

Two small optional points inline. Note: camel-jpa has the same rollback-only gap, as you mention in the description, so a follow-up JIRA would be welcome.


Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying. It is a static review against the project conventions and does not replace static analysis or specialized review tools.

Comment thread docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc Outdated
…ade note as breaking, own message for a rollback only exchange

The exception that breaks out of a transacted batch for a rollback only exchange no longer says there was an
error processing the exchange.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

🧪 CI tested the following changed modules:

  • catalog/camel-catalog
  • components/camel-mybatis
  • components/camel-sql
  • docs

🔬 Scalpel shadow comparison — Scalpel: 12 of 704 tested, 25 compile-only — current: 12 all tested

Maveniverse Scalpel detected 12 affected modules (current approach: 12).

Skip-tests mode would test 12 modules (4 direct + 10 downstream), skip tests for 25 (generated code, meta-modules)

Modules Scalpel would test (12)
  • camel-jbang-mcp ← downstream of org.apache.camel:camel-catalog
  • camel-jbang-plugin-mcp ← downstream of org.apache.camel:camel-jbang-core
  • camel-jbang-plugin-route-parser ← downstream of org.apache.camel:camel-route-parser
  • camel-jbang-plugin-tui ← downstream of org.apache.camel:camel-catalog
  • camel-jbang-plugin-validate ← downstream of org.apache.camel:camel-yaml-dsl-validator
  • camel-jta ← downstream of org.apache.camel:camel-sql
  • camel-kafka ← downstream of org.apache.camel:camel-sql
  • camel-launcher-container ← downstream of org.apache.camel:camel-launcher
  • camel-mybatis ← components/camel-mybatis/src/main/docs/mybatis-component.adoc, components/camel-mybatis/src/main/java/org/apache/camel/component/mybatis/MyBatisConsumer.java, components/camel-mybatis/src/test/java/org/apache/camel/component/mybatis/MyBatisOnConsumeFailedPooledExchangeTest.java, components/camel-mybatis/src/test/java/org/apache/camel/component/mybatis/MyBatisOnConsumeFailedTest.java
  • camel-sql ← components/camel-sql/src/main/java/org/apache/camel/component/sql/SqlConsumer.java, components/camel-sql/src/test/java/org/apache/camel/component/sql/SqlConsumerRollbackOnlyPooledExchangeTest.java, components/camel-sql/src/test/java/org/apache/camel/component/sql/SqlConsumerRollbackOnlyTest.java
  • camel-yaml-dsl-validator ← downstream of org.apache.camel:camel-catalog
  • camel-yaml-dsl-validator-maven-plugin ← downstream of org.apache.camel:camel-yaml-dsl-validator
Modules with tests skipped (25)
  • apache-camel
  • camel-allcomponents
  • camel-catalog-console
  • camel-catalog-maven
  • camel-catalog-suggest
  • camel-componentdsl
  • camel-endpointdsl
  • camel-endpointdsl-support
  • camel-itest
  • camel-jbang-core
  • camel-jbang-it
  • camel-jbang-main
  • camel-jbang-plugin-edit
  • camel-jbang-plugin-generate
  • camel-jbang-plugin-kubernetes
  • camel-jbang-plugin-test
  • camel-kamelet-main
  • camel-launcher
  • camel-report-maven-plugin
  • camel-route-parser
  • camel-yaml-dsl
  • camel-yaml-dsl-deserializers
  • camel-yaml-dsl-maven-plugin
  • coverage
  • dummy-component

ℹ️ Shadow mode — Scalpel observes but does not affect test execution. Learn more

All tested modules (39 modules, 6m 22s total)

Total reactor time: 6m 22s

Module Duration Status
Camel :: SQL 54.6s SUCCESS
Camel :: Launcher 38.4s SUCCESS
Camel :: MyBatis 38.2s SUCCESS
Camel :: JBang :: MCP 34.0s SUCCESS
Camel :: Component DSL 27.4s SUCCESS
Camel :: Catalog :: Camel Catalog 20.2s SUCCESS
Camel :: YAML DSL :: Validator 19.3s SUCCESS
Camel :: JBang :: Plugin :: Kubernetes 19.0s SUCCESS
Camel :: YAML DSL 17.5s SUCCESS
Camel :: JBang :: Plugin :: Validate 15.2s SUCCESS
Camel :: JTA 14.0s SUCCESS
Camel :: Docs 13.7s SUCCESS
Camel :: JBang :: Plugin :: Testing 11.2s SUCCESS
Camel :: Kamelet Main 9.8s SUCCESS
Camel :: YAML DSL :: Validator Maven Plugin 9.0s SUCCESS
Camel :: Catalog :: Camel Route Parser 6.1s SUCCESS
Camel :: YAML DSL :: Deserializers 5.9s SUCCESS
Camel :: Catalog :: Camel Report Maven Plugin 5.8s SUCCESS
Camel :: All Components Sync point 4.1s SUCCESS
Camel :: Catalog :: Maven 2.4s SUCCESS
Camel :: YAML DSL :: Maven Plugins 2.3s SUCCESS
Camel :: Catalog :: Suggest (deprecated) 2.1s SUCCESS
Camel :: Assembly 1.6s SUCCESS
Camel :: JBang :: Integration tests 1.5s SUCCESS
Camel :: JBang :: Plugin :: Edit 1.2s SUCCESS
Camel :: JBang :: Plugin :: MCP 1.2s SUCCESS
Camel :: Coverage 0.9s SUCCESS
Camel :: JBang :: Plugin :: Generate 0.9s SUCCESS
Camel :: Catalog :: Dummy Component 0.8s SUCCESS
Camel :: Catalog :: Console 0.7s SUCCESS
Camel :: Endpoint DSL :: Support 0.6s SUCCESS
Camel :: JBang :: Main 0.6s SUCCESS
Camel :: Launcher :: Container 0.6s SUCCESS
Camel :: JBang :: Plugin :: Route Parser 0.5s SUCCESS
Camel :: Endpoint DSL n/a
Camel :: Integration Tests n/a
Camel :: JBang :: Core n/a
Camel :: JBang :: Plugin :: TUI n/a
Camel :: Kafka n/a

Top 20 slowest modules:

  • Camel :: SQL (54.6s)
  • Camel :: Launcher (38.4s)
  • Camel :: MyBatis (38.2s)
  • Camel :: JBang :: MCP (34.0s)
  • Camel :: Component DSL (27.4s)
  • Camel :: Catalog :: Camel Catalog (20.2s)
  • Camel :: YAML DSL :: Validator (19.3s)
  • Camel :: JBang :: Plugin :: Kubernetes (19.0s)
  • Camel :: YAML DSL (17.5s)
  • Camel :: JBang :: Plugin :: Validate (15.2s)
  • Camel :: JTA (14.0s)
  • Camel :: Docs (13.7s)
  • Camel :: JBang :: Plugin :: Testing (11.2s)
  • Camel :: Kamelet Main (9.8s)
  • Camel :: YAML DSL :: Validator Maven Plugin (9.0s)
  • Camel :: Catalog :: Camel Route Parser (6.1s)
  • Camel :: YAML DSL :: Deserializers (5.9s)
  • Camel :: Catalog :: Camel Report Maven Plugin (5.8s)
  • Camel :: All Components Sync point (4.1s)
  • Camel :: Catalog :: Maven (2.4s)

⚙️ View full build and test results

@davsclaus davsclaus added this to the 4.23.0 milestone Oct 11, 2026
@davsclaus davsclaus added the bug Something isn't working label Oct 11, 2026
@davsclaus
davsclaus merged commit 06f6919 into apache:main Oct 11, 2026
6 checks passed
@allthingssecurity
allthingssecurity deleted the camel-mybatis-sql-on-consume-failed branch October 11, 2026 07:08
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