davsclaus opened a new pull request, #29103: URL: https://github.com/apache/camel/pull/29103
Fixes [CAMEL-26137](https://issues.apache.org/jira/browse/CAMEL-26137). ### 1. A List reply **Was wrong:** `DefaultNettyHttpBinding.toNettyResponse` converts the reply body with `getBody(byte[].class)` as a last resort before the `String` fallback. For a `List` this conversion throws a `TypeConversionException` ("argument type mismatch"), so the `String` fallback was never reached. Since CAMEL-26504 the consumer answers 500 for that; before, no response was sent at all. **Changed:** a `TypeConversionException` from the `byte[]` conversion now falls through to the existing `String` fallback, so a `List` is written as its `toString()`, as a `Map` or a POJO already is. A body that cannot be converted to a `String` either still gets the 500 of CAMEL-26504 (`NettyHttpResponseCannotBeBuiltTest` still passes). The `consumerReply` `@BodyTypes` text no longer says "a List is not written at all". ### 2. Sending the request body back with disableStreamCache=true **Was wrong:** with `disableStreamCache=true` the body is the request `ByteBuf`, which an on-completion releases when the exchange is done. A route that replies with that body unchanged (an echo) had the response write the same buffer, and writing the response releases it too. So the buffer was released once too often, the write failed with `IllegalReferenceCountException: refCnt: 0, decrement: 1`, and no response was sent. **Changed:** when the reply `ByteBuf` is the content of the exchange's request, the response writes a `retainedDuplicate()` of it. Any other `ByteBuf` body (one the route created, or the response of a netty-http producer in proxy mode) is written as before, so nothing new leaks. ### 3. sync=false on the producer The part reported in the issue (the response set on the exchange after it continued) was fixed in the shared `ClientChannelHandler` by CAMEL-26136, which `HttpClientChannelHandler` inherits. While testing it with netty-http, I found the exchange still ended with an empty message for `InOut`. In `NettyProducer` the `sync=false` branch read the close header with `exchange.getOut()`, which creates an empty out message on an `InOut` exchange. It now reads `exchange.getMessage()`. This is in camel-netty, so it applies to `netty` and `netty-http`. camel-mina had the same bug, fixed in 4.24. A side effect: for an `InOut` exchange with `sync=false`, a `CamelNettyCloseChannelWhenComplete` header on the message is now honoured. Before, the producer only looked at the new empty out message, so it never found the header. `InOnly` is unchanged. ### Tests (all failed before the fix) - `NettyHttpReplyBodyTest` (netty-http): a `List` reply returns 200 `[a, b]` (was 500). The echo with `disableStreamCache=true` works, also when the reply comes from another thread (was no response). The test also checks that the request buffer still reaches `refCnt` 0, and the leak detection of `BaseNettyTestSupport` passes. - `NettyHttpProducerSyncFalseTest` (netty-http): an `InOut` request with `sync=false` keeps its body (was an empty message). - `NettyProducerSyncFalseReplyTest#anInOutExchangeKeepsItsMessage` (camel-netty): the same for `netty:tcp`. - Full module suites: camel-netty 152 tests (9 skipped), camel-netty-http 371 tests (10 skipped). All pass with `-Psourcecheck`. ### Docs and generated files - netty-http: `@BodyTypes` text regenerated into `netty-http.json`, and copied to the catalog. - netty: `netty-component.adoc` says the message stays as it was with `sync=false`, also for `InOut`. The catalog copy is updated. - Upgrade guide 4.24: one sentence each in the existing `camel-netty - replies with the default codec, useByteBuf and sync=false` and `camel-netty-http - a response that cannot be built` sections. ### Not changed - `toNettyRequest` (the producer) still fails a `List` body. Its `@BodyTypes` text documents this ("A Map, a List or a POJO fails: marshal it first"), and the failure shows up on the exchange. - `NettyProducer` has the same `exchange.getOut()` close-header check in the `reuseChannel` on-completion. It runs after routing, so I left it out of this PR. _Claude Code on behalf of Claus Ibsen (davsclaus)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
