Fix nightly Stress Tests (Chaos TCP): transparent reconnect + robust chaos assertions - #3986
Merged
Merged
Conversation
…chaos assertions The nightly "Stress Tests" / Chaos TCP job had failed on every run since it was introduced (it runs on a schedule and never gated PRs). Root cause and fixes: Product: * ManagedSession.HandleConnectAsync now honours ConfiguredEndpoint.UpdateBeforeConnect (default true) instead of forcing a pre-connect re-discovery for non-OpenAPI endpoints. A caller that pins a URL (proxy / gateway / NAT, or the chaos TcpChaosProxy) can set it false so the channel opens against exactly that URL instead of re-discovering and adopting the server's advertised EndpointUrl. * ManagedTransportChannelLease.SendRequestAsync now transparently resends idempotent requests (Read/Browse/TranslateBrowsePaths/GetEndpoints/FindServers) across a channel-manager reconnect: on a transient transport-drop error it forces a coalesced reconnect (guarded by a new ChannelEntry.ReconnectGeneration so it only triggers for a genuinely undetected drop, never a stale in-flight failure) and resends once the shared channel recovers. Non-idempotent requests surface the error so higher-level recovery (e.g. the subscription engine's own re-create loop) stays in control and requests are never double-applied. Tests / harness: * TcpChaosProxy no longer rethrows the expected OperationAborted raised by an in-flight upstream ConnectAsync during DropAllConnections, which was faulting the awaited connection task and failing the test. * Chaos reconnect assertions are made robust to the inherent non-determinism of reconnect counts under concurrent chaos (coalescing merges drops that land in one recovery window; a stale keep-alive Bad can add one cycle). Survival is asserted via FailureRate, ReconnectFailed == 0, and the coalescing fan-out relationship instead of exact ReconnectStarted/Completed == drop count. * Subscription post-drop recovery window widened (2s -> 8s) to allow re-creating all sessions' subscriptions and monitored items on a loaded agent. Validated locally on net10.0: Chaos TCP passes 6/6 across 8 distinct random seeds; Opc.Ua.Client.Tests ChannelManager/ManagedSession 207/207; and Opc.Ua.Sessions.Tests ChannelManager/Reconnect 139/139 (0 failed).
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3986 +/- ##
==========================================
- Coverage 77.70% 77.36% -0.34%
==========================================
Files 1385 1385
Lines 183248 183293 +45
Branches 31926 31932 +6
==========================================
- Hits 142385 141799 -586
- Misses 29318 30000 +682
+ Partials 11545 11494 -51
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes the nightly Stress Tests / Chaos TCP suite by ensuring sessions truly connect through the chaos proxy (so chaos is actually applied), and by hardening the transparent reconnect logic + test assertions against non-deterministic reconnect counts under concurrent drops.
Changes:
- Honor
ConfiguredEndpoint.UpdateBeforeConnectinManagedSessionso pinned/proxied URLs aren’t overwritten by discovery. - Add idempotent-request retry across transient channel drops in
ManagedTransportChannelLease.SendRequestAsync, guarded by a newChannelEntry.ReconnectGeneration. - Update Chaos TCP stress tests and harness to avoid false failures: swallow expected proxy teardown exceptions, relax reconnect-count assertions, and widen recovery polling window.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/Opc.Ua.Stress.Tests/Channels/Fakes/TcpChaosProxy.cs | Swallows expected connection teardown exceptions so chaos-induced aborts don’t fault the test harness. |
| Tests/Opc.Ua.Stress.Tests/Channels/Chaos/TransparentReconnectChaosTests.cs | Makes reconnect assertions robust to coalescing/non-deterministic reconnect counts; improves failure diagnostics. |
| Tests/Opc.Ua.Stress.Tests/Channels/Chaos/SubscriptionSurvivalChaosTests.cs | Extends post-drop recovery wait to reduce CI flakiness under load. |
| Tests/Opc.Ua.Stress.Tests/Channels/Chaos/BlockAcceptChaosTests.cs | Adjusts reconnect assertions to assert exercised+no-failures rather than exact counts. |
| Stack/Opc.Ua.Core/Stack/Client/Channels/Internal/ManagedTransportChannelLease.cs | Adds bounded retry for idempotent requests across transient drops and coalesced reconnects. |
| Stack/Opc.Ua.Core/Stack/Client/Channels/Internal/ChannelEntry.cs | Introduces ReconnectGeneration counter to detect stale in-flight failures vs. undetected drops. |
| Libraries/Opc.Ua.Client/Session/ManagedSession.cs | Computes updateBeforeConnect based on ConfiguredEndpoint.UpdateBeforeConnect (and OpenAPI exceptions). |
…cp-stress-nightly # Conflicts: # Libraries/Opc.Ua.Client/Session/ManagedSession.cs # Stack/Opc.Ua.Core/Stack/Client/Channels/Internal/ChannelEntry.cs
…l transient error on terminal entry, wrap long stats string
romanett
approved these changes
Jul 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes the nightly Stress Tests / Chaos TCP job (e.g. run 29068846476), which had failed on every run since it was introduced — the workflow runs on a schedule and never gated PRs, so the failures went unnoticed.
Root cause
The Chaos TCP tests point a
ConfiguredEndpointat a localTcpChaosProxyand setUpdateBeforeConnect = falseso the session connects through the proxy. Two problems prevented this from working:ManagedSession.HandleConnectAsyncignoredConfiguredEndpoint.UpdateBeforeConnectand hard-coded a pre-connect re-discovery for all non-OpenAPI endpoints. Re-discovery adopted the server's advertisedEndpointUrl, so the session connected directly to the server, bypassing the proxy — no chaos ever reached the connection. Two tests "passed" vacuously.BadConnectionClosedinstead of transparently recovering.Changes
Product
ManagedSessionnow honoursConfiguredEndpoint.UpdateBeforeConnect(defaulttrue). Setting itfalseopens the channel against exactly the supplied URL (proxy / gateway / NAT / pinned endpoint) without re-discovering.ManagedTransportChannelLease.SendRequestAsynctransparently resends idempotent requests (Read/Browse/TranslateBrowsePaths/GetEndpoints/FindServers) across a channel-manager reconnect. On a transient transport-drop error it forces a coalesced reconnect and resends once the shared channel recovers.ChannelEntry.ReconnectGenerationcounter guards this so a reconnect is only forced for a genuinely undetected drop (same generation, stillReady), never for a stale in-flight failure that arrives after recovery — which previously spawned spurious extra reconnect cycles that tore down healthy channels.Create*,Write,Call,Publish, …) surface the error so higher-level recovery (e.g. the subscription engine's own re-create loop) stays in control; requests are never double-applied.Tests / harness
TcpChaosProxyno longer rethrows the expectedOperationAbortedraised by an in-flight upstreamConnectAsyncduringDropAllConnections(it was faulting the awaited connection task and failing the test).Badcan add one cycle). Survival is asserted viaFailureRate,ReconnectFailed == 0, and the coalescing fan-out relationship rather than exactReconnectStarted/Completed == dropCount.2s → 8s) to allow re-creating all sessions' subscriptions and monitored items on a loaded CI agent.ClientChannelManagerManagedTestscover the new product paths (fast-PR / codecov, since the nightly ChaosTCP job does not run in the fast pipeline): idempotent transient-drop retry, non-idempotent no-retry, non-transient no-retry, andChannelEntry.ReconnectGenerationincrement.Validation (local, net10.0)
338475986and1999999999).Opc.Ua.Client.TestsChannelManager/ManagedSession: 256/256 (incl. the 4 new tests).Opc.Ua.Sessions.TestsChannelManager/Reconnect: 139/139.Notes
master(redundancy PR OPC UA Part 4 6.6 Redundancy (server + client) with opt-in distributed high-availability #3918).Opc.Ua.PubSub.Testsin CI —UdpLoopbackActionResponderAnswersRequesterAsyncandUdpLoopbackDiscoveryPublisherAnswersSubscriberRequests— are unrelated to this PR (UDP-loopback request/response tests in the PubSub library, associated with OPC UA Part 4 6.6 Redundancy (server + client) with opt-in distributed high-availability #3918) and are timing/environment-flaky. They are out of scope for this change.Related Issues
.github/workflows/stress-test.yml).Checklist