[
https://issues.apache.org/jira/browse/CAMEL-24789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18116264#comment-18116264
]
Andrea Cosentino commented on CAMEL-24789:
------------------------------------------
PR opened: https://github.com/apache/camel/pull/26541
The empty-peer case now completes and returns true, matching the null-body
branch, and says at WARN that the message was not delivered. Verified by
reverting the change: the new test then fails on the process() return value.
Note for whoever merges: both this and
https://github.com/apache/camel/pull/26533 add assertj-core to the
camel-vertx-websocket test dependencies, so the second one to merge will need
that hunk dropped.
_Claude Code on behalf of oscerd_
> camel-vertx-websocket: the producer completes an exchange both synchronously
> and asynchronously when there is no peer
> ---------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-24789
> URL: https://issues.apache.org/jira/browse/CAMEL-24789
> Project: Camel
> Issue Type: Bug
> Components: camel-vertx-websocket
> Reporter: Andrea Cosentino
> Assignee: Andrea Cosentino
> Priority: Minor
>
> h3. Summary
> {{VertxWebsocketProducer.process}} tells its callback the exchange finished
> synchronously and simultaneously tells the caller it will finish
> asynchronously, whenever there is no peer to send to.
> h3. Detail
> {{VertxWebsocketProducer}} lines 66-99:
> {code:java}
> if (connectedPeers.isEmpty()) {
> callback.done(true);
> }
> // Send message to each peer then record and process the results
> asynchronously
> connectedPeers.forEach((connectionKey, webSocket) -> { ... });
> return false;
> {code}
> With no connected peers the {{forEach}} does nothing, so the method has
> already completed the callback with {{doneSync=true}} and then returns
> {{false}}. {{AsyncProcessor#process}} documents the return value as
> {{doneSync}} - "true to continue execution synchronously, false to continue
> being executed asynchronously" - so the two signals contradict each other.
> The null-body branch eight lines earlier in the same method gets this right:
> {code:java}
> if (message == null) {
> // Nothing to do for a null body
> callback.done(true);
> return true;
> }
> {code}
> The path is easy to reach: {{sendToAll}} with nothing connected yet, or a
> {{CamelVertxWebsocket.connectionKey}} that matches no current peer.
> h3. Related silence on the same path
> Line 123-125 filters the requested connection keys against the known peers:
> {code:java}
> Stream.of(connectionKey.split(","))
> .filter(peers::containsKey)
> .forEach(key -> connectedPeers.put(key,
> endpoint.findPeerForConnectionKey(key)));
> {code}
> A key that matches nothing is dropped without a word, and the exchange
> completes as if it had been delivered. One line further on, a peer whose
> websocket turns out to be null does produce a warning, so the quiet case is
> the inconsistent one.
> Found by a source audit of {{components/camel-vertx}} at be3dd651866f.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
