CAMEL-24594: Log REST DSL response marshalling failures - #26060
Merged
Conversation
When the REST DSL response binding (bindingMode json/xml) fails to marshal the response body, the failure happens in RestBindingAdvice#after (post-routing) so it is not routed through the error handler and was previously invisible - nothing was logged at any level. This made a downstream serialization failure very hard to diagnose. Log the exception (WARN, with stacktrace) when marshalling the response body fails, bringing the post-routing binding step in line with how every other failure in Camel surfaces. The response code behaviour is unchanged: an explicitly set HTTP_RESPONSE_CODE still wins, consistent with plain routes. Extended the regression test with a variant whose processor sets HTTP_RESPONSE_CODE=200 before the failing marshal, to pin that an explicitly set code still wins. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
Croway
approved these changes
Sep 3, 2026
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 562 tested, 27 compile-only — current: 562 all testedMaveniverse Scalpel detected 589 affected modules (current approach: 562).
|
gnodet
approved these changes
Sep 3, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Clean, well-scoped fix. The previously silent marshalling failure path in RestBindingAdvice#marshal now logs at WARN with stacktrace — correct level and placement given that this runs in #after (post-routing, outside error handler reach). The SLF4J invocation uses placeholders correctly and Exchange.toString() only emits the exchange ID, so no sensitive data leaks.
The new test (testExplicitResponseCodeStillWinsOnMarshalFailure) properly pins the contract that an explicitly set HTTP_RESPONSE_CODE is preserved through a marshal failure. No behavioral change to status codes, exception handling, or response bodies — purely additive logging.
📋 PR Metadata
| Aspect | Current | Suggested |
|---|---|---|
| Labels | components, core |
+ bug |
| Milestone | (none) | 4.23.0 |
🔀 Backport Status
main but no backport PRs or labels were found for active maintenance branches:
camel-4.22.x— no backport PR foundcamel-4.18.x— no backport PR foundcamel-4.14.x— no backport PR found
The core change (catch-block logging in RestBindingAdvice.java) has been stable since 2017 and should backport cleanly. The test file VertxRestBindingMarshalFailureTest was recently added to main; branches that lack it would need the test ported first.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Guillaume Nodet
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
CAMEL-24594: when the REST DSL response binding (
bindingMode(json)/xml) fails to marshal the response body, the failure is genuinely invisible.Response marshalling happens in
RestBindingAdvice#after(i.e. after routing has completed), so the exception is not routed through the error handler. Unlike an in-route exception — whichDefaultErrorHandlerstill logs with a stacktrace even when the status is masked to 200 — a marshal failure was not logged at any level. That is the only Camel failure path where the error is completely silent, and it is what made a downstream serialization failure take a full day to diagnose after a platform upgrade.Per the discussion on the ticket (thanks @jamesnetherton), this PR narrows the fix to logging only and deliberately makes no change to response-code behaviour:
HTTP_RESPONSE_CODEcontinues to win over a failed exchange, consistent with plain routes — people rely on this.RestBindingAdvice#marshalto forceHTTP_RESPONSE_CODE=500was considered and rejected, because it would make the REST binding inconsistent with every other failure path and override a code the user deliberately set.Changes
RestBindingAdvice#marshalnow logs the marshalling failure atWARN(with stacktrace) before setting the exception on the exchange, bringing the post-routing binding step in line with how every other failure in Camel surfaces.VertxRestBindingMarshalFailureTestwith a variant whose processor setsHTTP_RESPONSE_CODE=200before the failing marshal, pinning that the explicitly set code still wins (and the failure is logged rather than masking the code).Testing
VertxRestBindingMarshalFailureTest— 3 tests, all green.Claude Code on behalf of Claus Ibsen (davsclaus)
🤖 Generated with Claude Code