Serdar Gökay created CAMEL-25694:
------------------------------------
Summary: camel-milo - awaited write/call service failures are
reported as success
Key: CAMEL-25694
URL: https://issues.apache.org/jira/browse/CAMEL-25694
Project: Camel
Issue Type: Bug
Components: camel-milo
Affects Versions: 4.22.1
Environment: Camel 4.22.1, Eclipse Milo 1.1.6, OpenJDK 25.0.4, Maven
3.9.16; macOS arm64. Real loopback OPC UA test servers: Milo (attached
reproducer) and independent asyncua 1.1.8 / Python 3.12 with a TCP relay. Also
reproduced using compiled camel-milo source at
9c6d4cfc2164f36f2f2b10eb4e09b8956e2925d9 with cached 4.24.0-SNAPSHOT
dependencies. No physical PLC or production deployment was used.
Reporter: Serdar Gökay
Attachments: camel-milo-awaited-failure-reproducer.zip
h3. Problem
An awaited milo-client write can fail at the OPC UA service/transport layer
while the Camel exchange completes successfully with a null body. The success
continuation runs and configured redelivery, onException and dead-letter
handling are skipped.
Reproduced on released Camel 4.22.1 / Eclipse Milo 1.1.6 with a real TCP OPC UA
server and a separate OPC UA readback client. The same failure was reproduced
with the camel-milo source compiled at 9c6d4cfc2164f36f2f2b10eb4e09b8956e2925d9
(4.24.0-SNAPSHOT, PR #27910 revision). This predates the subscription watchdog
work.
Awaiting is enabled with message header {{CamelMiloAwait=true}} or endpoint
option {{defaultAwaitWrites=true}}. Both reproduce the issue.
h3. Reproduction
The attached standalone Maven project starts its own loopback TCP OPC UA
server. It does not mock a client, connection, future or transport, and does
not patch Camel classes.
{code:bash}
mvn -B -ntp test -Dtest=AwaitedWriteContractTest
{code}
# Establish the connection and successfully write 41; assert a Good StatusCode
and independent readback of 41.
# Ask Camel to write 99. The real server responds with a Bad_Timeout
ServiceFault before applying the write.
# Read the node through the independent client: it is still 41.
# Inspect the Camel exchange and route counters:
{noformat}
attempted=99 readback=41 serverValue=41
writes=1 redeliveries=0 downstream=1
failed=false exception=null body=null
{noformat}
The test requires the service failure to reach the exchange and fails for that
assertion on 4.22.1. The native Milo client against the same server/fault
throws UaException with the matching status.
The attachment also contains 15 behavior/control tests ({{mvn test
'-Dtest=*EvidenceTest'}}). These intentionally characterize current behavior,
including the defect; passing them does not mean the bug is fixed. They cover
both await settings, Bad_Timeout and Bad_SessionClosed service faults, normal
bad per-node status, retry/DLQ/onException controls, method calls and gated
fire-and-forget behavior. Bad_SessionClosed is a controlled service-fault
response rather than an actual session-expiry simulation.
An independent asyncua server behind a TCP relay also reproduced the failure
when request traffic was dropped or an established connection was closed.
Readback remained unchanged while Camel continued successfully. Dropping only
the reply instead allowed the write to apply, but Camel still incorrectly
returned a successful null result. These are distinct outcome cases.
h3. Expected behavior
When the awaited operation completes exceptionally, preserve that exceptional
completion so MiloClientProducer fails the exchange and Camel error handling
can run. Preserve existing successful write StatusCode results, ordinary
per-node bad StatusCode results, and explicitly non-awaited behavior.
An already-disconnected manager has a separate exceptional {{No connected}}
path. Merely starting against an absent server can miss this defect; the
reproduction first establishes a working connection.
h3. Cause and proposed direction
In {{SubscriptionManager.write}}, {{handleAsync}} calls
{{handleConnectionFailure(error)}} and then returns {{status}}, which is null
after exceptional completion. That converts the failed future into normal
completion. {{MiloClientProducer}} already sets the exchange exception when it
receives an exceptional future.
Proposed write change, retaining the executor and recovery callback:
{code:java}
return this.connected.write(nodeId, value).whenCompleteAsync((status, error) ->
{
if (error != null) {
handleConnectionFailure(error);
}
}, this.executor);
{code}
This is a proposed direction, not an applied or verified fix. Preserve
asynchronous dispatch and recovery behavior. The result/exception should remain
the source future's completion.
{{call}} has the same failure-propagation defect, also reproduced. It
additionally returns null even on success. Changing to {{whenCompleteAsync}}
would expose successful CallMethodResult bodies too; either explicitly cover
that restoration or retain the historical success-null result after preserving
completion. Ordinary bad per-node write statuses remain a separate
status-as-body contract, as introduced by CAMEL-18618.
Propagating a timeout restores error handling; it does not prove that the
device did not act or make arbitrary command retries exactly-once. Read/browse
have similar handlers but were outside this reproduction's scope.
Pinned released source: [SubscriptionManager
write/call|https://github.com/apache/camel/blob/c056bef67cdfeffdc0ad7255e5e052cab71f2921/components/camel-milo/src/main/java/org/apache/camel/component/milo/client/internal/SubscriptionManager.java#L971-L1012]
and
[MiloClientProducer|https://github.com/apache/camel/blob/c056bef67cdfeffdc0ad7255e5e052cab71f2921/components/camel-milo/src/main/java/org/apache/camel/component/milo/client/MiloClientProducer.java#L91-L100].
_Report and reproduction prepared with Codex on behalf of GitHub user
etzel-mes._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)