CAMEL-24176: camel-cxfrs - Fix async producer never mapping HTTP errors to CxfOperationException - #24924
Conversation
…rs to CxfOperationException Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Great catch — two real bugs fixed.
Bug 1: Reversed isInstance — throwable.getClass().isInstance(WebApplicationException.class) always returns false because it tests whether the Class<WebApplicationException> object is an instance of the throwable's runtime class (which it never is — Class is not an exception). This made both if/else if branches dead code, so every async WebApplicationException and ResponseProcessingException fell through to the raw exchange.setException(throwable) path, bypassing CxfOperationException wrapping entirely. The fix correctly uses instanceof with pattern matching.
Bug 2: Silent exception swallowing — When shouldHandleError() returned false (e.g., throwExceptionOnFailure=false or status ≤ 207), neither handleError() nor exchange.setException() was called — the failure was silently dropped. The added else { exchange.setException(throwable); } ensures the exception always reaches the exchange.
Code quality: Modern instanceof pattern matching replaces verbose .getClass().isInstance() + .class.cast() — cleaner and correct. Fix correctly applied to both CxfInvocationCallback and CxfProxyInvocationCallback.
Test: Well-structured — sends an invalid ID that triggers a server-side 500, then asserts the async proxy producer properly maps the WebApplicationException to CxfOperationException with the correct status code.
LGTM ✅
Claude Code on behalf of gnodet — AI-generated review
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 16 tested, 29 compile-only — current: 16 all testedMaveniverse Scalpel detected 45 affected modules (current approach: 16).
|
2 participants
Claude Code on behalf of davsclaus
Summary
Fix two bugs in the async CXF-RS producer's
fail()callback in bothCxfInvocationCallbackandCxfProxyInvocationCallback:Reversed
isInstancecheck —throwable.getClass().isInstance(WebApplicationException.class)always evaluates tofalsebecause it tests whether theClassobject is an instance of the throwable's class (which it never is). Bothifbranches were dead code, so every async failure fell through toexchange.setException(throwable)with the raw JAX-RS exception instead of wrapping it inCxfOperationException. Fixed by replacing withthrowable instanceof WebApplicationException(same forResponseProcessingException).Silent exception swallowing — When
shouldHandleError()returnedfalse(e.g.,throwExceptionOnFailure=falseor status ≤ 207), neitherhandleError()norexchange.setException()was called — the exception was silently dropped. Addedelse { exchange.setException(throwable); }fallback in each branch.Impact
With the default
synchronous=false, a server error (404/500) now correctly yields aCxfOperationException(carrying status code, response headers and body) — matching the synchronous path behavior.onException(CxfOperationException.class)handlers now work consistently regardless of sync/async mode.Test plan
testAsyncProxyProducerServerErrorMappedToCxfOperationException— calls via the proxy client API with an invalid ID that triggers a 500 on the server; verifies the exchange exception isCxfOperationExceptionwith status 500 (not a rawInternalServerErrorException)CxfRsAsyncProducerTesttests pass🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com