[
https://issues.apache.org/jira/browse/CAMEL-24594?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18110982#comment-18110982
]
James Netherton commented on CAMEL-24594:
-----------------------------------------
Thanks for digging into this. Two things: the reproducer is missing the line
that triggers the behaviour, and I think the framing of the fix (mine included,
in the original description) needs narrowing.
First, the "camel-quarkus specific" conclusion does not hold. The reporter's
processor sets an explicit response code:
{code}
exchange.getMessage().setBody(new Item(42, "demo"));
exchange.getMessage().setHeader(Exchange.HTTP_RESPONSE_CODE, 200);
{code}
With that line present this reproduces on pure core Camel, no camel-quarkus on
the classpath. Standalone project with camel-core, camel-platform-http-vertx,
camel-rest and camel-jackson only, Camel 4.22.0, DefaultCamelContext plus
VertxPlatformHttpServer. Response POJO uses a getter that throws, so it is
independent of java.time, same as your reproducer.
Also worth noting: camel-quarkus has no consumer of its own. Its platform-http
extension is a build item, a build step, a handler class and a recorder, and
the recorder returns new VertxPlatformHttpEngine(). It is core's
VertxPlatformHttpConsumer, VertxPlatformHttpSupport#toHttpResponse and
HttpUtil#determineResponseCode throughout. There is no
QuarkusPlatformHttpConsumer.
Second, and more importantly, I no longer think the response code precedence
should change. I extended the reproducer to compare a post-routing marshal
failure against an ordinary in-route exception:
{code}
marshal failure, no explicit code 500 nothing logged
marshal failure, explicit 200 200 nothing logged
in-route exception, no explicit code 500 ERROR + stacktrace from
DefaultErrorHandler
in-route exception, explicit 200 200 ERROR + stacktrace from
DefaultErrorHandler
in-route exception, explicit 202 202 ERROR + stacktrace from
DefaultErrorHandler
{code}
An explicitly set HTTP_RESPONSE_CODE winning over isFailed() is consistent
behaviour across REST binding and plain routes alike, and people rely on it.
Please disregard the suggestion in my original description about letting a
failed exchange override it. For the same reason I would be cautious about
hardening RestBindingAdvice#marshal to set HTTP_RESPONSE_CODE=500, since that
would make the REST binding inconsistent with every other failure path and
would override a code the user deliberately set.
What the comparison does show is a genuine asymmetry, and it is the logging,
not the status code. When a route throws, the status can be masked to 200 but
DefaultErrorHandler still logs the failure with a stacktrace, so an operator
can see it. When RestBindingAdvice#marshal fails in after(), nothing logs at
any level, in either the 500 or the 200 case. That is the only path where the
failure is genuinely invisible, and it is what made this take a day to diagnose
downstream.
So I would narrow this ticket to: log the marshalling failure in
RestBindingAdvice#marshal rather than only calling exchange.setException(e).
That brings the post-routing binding step in line with how every other failure
in Camel surfaces, and needs no behavioural change to response codes.
The regression guard test would still be worth extending with a variant whose
processor sets HTTP_RESPONSE_CODE=200 before the failing marshal, to pin the
interaction.
Happy to attach the standalone reproducer if useful.
> RestBindingAdvice response marshalling failure is silently swallowed
> --------------------------------------------------------------------
>
> Key: CAMEL-24594
> URL: https://issues.apache.org/jira/browse/CAMEL-24594
> Project: Camel
> Issue Type: Bug
> Reporter: James Netherton
> Priority: Minor
> Fix For: 4.23.0
>
>
> With bindingMode(json) on the REST DSL, if Jackson cannot marshal the
> response body, the caller receives HTTP 200 with an empty body and
> Content-Type: text/plain; charset=utf-8. No error is logged, no exception
> reaches the route's error handler, and the status code stays 200.
> The silent 200 is the actual problem: a serialization failure is
> indistinguishable from a successful empty response, so a client (or a browser
> app) just sees "no data" and everything downstream fails in confusing ways.
> We lost a full day to this after a platform upgrade.
> More details with reproducer can be found here:
> https://github.com/apache/camel-quarkus/issues/9077
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
