Skip to content

Release 3.0.0-preview.6#1140

Closed
mtmk wants to merge 8 commits into
release/3.0from
release-3.0.0-preview.6
Closed

Release 3.0.0-preview.6#1140
mtmk wants to merge 8 commits into
release/3.0from
release-3.0.0-preview.6

Conversation

@mtmk

@mtmk mtmk commented May 13, 2026

Copy link
Copy Markdown
Member

Preview release on the 3.0 line bumping version to 3.0.0-preview.6. Syncs everything merged to main since preview.5: a new server error event on NatsConnection, slnx solution-format migration, README and example docs additions, and a CI matrix bump to nats-server v2.12. No 3.0-only changes.

From main:

mtmk and others added 8 commits May 8, 2026 12:05
* examples: add Example.NatsIODocs project

* ci: add Example.NatsIODocs build workflow

* examples: address review feedback

- replace placeholder GUID with proper random one
- add Ctrl+C cancellation to subscribe examples so they exit cleanly
- restrict workflow GITHUB_TOKEN to contents:read

* ci: temporarily skip non-essential workflows on this branch

Will be reverted before merge.

* examples: remove cancellation plumbing; add timeout to harness

Subscribe examples no longer have CTS/CancelKeyPress plumbing, keeping
doc snippets focused on the API. The Program.cs harness uses
Task.WhenAny with a 5s timeout so subscribe examples can run unattended
in CI without hanging.

* examples: add 13 more NatsIODocs examples translated from java

Covers subjects (single/multi wildcards, monitoring), queue groups
(basic, dynamic scaling, request/reply, mixed subscribers), and
request/reply variants (basic, timeout, multiple responders, no
responders, headers, calculator). Each uses idiomatic .NET patterns:
NatsClient, await foreach over SubscribeAsync, and Task.Run for
background subscribers. Harness timeout bumped to 10s and CI runs all
17 examples.

* examples: split getting-started into standalone console projects

The getting-started examples are what new users will copy into a fresh
'dotnet new console' project, so they should be standalone rather than
called via a static-class dispatcher. Each is now its own .csproj with
its own Program.cs as the actual entry point. CI builds and runs both,
using timeout(1) (or gtimeout on macOS) to bound the subscribe example.

* examples: tighten NATS-DOC marker placement

- Move markers off class scope into RunAsync body for QueueGroupsBasic,
  QueueGroupsDynamicScaling, SubjectsMultiWildcard so the doc snippet
  doesn't drag in the static-class harness scaffolding.
- Stop running standalone getting-started projects in CI; build is
  enough since they target demo.nats.io which isn't reliably reachable
  from the macOS runner.

* examples: use NatsSubOpts.Timeout in RequestReplyTimeout, tighten SubjectsMonitoring snippet

RequestReplyTimeout now sets the per-request timeout via NatsSubOpts
and catches NatsNoReplyException, which is the idiomatic NATS .NET
pattern. SubjectsMonitoring's NATS-DOC markers now wrap just the
background subscribe pattern (matching the Java original), with a
count-based break replacing the cts.Token + try/catch noise.

* Revert "ci: temporarily skip non-essential workflows on this branch"

This reverts commit 21b97cd.

* examples: address review feedback

* examples: convert NatsIODocs to xunit test project

* examples: simplify NatsIODocs samples to doc-snippet style

* examples: refine NatsIODocs sample tweaks
Explains the split between the core NATS.Net client and the Orbit
utility packages (https://github.com/synadia-io/orbit.net): what each
is for, what goes where, and a small layering diagram. Mirrors the
section added in nats.rs#1580.

Signed-off-by: Tomasz Pietrek <tomasz@synadia.com>
* Add server error event

* Bump test session timeout to 600s

* Increase test ConnectTimeout to match other proxy-based tests

* Simplify ErrorHandlerTest publish-and-wait helper

* Route NatsConnection events through PushEvent

* Parse server error kind on NatsServerErrorEventArgs

* Bump auth timeout in restricted-user fixture

* Cover null input in NatsServerErrorEventArgs tests

* Add server-errors docs page
@synadia-claude-reviewer

synadia-claude-reviewer Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mtmk's task in 2m 29s —— View job


PR Review: Release 3.0.0-preview.6

  • Gather context and understand request
  • Read PR diff and changed files
  • Review server error event implementation
  • Review async/await correctness and thread safety
  • Review API surface and nullable annotations
  • Post findings

Summary

The main functional addition is the ServerError event on NatsConnection / INatsConnection. The rest of the diff is docs, examples, CI updates, and the slnx migration. No 3.0-specific changes.

The implementation is solid overall — it follows the existing async event-channel pattern correctly, error strings are never executed, and thread safety is consistent with the rest of the event dispatch path.

A couple of observations worth noting:


NatsServerErrorEventArgs.Error nullability contract

Error is typed string (non-nullable), but the unit test bypasses the type system with null! and asserts Assert.Equal(error, args.Error) — which actually stores null in a non-nullable property.

From the production call path (ParseError) this can never happen — ParseError always returns a string (either string.Empty or a decoded UTF-8 slice). So the null test is a false-positive edge case that the constructor's public API will never receive in real use. Two reasonable fixes:

  1. Make Error nullable (string?) and update the test to be Assert.Equal(error ?? string.Empty, args.Error) if you want to guard against misuse.
  2. Remove the null test case since the constructor is only ever called from the protocol processor with a non-null result.

Option 2 is simpler given the production reality.

Fix this →


Event ordering: PushEvent before TrySetObservedException

In NatsReadProtocolProcessor.cs lines 407–408 and 415–416, the event is pushed to the channel before the pong-signal exception is set:

_connection.PushEvent(NatsEvent.ServerError, new NatsServerErrorEventArgs(error));
_waitForPongOrErrorSignal.TrySetObservedException(new NatsServerException(error));

Because PushEvent only enqueues (non-blocking) and the actual ServerError handler fires asynchronously from the event loop, the TrySetObservedException will almost certainly resolve first — reconnect logic may begin before any registered ServerError handler runs. This is the same behavior as all other events in the system, so it's acceptable, but worth documenting if users ask why the event arrives "after" reconnection has started.


ParseKind prefix ordering: no bug

The order "Maximum Account Active Connections" before "Maximum Connections" is safe: the two prefixes are mutually exclusive, so no wrong branch is taken regardless of ordering. ✓


CI: test_natsiodocs.yml always uses nats-server@latest

The new workflow installs the server via go install github.com/nats-io/nats-server/v2@latest. This is intentional for docs examples but it does mean any breaking server change in main could silently break the docs CI without a pinned version. This is a deliberate trade-off and fine for examples.


Overall the change is correct and ready to merge. The null-in-Error observation above is the only thing worth a follow-up (even if as a deferred cleanup).

@github-actions

Copy link
Copy Markdown

[InlineData("Authentication Timeout")]
[InlineData("Maximum Payload Violation")]
[InlineData("Invalid Subject")]
[InlineData("Invalid Publish Subject")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

null is passed with ! suppression, but NatsServerErrorEventArgs.Error is typed string (non-nullable). Since ParseError in NatsReadProtocolProcessor never returns null (it returns string.Empty for short sequences, a decoded UTF-8 string otherwise), this test case can't happen in production and silently stores null in a non-nullable property. Consider removing the null case here.

@mtmk

mtmk commented May 13, 2026

Copy link
Copy Markdown
Member Author

Closing in favour of a clean PR. The diff here picked up all of the main sync because release/3.0 hadn't been pushed with the merge before this branch was cut; reopening with just the version.txt bump on top of the now-synced release/3.0.

@mtmk mtmk closed this May 13, 2026
@mtmk
mtmk deleted the release-3.0.0-preview.6 branch May 13, 2026 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants