allthingssecurity opened a new pull request, #27761: URL: https://github.com/apache/camel/pull/27761
# Description [CAMEL-25577](https://issues.apache.org/jira/browse/CAMEL-25577) `TelegramProducer.process` returns `false` and `TelegramMessageHandler.sendMessage` completes the exchange. It sent the request with `HttpClient.sendAsync(...).thenApply(handler::handleCompressedResponse).get()`. `TelegramAsyncHandler.handleCompressedResponse` throws for a status outside 2xx and for a body it cannot parse, and called `callback.done(false)` in a `finally` block, without setting the exception on the exchange. So for a 400 (unknown chat), a 403 (bot blocked), a 429 or the HTML error page of a proxy: 1. the callback was called while the exchange had no exception, and the route continued after the producer as if the message had been sent; 2. `get()` threw, `sendMessage` rethrew it out of `process()`, and the caller (`SendProcessor`) set the exception and called the callback a second time. With `from(...).to("telegram:bots").to("mock:after")`, `mock:after` received the exchange of a rejected message, and the exchange failed as well. This change keeps the wait and fixes only the completion. The stage that handles the response no longer calls the callback. The calling thread still waits in `get()`, as on main, then sets the failure on the exchange and calls `callback.done(false)` once (`process()` of the producer returns `false`, as before): for an answer, for a request that failed without one, and when the wait was interrupted (the stage is cancelled then, so it does not write to the completed exchange later). I kept the blocking `get()` on purpose. It has been there since CAMEL-18506 (the change to the JDK HTTP client); without it, callers such as a seda consumer would send concurrently, the order of the messages of one caller would no longer be kept, and more requests at the same time make a 429 more likely. That would be a separate change. What changes for users (4.24 upgrade guide entry; this PR creates the 4.24 guide because it does not exist on main yet): - a rejected message fails the exchange once and the route stops at the producer; - the exception of the exchange is the same as before: the `RuntimeException` that wraps the `ExecutionException`, with the `RuntimeCamelException` (status and body) or the `IOException` as its cause. On main `process()` threw it and the caller set it on the exchange; now the producer sets it; - the steps after the producer run on the calling thread. On main they ran on a thread of the HTTP client after a successful answer, while the calling thread was blocked in `get()`. So no thread blocks that did not block before, and the route no longer runs on a thread of the HTTP client. Not changed: the request has no timeout. A server that accepts the connection and never answers leaves the calling thread waiting, as on main. Possible conflict: CAMEL-25564 (telegram webhook, in progress by a maintainer) may add its own camel-telegram note to the upgrade guide, so the guide may need a rebase, depending on which is merged first. Tests: - New `TelegramProducerFailureTest` (6 tests) in camel-telegram, with the mock Bot API of the module. A 400 answer, a 200 answer with an HTML body, and a request to a port nothing listens on: each is sent through a route (`mock:after` must stay empty, the exchange fails) and with the producer and a callback that records every call (called once, on the calling thread, before `process()` returns, the exchange failed at that time, `process()` does not throw, same exception chain as before). A 400 answer in a route with `onException(...).handled(true)`: `onException` gets the exchange, the step after the producer does not. An interrupted caller: completed once, interrupt flag kept. Two sends through one producer against an API that holds the first answer back for up to a second: the requests arrive one after the other, and the body is the `MessageResult`. - Without the change (main's two classes, same test) all six fail: `[exchanges at the step after the producer] expected: 0 but was: 1`, `[exchange failed when it was completed] expected: true but was: false`, `[thrown by process() of the producer] expected: null but was: java.lang.RuntimeException: java.util.concurrent.ExecutionException: ...`, and for `onException`: `[exchanges at onException] expected: 1 but was: 0`. The test without an answer and the interrupt test fail on main only because `process()` throws instead of completing the exchange through the callback. The test with two sends fails on main only on `[thread that called the callback]`; the order of the requests is the same there. - With the change, camel-telegram tests pass: 99 run, 0 failures, 1 skipped. The integration tests need a bot token and were not run. How it was found: a small TLA+ model of one send (outcome: 2xx, error status, unreadable body, no response) showed a completion as a success for a failed request, and two completions, for an error status and for an unreadable body. The model is only a sketch of the code; I reproduced the same outcome matrix on the real producer with the test above. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built and tested camel-telegram, including the formatter and import-sort plugins. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 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]
