diff --git a/clients/dotnet/README.md b/clients/dotnet/README.md index 136ea22..0f4906f 100644 --- a/clients/dotnet/README.md +++ b/clients/dotnet/README.md @@ -134,8 +134,8 @@ dotnet run --project clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli -- advise --s dotnet run --project clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli -- write --session-id --server-handle 1 --item-handle 1 --type int32 --value 123 --json dotnet run --project clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli -- write2 --session-id --server-handle 1 --item-handle 1 --type int32 --value 123 --timestamp 2026-01-01T00:00:00Z --json dotnet run --project clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli -- stream-events --session-id --max-events 1 --json -dotnet run --project clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli -- stream-alarms --session-id --max-messages 1 --json -dotnet run --project clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli -- acknowledge-alarm --session-id --alarm-reference "\\Galaxy\Area001.Pump001.PumpFault" --json +dotnet run --project clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli -- stream-alarms --filter-prefix Area001 --max-events 1 --json +dotnet run --project clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli -- acknowledge-alarm --reference "\\Galaxy\Area001.Pump001.PumpFault" --comment "ack from cli" --operator operator1 --json dotnet run --project clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli -- smoke --endpoint http://localhost:5000 --api-key-env MXGATEWAY_API_KEY --item Area001.Pump001.Speed --json ``` diff --git a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs index 96f986e..8f2ff53 100644 --- a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs +++ b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs @@ -487,7 +487,7 @@ public static class MxGatewayClientCli ReadBulkCommand command = new() { ServerHandle = arguments.GetInt32("server-handle"), - TimeoutMs = (uint)arguments.GetInt32("timeout-ms", 0), + TimeoutMs = ParseTimeoutMs(arguments, defaultValue: 0), }; command.TagAddresses.Add(ParseStringList(arguments.GetRequired("items"))); @@ -692,6 +692,49 @@ public static class MxGatewayClientCli } } + /// + /// Parses the optional --timeout-ms argument as a non-negative + /// unsigned millisecond count. Mirrors the SDK-side (uint)Math.Min + /// guard on MxGatewaySession.ReadBulkAsync: a negative value + /// (e.g. -1, an easy copy-paste mistake for "unbounded") is + /// rejected loudly rather than silently wrapped to ~49.7 days, + /// which would park one worker thread per pending tag for hours. + /// Resolves Client.Dotnet-021. + /// + private static uint ParseTimeoutMs(CliArguments arguments, int defaultValue) + { + int raw = arguments.GetInt32("timeout-ms", defaultValue); + if (raw < 0) + { + throw new ArgumentException( + "--timeout-ms must be a non-negative integer (use 0 for the gateway default)."); + } + + return (uint)raw; + } + + /// + /// Extracts the ServerHandle from a Register reply, throwing a + /// descriptive when the typed + /// Register payload is absent on an otherwise-successful reply. + /// The typed sub-message is the contract for the Register command, so + /// its absence must not silently fall through to + /// ReturnValue.Int32Value (which would be 0 for an empty + /// reply, driving the rest of the bench against an invalid handle). + /// Resolves Client.Dotnet-019. + /// + private static int RequireRegisterServerHandle(MxCommandReply reply, string sessionId) + { + if (reply.Register is null) + { + throw new MxGatewayException( + $"Gateway reply for Register on session '{sessionId}' (correlation '{reply.CorrelationId}') " + + "succeeded but is missing the typed 'register' payload required to read ServerHandle."); + } + + return reply.Register.ServerHandle; + } + /// /// Cross-language stress benchmark for ReadBulk. Opens its own session, /// subscribes to N tags so the worker's MxAccessValueCache populates from @@ -712,7 +755,7 @@ public static class MxGatewayClientCli int tagStart = arguments.GetInt32("tag-start", 1); string tagPrefix = arguments.GetOptional("tag-prefix") ?? "TestMachine_"; string tagAttribute = arguments.GetOptional("tag-attribute") ?? "TestChangingInt"; - uint timeoutMs = (uint)arguments.GetInt32("timeout-ms", 1500); + uint timeoutMs = ParseTimeoutMs(arguments, defaultValue: 1500); string clientName = arguments.GetOptional("client-name") ?? "mxgw-dotnet-bench"; string[] tags = new string[bulkSize]; @@ -742,7 +785,7 @@ public static class MxGatewayClientCli }), cancellationToken) .ConfigureAwait(false); - int serverHandle = registerReply.Register?.ServerHandle ?? registerReply.ReturnValue.Int32Value; + int serverHandle = RequireRegisterServerHandle(registerReply, sessionId); SubscribeBulkCommand subscribe = new() { ServerHandle = serverHandle }; subscribe.TagAddresses.Add(tags); @@ -801,8 +844,13 @@ public static class MxGatewayClientCli .ConfigureAwait(false); sw.Stop(); } - catch + catch (Exception ex) when (ex is not OperationCanceledException) { + // Client.Dotnet-020: never swallow OperationCanceledException + // here. A bare `catch` would let Ctrl+C / parent CTS / + // wall-clock timeouts keep spinning until --duration-seconds + // elapsed, burning CPU and skewing the p99/max latency numbers + // with hundreds of immediate-OCE iterations. sw.Stop(); failedCalls++; latencyMillis.Add(sw.Elapsed.TotalMilliseconds); diff --git a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientCliTests.cs b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientCliTests.cs index fa790c0..4cf8e46 100644 --- a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientCliTests.cs +++ b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientCliTests.cs @@ -509,6 +509,354 @@ public sealed class MxGatewayClientCliTests Assert.Contains("gateway-protocol=", text); } + /// + /// Client.Dotnet-018: the README CLI examples for the alarm subcommands at + /// `clients/dotnet/README.md` must drive cleanly through the production + /// CLI argument parser. The previous text used non-existent flags + /// (`--session-id`, `--max-messages`, `--alarm-reference`) that would + /// fail with "Unknown command" / "Missing required option --reference". + /// Each documented example is extracted from the README, parsed via the + /// production , and asserted + /// against exit code 0. + /// + [Theory] + [InlineData("stream-alarms")] + [InlineData("acknowledge-alarm")] + public async Task RunAsync_ReadmeExamples_ForAlarmCommands_ParseSuccessfully(string command) + { + string readme = LocateClientReadme(); + string[] commandLine = ExtractReadmeCommandLine(readme, command); + // The documented examples do not include --api-key (the README assumes + // the env var path documented elsewhere). Inject an API key via the + // standard env var so CreateOptions succeeds and the parser fully + // exercises the documented flag shape. + string? previousKey = Environment.GetEnvironmentVariable("MXGATEWAY_API_KEY"); + Environment.SetEnvironmentVariable("MXGATEWAY_API_KEY", "test-api-key"); + + try + { + using var output = new StringWriter(); + using var error = new StringWriter(); + FakeCliClient fakeClient = new(); + fakeClient.AlarmFeedMessages.Add(new AlarmFeedMessage + { + ActiveAlarm = new ActiveAlarmSnapshot { AlarmFullReference = "fixture" }, + }); + fakeClient.AcknowledgeAlarmReplies.Enqueue(new AcknowledgeAlarmReply + { + CorrelationId = "ack-fixture", + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + }); + + int exitCode = await MxGatewayClientCli.RunAsync( + commandLine, + output, + error, + _ => fakeClient); + + Assert.True( + exitCode == 0, + $"README example for '{command}' exited {exitCode}; stderr=<<{error}>>"); + Assert.DoesNotContain("Unknown command", error.ToString()); + Assert.DoesNotContain("Missing required option", error.ToString()); + } + finally + { + Environment.SetEnvironmentVariable("MXGATEWAY_API_KEY", previousKey); + } + } + + /// + /// Client.Dotnet-019: `BenchReadBulkAsync` previously fell back to + /// reply.ReturnValue.Int32Value when the register reply had no + /// typed Register payload, silently driving the rest of the bench + /// against a zero server handle. The fix must fail loudly with a + /// descriptive . + /// + [Fact] + public async Task RunAsync_BenchReadBulk_WhenRegisterReplyMissingTypedPayload_FailsLoudly() + { + using var output = new StringWriter(); + using var error = new StringWriter(); + FakeCliClient fakeClient = new(); + // Successful protocol + MX status but no typed `Register` payload. + // Before the Client.Dotnet-019 fix this silently became serverHandle=0 + // and the bench proceeded through SubscribeBulk / warmup / steady-state + // against an invalid handle, producing a misleading zero-result summary. + fakeClient.InvokeReplies.Enqueue(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.Register, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + }); + + int exitCode = await MxGatewayClientCli.RunAsync( + [ + "bench-read-bulk", + "--endpoint", + "http://localhost:5000", + "--api-key", + "test-api-key", + "--duration-seconds", + "1", + "--warmup-seconds", + "0", + "--bulk-size", + "1", + ], + output, + error, + _ => fakeClient); + + Assert.Equal(1, exitCode); + // Descriptive message that names the missing typed payload. + string err = error.ToString(); + Assert.Contains("Register", err); + // The bench must not produce any aggregate stats JSON. + Assert.DoesNotContain("bench-read-bulk", output.ToString()); + } + + /// + /// Client.Dotnet-020: the steady-state loop in `BenchReadBulkAsync` had a + /// bare `catch { failedCalls++; continue; }` that swallowed + /// , so token-driven cancellation + /// kept spinning until --duration-seconds elapsed. After the fix + /// the bench must exit promptly when the supplied token cancels. + /// + [Fact] + public async Task RunAsync_BenchReadBulk_WhenSteadyStateLoopReceivesCancellation_ExitsPromptly() + { + using var output = new StringWriter(); + using var error = new StringWriter(); + + int invokeCount = 0; + FakeCliClient fakeClient = new() + { + InvokeHandler = (request, ct) => + { + int n = Interlocked.Increment(ref invokeCount); + + // Reply 1 = Register (success with typed payload). + if (request.Command.Kind == MxCommandKind.Register) + { + return Task.FromResult(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.Register, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + Register = new RegisterReply { ServerHandle = 1 }, + }); + } + + // Reply 2 = SubscribeBulk (success). + if (request.Command.Kind == MxCommandKind.SubscribeBulk) + { + var subscribeReply = new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.SubscribeBulk, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + SubscribeBulk = new BulkSubscribeReply(), + }; + return Task.FromResult(subscribeReply); + } + + // ReadBulk reply 1 = success (so the steady-state loop enters + // and starts iterating). Reply 2+ = simulated cancellation. + if (request.Command.Kind == MxCommandKind.ReadBulk && n <= 3) + { + return Task.FromResult(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.ReadBulk, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + ReadBulk = new BulkReadReply(), + }); + } + + // From here on every ReadBulk throws OCE — the steady-state + // loop must exit promptly rather than spinning until + // --duration-seconds elapses. + throw new OperationCanceledException(); + }, + }; + + var sw = System.Diagnostics.Stopwatch.StartNew(); + await Assert.ThrowsAsync(async () => + await MxGatewayClientCli.RunAsync( + [ + "bench-read-bulk", + "--endpoint", + "http://localhost:5000", + "--api-key", + "test-api-key", + "--duration-seconds", + "30", + "--warmup-seconds", + "0", + "--bulk-size", + "1", + ], + output, + error, + _ => fakeClient)); + sw.Stop(); + + // Without the fix the loop swallows OCE and continues until the 30 s + // steady-state deadline expires. With the fix it exits as soon as OCE + // surfaces. Generous 10 s ceiling to keep the test stable under load. + Assert.True( + sw.Elapsed < TimeSpan.FromSeconds(10), + $"Bench did not exit promptly on cancellation; took {sw.Elapsed}."); + } + + /// + /// Client.Dotnet-021: both `ReadBulkAsync` and `BenchReadBulkAsync` cast + /// the user-supplied --timeout-ms to without + /// bounds checking, so a negative value (e.g. -1) silently wraps + /// to ~49.7 days. The fix must reject negatives with a clear error. + /// + [Theory] + [InlineData("read-bulk")] + [InlineData("bench-read-bulk")] + public async Task RunAsync_TimeoutMs_NegativeValue_RejectsWithClearError(string command) + { + using var output = new StringWriter(); + using var error = new StringWriter(); + FakeCliClient fakeClient = new(); + + string[] args = command is "read-bulk" + ? [ + "read-bulk", + "--endpoint", + "http://localhost:5000", + "--api-key", + "test-api-key", + "--session-id", + "session-fixture", + "--server-handle", + "1", + "--items", + "Area001.Pump001.Speed", + "--timeout-ms", + "-1", + ] + : [ + "bench-read-bulk", + "--endpoint", + "http://localhost:5000", + "--api-key", + "test-api-key", + "--duration-seconds", + "1", + "--warmup-seconds", + "0", + "--bulk-size", + "1", + "--timeout-ms", + "-1", + ]; + + int exitCode = await MxGatewayClientCli.RunAsync( + args, + output, + error, + _ => fakeClient); + + Assert.NotEqual(0, exitCode); + string err = error.ToString(); + Assert.Contains("timeout-ms", err); + Assert.Contains("non-negative", err); + } + + /// + /// Locates the .NET client README by walking up from the test assembly's + /// base directory until clients/dotnet/README.md is found. Keeps + /// the regression test independent of the current working directory. + /// + private static string LocateClientReadme() + { + string? directory = AppContext.BaseDirectory; + while (!string.IsNullOrEmpty(directory)) + { + string candidate = Path.Combine(directory, "clients", "dotnet", "README.md"); + if (File.Exists(candidate)) + { + return candidate; + } + + directory = Path.GetDirectoryName(directory); + } + + throw new FileNotFoundException("clients/dotnet/README.md not found above test assembly base directory."); + } + + /// + /// Extracts the documented CLI invocation for the requested subcommand + /// from the README, returning only the arguments after the + /// mxgw-dotnet-equivalent prefix so they can be passed straight + /// to . + /// + private static string[] ExtractReadmeCommandLine(string readmePath, string command) + { + string[] lines = File.ReadAllLines(readmePath); + // Look for the documented `dotnet run ... -- ...` line. + foreach (string line in lines) + { + int dashes = line.IndexOf("-- " + command, StringComparison.Ordinal); + if (dashes < 0) + { + continue; + } + + string after = line[(dashes + 3)..].Trim(); + // Tokenize by whitespace, respecting "..." quoted segments. + return TokenizeCommandLine(after); + } + + throw new InvalidOperationException( + $"README at '{readmePath}' has no documented example for subcommand '{command}'."); + } + + /// + /// Splits a single command-line string into argv tokens, honouring + /// double-quoted segments so paths with embedded spaces survive intact. + /// + private static string[] TokenizeCommandLine(string input) + { + var tokens = new List(); + var current = new System.Text.StringBuilder(); + bool inQuotes = false; + + foreach (char ch in input) + { + if (ch == '"') + { + inQuotes = !inQuotes; + continue; + } + + if (!inQuotes && char.IsWhiteSpace(ch)) + { + if (current.Length > 0) + { + tokens.Add(current.ToString()); + current.Clear(); + } + continue; + } + + current.Append(ch); + } + + if (current.Length > 0) + { + tokens.Add(current.ToString()); + } + + return tokens.ToArray(); + } + /// Fake CLI client for testing. private sealed class FakeCliClient : IMxGatewayCliClient { @@ -527,6 +875,9 @@ public sealed class MxGatewayClientCliTests /// Exception to throw on invoke, if any. public Exception? InvokeFailure { get; init; } + /// Optional per-call handler that overrides queue-based behaviour. + public Func>? InvokeHandler { get; init; } + /// public ValueTask DisposeAsync() { @@ -572,6 +923,11 @@ public sealed class MxGatewayClientCliTests throw InvokeFailure; } + if (InvokeHandler is not null) + { + return InvokeHandler(request, cancellationToken); + } + return Task.FromResult(InvokeReplies.Dequeue()); } diff --git a/code-reviews/Client.Dotnet/findings.md b/code-reviews/Client.Dotnet/findings.md index bac9b6a..8dc7543 100644 --- a/code-reviews/Client.Dotnet/findings.md +++ b/code-reviews/Client.Dotnet/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-05-24 | | Commit reviewed | `42b0037` | | Status | Re-reviewed | -| Open findings | 4 | +| Open findings | 0 | ## Checklist coverage @@ -390,7 +390,7 @@ Re-review pass at `42b0037`. Diff against `d692232` consists of four commits: | Severity | Medium | | Category | Documentation & comments | | Location | `clients/dotnet/README.md:137-138` | -| Status | Open | +| Status | Resolved | **Description:** The README example block for the two new alarm CLI subcommands shipped in commit `11cc671` shows: @@ -412,6 +412,8 @@ mxgw-dotnet acknowledge-alarm --reference "\\Galaxy\Area001.Pump001.PumpFault" - A quick sanity check would be to drive each example through the test harness's `MxGatewayClientCli.RunAsync` shape and confirm exit 0 — copy/paste safety on the documented examples is the only realistic safeguard. +**Resolution:** 2026-05-24 — Confirmed against source: the README `dotnet run -- stream-alarms` example used `--session-id --max-messages 1`, neither of which the CLI accepts (the production parser routes through `--filter-prefix` / `--max-events`), and `acknowledge-alarm` used `--session-id --alarm-reference ` against a session-less central alarm monitor that actually consumes `--reference`. Replaced both README example lines with the parser-correct shape: `stream-alarms --filter-prefix Area001 --max-events 1 --json` and `acknowledge-alarm --reference "\\Galaxy\Area001.Pump001.PumpFault" --comment "ack from cli" --operator operator1 --json`. Regression test `MxGatewayClientCliTests.RunAsync_ReadmeExamples_ForAlarmCommands_ParseSuccessfully` (xUnit `[Theory]` over `stream-alarms` and `acknowledge-alarm`) locates `clients/dotnet/README.md`, extracts the documented CLI command line for each subcommand, tokenizes it (preserving quoted segments), and drives it through the production `MxGatewayClientCli.RunAsync` against a `FakeCliClient`; the test asserts exit code 0 and that stderr contains neither "Unknown command" nor "Missing required option". Verified red against the original README text (both theory rows failed with exit code 1) and green after the README update, so any future documentation drift between README examples and the actual parser shape is caught at test time. + ### Client.Dotnet-019 | Field | Value | @@ -419,7 +421,7 @@ A quick sanity check would be to drive each example through the test harness's ` | Severity | Low | | Category | Correctness & logic bugs | | Location | `clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs:745` | -| Status | Open | +| Status | Resolved | **Description:** Client.Dotnet-005 / 010 documented (and recorded as resolved) the silent register-handle fallback pattern `reply.Register?.ServerHandle ?? reply.ReturnValue.Int32Value`, where a successful protocol+MX-status reply missing its typed `register` oneof case falls through to `ReturnValue.Int32Value` and silently yields `0` for the handle. The new `BenchReadBulkAsync` handler introduced in commit `b3ae200` reinstates exactly that pattern at line 745: @@ -431,6 +433,8 @@ The bench then drives the rest of the run — `SubscribeBulk`, warmup `ReadBulk` **Recommendation:** Replace the fallback with an explicit null-check on `registerReply.Register` that throws `MxGatewayException` with the missing-payload context (kind = `Register`, session id, correlation id) — the same shape Client.Dotnet-005 prescribes. If the upstream SDK helpers in `MxGatewaySession` are restored to throw, route the bench through `MxGatewaySession.RegisterAsync` instead so the CLI inherits the SDK's protection. +**Resolution:** 2026-05-24 — Confirmed against source: `BenchReadBulkAsync` at line 745 carried the silent fallback `registerReply.Register?.ServerHandle ?? registerReply.ReturnValue.Int32Value`, so a successful protocol+MX-status reply missing its typed `register` payload would yield ServerHandle=0 and the bench would drive the rest of SubscribeBulk / warmup / steady-state ReadBulk / UnsubscribeBulk against an invalid handle. Added a private `RequireRegisterServerHandle(MxCommandReply reply, string sessionId)` helper that throws a descriptive `MxGatewayException` naming the missing typed payload, the session id, and the correlation id, then replaced the fallback site with `int serverHandle = RequireRegisterServerHandle(registerReply, sessionId);`. The bench's outer `RunCoreAsync` catch then surfaces the throw as exit code 1 plus the descriptive message on stderr, so the failure mode is loud rather than a wall of zero-result stats. Regression test `MxGatewayClientCliTests.RunAsync_BenchReadBulk_WhenRegisterReplyMissingTypedPayload_FailsLoudly` enqueues an `Ok` MX command reply with no typed `Register` payload, runs `bench-read-bulk` against the fake, and asserts exit code 1, that stderr names the `Register` payload, and that no `bench-read-bulk` stats JSON was emitted on stdout. Verified red against the original fallback (an NRE bubbled up later in the run rather than the descriptive throw) and green after the helper landed. + ### Client.Dotnet-020 | Field | Value | @@ -438,7 +442,7 @@ The bench then drives the rest of the run — `SubscribeBulk`, warmup `ReadBulk` | Severity | Low | | Category | Error handling & resilience | | Location | `clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs:792-810`, `clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs:774-780` | -| Status | Open | +| Status | Resolved | **Description:** `BenchReadBulkAsync`'s steady-state `while (DateTime.UtcNow < steadyDeadline)` loop wraps each `client.InvokeAsync(...)` in a bare `catch`: @@ -466,6 +470,8 @@ The warmup loop above (lines 774-780) has no catch at all, so a warmup-time OCE **Recommendation:** Replace the bare `catch` with `catch (Exception) when (!cancellationToken.IsCancellationRequested)`, or split into `catch (OperationCanceledException) { throw; } catch (Exception) { failedCalls++; ... continue; }`. The first form is the smallest diff and matches the pattern used elsewhere in the CLI. Add a regression test that runs `bench-read-bulk` with a `--duration-seconds 10` budget against a fake that throws on every `InvokeAsync`, cancels the supplied token after 100 ms, and asserts the run exits in well under 10 s. The wider precedent — Client.Dotnet-016's `BenchStreamEventsAsync` cancellation hardening — should already cover the shape of the test fixture. +**Resolution:** 2026-05-24 — Confirmed against source: the steady-state loop wrapped `client.InvokeAsync` in a bare `catch { sw.Stop(); failedCalls++; latencyMillis.Add(...); continue; }` with no type filter, so an `OperationCanceledException` thrown from a cancelled token (Ctrl+C, parent CTS, or the wall-clock budget) was swallowed and the loop spun until `DateTime.UtcNow >= steadyDeadline`. Replaced the bare clause with `catch (Exception ex) when (ex is not OperationCanceledException)`, so OCE now propagates out of the bench and unwinds through the outer `RunCoreAsync` (which intentionally lets OCE escape). Inline comment names the finding and the reason the filter exists. Regression test `MxGatewayClientCliTests.RunAsync_BenchReadBulk_WhenSteadyStateLoopReceivesCancellation_ExitsPromptly` configures the fake CLI client so Register and SubscribeBulk succeed, the first three ReadBulk calls succeed (the loop enters the steady-state body), then every subsequent ReadBulk throws `OperationCanceledException`; the test runs `bench-read-bulk --duration-seconds 30` and asserts the call throws OCE and wall-clock elapsed time stays under 10 s. Verified red against the original bare `catch` (the test ran for the full 30 s without throwing) and green after the filter landed (the bench exited promptly with OCE). + ### Client.Dotnet-021 | Field | Value | @@ -473,7 +479,7 @@ The warmup loop above (lines 774-780) has no catch at all, so a warmup-time OCE | Severity | Low | | Category | Correctness & logic bugs | | Location | `clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs:487`, `clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs:715` | -| Status | Open | +| Status | Resolved | **Description:** Both new bulk-read CLI handlers cast a signed `--timeout-ms` argument to `uint` without bounds checking: @@ -499,3 +505,5 @@ uint timeoutMs = (uint)timeoutMsRaw; ``` A single shared helper (e.g. `ParseTimeoutMs(CliArguments, string, int)`) on `MxGatewayClientCli` would cover both call sites and remove the duplication. + +**Resolution:** 2026-05-24 — Confirmed against source: both `ReadBulkAsync` (line 490) and `BenchReadBulkAsync` (line 715) cast `arguments.GetInt32("timeout-ms", ...)` straight to `uint`, so `--timeout-ms -1` silently wrapped to `0xFFFFFFFF` (~49.7 days). Added a single shared private helper `ParseTimeoutMs(CliArguments arguments, int defaultValue)` on `MxGatewayClientCli` that reads the int32, rejects negatives with a clear `ArgumentException` ("--timeout-ms must be a non-negative integer (use 0 for the gateway default)."), and returns the safe `(uint)`. Both call sites now route through the helper. Regression test `MxGatewayClientCliTests.RunAsync_TimeoutMs_NegativeValue_RejectsWithClearError` (xUnit `[Theory]` over `read-bulk` and `bench-read-bulk`) drives the CLI with `--timeout-ms -1` and asserts the exit code is non-zero, that stderr contains "timeout-ms", and that the "non-negative" guard text is present. Verified red against the original `(uint)arguments.GetInt32(...)` casts (the bench proceeded past the timeout parse and tripped a downstream "Queue empty" error rather than the descriptive guard message) and green after the helper landed.