CAMEL-23469: Fix infinite redelivery loop when child route removes headers#23090
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
cdc980d to
4e94987
Compare
|
🧪 CI tested the following changed modules:
Build reactor — dependencies compiled but only changed modules were tested (4 modules)
|
|
yeah so Camel has historically stored such metadata as headers and/or exchange properties. EIPs usually use properties but the error handler was headers and as such it kinda got stuck there - end user can check those headers to track how many time attempt of redelivery and so on. Now that the exchange has this rather new state stuff we can put it there, and copy over the info to the headers so they are read-only. |
…aders
RedeliveryErrorHandler.incrementRedeliveryCounter() read the counter from
the CamelRedeliveryCounter exchange header. When a child route with
NoErrorHandler used removeHeaders("*"), the header was wiped during each
redelivery attempt, causing the counter to reset to 1 and loop forever.
Fix incrementRedeliveryCounter() and prepareExchangeForRedelivery() to
use the internal redeliveryCounter field as the authoritative source
instead of reading from the mutable exchange header.
Move decrementRedeliveryCounter() from the outer RedeliveryErrorHandler
class into RedeliveryTask where the internal redeliveryCounter field is
accessible. Uses the same Math.max(header, field) pattern as
incrementRedeliveryCounter() to respect nested error handler counters
while falling back to the internal field when headers are wiped.
Add redeliveryCounter and redeliveryMaxCounter fields to
ExchangeExtension so redelivery state survives removeHeaders("*").
RedeliveryErrorHandler now populates these fields alongside headers.
PollEnricher reads from ExchangeExtension instead of headers for its
save/restore pattern around aggregation with bridgeErrorHandler.
SagaProcessor stores the coordinator ID exclusively as the
Long-Running-Action message header. If a sub-route calls
removeHeaders("*"), the coordinator lookup returns null — causing
silent saga corruption (REQUIRED), hard failure (MANDATORY), or
broken compensation (REQUIRES_NEW).
Dual-store the coordinator ID in ExchangeExtension alongside the
header. getCurrentSagaCoordinator() reads from extension first, then
falls back to the header for LRA protocol interoperability.
InMemorySagaCoordinator also sets the extension field when creating
exchanges for compensation/completion.
These are now the official API for redelivery counter and saga coordinator state. The corresponding headers are set for backward compatibility.
3ba228b to
2bc34f9
Compare
…aders (#23090) * CAMEL-23469: Fix infinite redelivery loop when child route removes headers RedeliveryErrorHandler.incrementRedeliveryCounter() read the counter from the CamelRedeliveryCounter exchange header. When a child route with NoErrorHandler used removeHeaders("*"), the header was wiped during each redelivery attempt, causing the counter to reset to 1 and loop forever. Fix incrementRedeliveryCounter() and prepareExchangeForRedelivery() to use the internal redeliveryCounter field as the authoritative source instead of reading from the mutable exchange header. * Fix redelivery state resilience to header removal Move decrementRedeliveryCounter() from the outer RedeliveryErrorHandler class into RedeliveryTask where the internal redeliveryCounter field is accessible. Uses the same Math.max(header, field) pattern as incrementRedeliveryCounter() to respect nested error handler counters while falling back to the internal field when headers are wiped. Add redeliveryCounter and redeliveryMaxCounter fields to ExchangeExtension so redelivery state survives removeHeaders("*"). RedeliveryErrorHandler now populates these fields alongside headers. PollEnricher reads from ExchangeExtension instead of headers for its save/restore pattern around aggregation with bridgeErrorHandler. * Fix saga coordinator ID resilience to header removal SagaProcessor stores the coordinator ID exclusively as the Long-Running-Action message header. If a sub-route calls removeHeaders("*"), the coordinator lookup returns null — causing silent saga corruption (REQUIRED), hard failure (MANDATORY), or broken compensation (REQUIRES_NEW). Dual-store the coordinator ID in ExchangeExtension alongside the header. getCurrentSagaCoordinator() reads from extension first, then falls back to the header for LRA protocol interoperability. InMemorySagaCoordinator also sets the extension field when creating exchanges for compensation/completion. * Address review: drop "internal" from ExchangeExtension javadocs These are now the official API for redelivery counter and saga coordinator state. The corresponding headers are set for backward compatibility.
Summary
RedeliveryErrorHandler.incrementRedeliveryCounter()to use the internalredeliveryCounterfield as the authoritative counter instead of reading from theCamelRedeliveryCounterexchange header, which user routes can remove (e.g., viaremoveHeaders("*"))prepareExchangeForRedelivery()to restore redelivery headers from internal state rather than saving/restoring from exchange headers — resolves a longstanding TODO present since Camel 2.8RedeliverToSubRouteRemoveHeadersTest) with two test methods: one verifying the infinite loop is fixed, one verifyingonRedeliveryprocessors see the correct counterScenario: parent route with
onException/maximumRedeliveries(3)calls a child route (viadirect:) that usesNoErrorHandlerandremoveHeaders("*"). TheremoveHeaderswipesCamelRedeliveryCounterduring each attempt. Before this fix, the counter reset to 1 every time, causing an infinite redelivery loop.The defensive-copy mechanism (
copyFrom(original)) was introduced in Camel 2.8.0, butincrementRedeliveryCounter()was never updated to use the internal field — it kept reading from the header as it had since before the defensive copy existed. In Camel 2.7,prepareExchangeForRedelivery()did not copy from the original, so headers survived intact and this bug did not manifest.Fix three instances of internal state being read from mutable exchange message headers instead of authoritative internal fields, making them vulnerable to removeHeaders("*") in sub-routes.