[
https://issues.apache.org/jira/browse/CAMEL-25577?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Claus Ibsen updated CAMEL-25577:
--------------------------------
Fix Version/s: 4.24.0
> camel-telegram - when Telegram answers with an error status or an unreadable
> body, the route first continues as if the message was sent and then the
> exchange fails too (completed twice)
> -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-25577
> URL: https://issues.apache.org/jira/browse/CAMEL-25577
> Project: Camel
> Issue Type: Bug
> Components: camel-telegram
> Reporter: shashank
> Assignee: shashank
> Priority: Major
> Fix For: 4.24.0
>
>
> {{TelegramProducer.process}} returns {{false}} (line 84 at main c3829069cb72)
> and leaves the completion of the exchange to
> {{TelegramMessageHandler.sendMessage}} (lines 70-71):
> {code:java}
> apiClient.client().sendAsync(builder.build(),
> HttpResponse.BodyHandlers.ofInputStream())
> .thenApply(telegramAsyncHandler::handleCompressedResponse).get();
> {code}
> {{TelegramAsyncHandler.handleCompressedResponse}} (lines 81-120) throws a
> {{RuntimeCamelException}} for a status outside 2xx (line 108) and for a body
> it cannot parse (line 114), and calls {{callback.done(false)}} in a
> {{finally}} block (lines 115-117). The exception is not set on the exchange;
> it only completes the future exceptionally. So for such a response:
> # the {{finally}} block calls the callback while the exchange has no
> exception: the route continues after the producer as if the message had been
> sent (on the thread of the HTTP client);
> # {{get()}} throws {{ExecutionException}}, {{sendMessage}} rethrows it as
> {{RuntimeException}} (line 76) out of {{process()}};
> # the caller of the producer catches it, sets it on the exchange and calls
> the callback again ({{SendProcessor.sendUsingProducer}}, lines 273-276).
> The exchange is completed twice, once as a success and once as a failure.
> With {{from(...).to("telegram:bots").to("jms:sent")}} a message that Telegram
> rejected (400 for an unknown chat, 403 when the bot was blocked, 429 when
> rate limited, a 502 page of a proxy) is reported to {{jms:sent}} as sent. The
> exception that {{process()}} throws afterwards is a {{RuntimeException}} that
> wraps an {{ExecutionException}} that wraps the {{RuntimeCamelException}};
> without an {{onException}} it is on the exchange that the caller of the route
> gets back.
> The failure arrives after the route has finished, so the error handling of
> the route does not see it properly. Tested on main with a route-scoped
> {{onException(Exception.class).handled(true).to("mock:failed")}} in front of
> the producer: {{mock:after}} gets the exchange, {{mock:failed}} gets nothing,
> and {{RedeliveryErrorHandler}} logs {{Cannot determine current route from
> Exchange with id: ..., will fallback and use first error handler.}} I tested
> this one case only (no redelivery, no context-scoped {{onException}}).
> A request that fails without a response (connection refused) is handled
> correctly on main: {{thenApply}} is skipped, so only the caller completes the
> exchange.
> h3. Reproduction
> New {{TelegramProducerFailureTest}} (6 tests) with the mock Bot API of the
> module ({{TelegramMockRoutes}}); all six fail with the two classes of main:
> * {{errorStatusFailsTheExchangeOnce}} ({{sendMessage}} answers 400) and
> {{unreadableAnswerFailsTheExchangeOnce}} (200 with an HTML body) send through
> {{direct:telegram -> telegram:bots -> mock:after}} and with the producer and
> a callback that records each call: {{[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:
> org.apache.camel.RuntimeCamelException: ... responded: 400...}}.
> * {{errorStatusIsHandledByOnExceptionAndTheRouteDoesNotContinue}}:
> {{[exchanges at the step after the producer] expected: 0 but was: 1}} and
> {{[exchanges at onException] expected: 1 but was: 0}}.
> * {{requestWithoutAnswerFailsTheExchangeOnce}} ({{baseUri}} with a port
> nothing listens on) and {{interruptedCallerFailsTheExchangeOnce}} fail on
> main only because {{process()}} throws instead of completing the exchange
> through the callback; they cover the new handling of these two cases.
> * {{twoSendsAreNotInterleavedAndSetTheResult}} (the mock API holds the first
> answer back for up to a second): the order of the requests and the
> {{MessageResult}} body are the same on main; it fails there only on the
> thread that calls the callback (a thread of the HTTP client on main).
> h3. Proposed fix
> Keep the wait, fix the completion. {{handleCompressedResponse}} no longer
> calls the callback. {{sendMessage}} still waits in {{get()}} on the calling
> thread, then sets the failure on the exchange and calls
> {{callback.done(false)}} once ({{process()}} returns {{false}}, as before):
> for an answer, for a request that failed without one, and when the wait was
> interrupted.
> The blocking {{get()}} stays on purpose. It came with CAMEL-18506; removing
> it would let asynchronous callers (a seda consumer, for example) send
> concurrently, the order of the messages of one caller would no longer be
> kept, and a 429 would become more likely. That would be a separate change.
> Behaviour change, with a 4.24 upgrade-guide entry:
> * a rejected message fails the exchange once and the route stops at the
> producer;
> * the exception of the exchange is the same as before ({{RuntimeException}}
> -> {{ExecutionException}} -> {{RuntimeCamelException}} or {{IOException}});
> on main {{process()}} threw it, now the producer sets it on the exchange;
> * 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()}}.
> Not changed: the request has no timeout (a server that accepts the connection
> and never answers leaves the calling thread waiting, as on main).
> camel-telegram tests with the fix: 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 (request, outcome in
> \{response 2xx, error status, unreadable body, no response\}, the handler
> stage, the blocked caller). For the code of main TLC reports a violation of
> {{NoFalseSuccess}} (3 states: Send, Handle: callback without exception) and
> of {{CompletedOnce}} (4 states: then CallerGet: callback again) for an error
> status and for an unreadable body; a 2xx response and a request without a
> response hold. The model is a sketch of the code, not a proof of the fix; the
> same outcome matrix was then reproduced on the real producer with the test
> above.
> Affected: main (c3829069cb72; its two classes were tested), camel-4.23.x
> (197e70656a41: the same code at the same lines, {{TelegramAsyncHandler}}
> 108-117 and {{TelegramMessageHandler}} 70-77; read, not run), camel-4.22.x
> and camel-4.18.x (the same two lines; read, not run). camel-4.14.x has a
> different class layout and was not checked.
> Duplicate check (2026-10-12): JIRA text "TelegramAsyncHandler" (none),
> "telegram producer callback" (none), "telegram ExecutionException" (none),
> "telegram responded" (11, none about the completion of the exchange; closest
> CAMEL-13903 and CAMEL-18506, the two HTTP client changes); GitHub pull
> requests "telegram producer" in title and "TelegramAsyncHandler": none.
> _Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
