allthingssecurity commented on code in PR #27677:
URL: https://github.com/apache/camel/pull/27677#discussion_r4240516168
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc:
##########
@@ -863,6 +863,15 @@ comma and the statement ran once for each part (once for
each byte of a `byte[]`
Collections, iterators and other arrays (including `int[]`, `long[]` and other
primitive arrays) still run the statement
once per element; send a `List` to run it once per value.
+=== camel-mybatis, camel-sql - onConsume does not run for a failed or rollback
only exchange
Review Comment:
Applied in 19ea6bb49bf1: the heading is now
`=== camel-mybatis, camel-sql - onConsume does not run for a failed or
rollback only exchange (Breaking change)`.
_Claude Code on behalf of allthingssecurity_
##########
components/camel-mybatis/src/main/java/org/apache/camel/component/mybatis/MyBatisConsumer.java:
##########
@@ -136,18 +136,21 @@ public int processBatch(Queue<Object> exchanges) throws
Exception {
try {
getProcessor().process(exchange);
- if (onConsume != null) {
+ // only consume the data when the exchange was processed
successfully: a failed or rollback only
+ // exchange leaves the data as it is, so the next poll
consumes it again
+ if (onConsume != null && !exchange.isFailed() &&
!exchange.isRollbackOnly()) {
endpoint.getProcessingStrategy().commit(endpoint,
exchange, data, onConsume);
}
} catch (Exception e) {
handleException(e);
}
- if (getEndpoint().isTransacted() && exchange.isFailed()) {
+ if (getEndpoint().isTransacted() && (exchange.isFailed() ||
exchange.isRollbackOnly())) {
// break out as we are transacted and should rollback
cause = exchange.getException();
if (cause == null) {
- cause = new RollbackExchangeException("Rollback
transaction due error processing exchange", null);
+ // rollback only (the exception must be created before the
exchange is released)
+ cause = new RollbackExchangeException("Rollback
transaction due error processing exchange", exchange);
Review Comment:
Confirmed: this branch is only reached when the exchange has no exception,
and the exception ends up in the WARN
"Failed polling endpoint ..." that `ScheduledPollConsumer` writes through
the exception handler at every such poll.
Changed in 19ea6bb49bf1 to "Rollback transaction as the exchange was marked
as rollback only"; a failed exchange
still throws its own exception.
`MyBatisOnConsumeFailedTest.testTransactedRollbackOnlyExchangeBreaksOutOfBatch`
now
asserts the message. camel-mybatis suite: 52 tests, 0 failures.
_Claude Code on behalf of allthingssecurity_
##########
components/camel-sql/src/main/java/org/apache/camel/component/sql/SqlConsumer.java:
##########
@@ -278,20 +278,23 @@ public int processBatch(Queue<Object> exchanges) throws
Exception {
exchange.setException(e);
}
- if (getEndpoint().isTransacted() && exchange.isFailed()) {
+ // a rollback only exchange (without an exception) did not
complete successfully either
+ boolean failed = exchange.isFailed() || exchange.isRollbackOnly();
+
+ if (getEndpoint().isTransacted() && failed) {
// break out as we are transacted and should rollback
Exception cause = exchange.getException();
+ if (cause == null) {
+ // rollback only (the exception must be created before the
exchange is released)
+ cause = new RollbackExchangeException("Rollback
transaction due error processing exchange", exchange);
Review Comment:
Same change in `SqlConsumer` (19ea6bb49bf1), asserted in
`SqlConsumerRollbackOnlyTest.testTransactedRollbackOnlyExchangeBreaksOutOfBatch`.
camel-sql suite: 323 tests,
0 failures, 3 skipped; `SqlFunctionDataSourceTest` (MariaDB4j) could not
start its embedded database locally
(missing native library, same on main) and was not run.
_Claude Code on behalf of allthingssecurity_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
