Skip to content

Release 2.8.1#1164

Merged
mtmk merged 1 commit into
mainfrom
release-2.8.1
May 27, 2026
Merged

Release 2.8.1#1164
mtmk merged 1 commit into
mainfrom
release-2.8.1

Conversation

@mtmk

@mtmk mtmk commented May 27, 2026

Copy link
Copy Markdown
Member

Patch release with bug fixes and a Services helper addition for detecting service errors on responses.

@synadia-claude-reviewer

synadia-claude-reviewer Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mtmk's task in 3m 57s —— View job


PR Review: Release 2.8.1

  • Gather context and understand request
  • Examine PR diff and changed files
  • Review commits included in this release
  • Analyze key bug fixes for correctness
  • Post review findings

This release PR only bumps version.txt from 2.8.0 to 2.8.1. All substantive changes landed via the referenced PRs already merged to main. I reviewed each bug fix and the new service-error helpers below.


Fix: Subscription timeout overflow (#1134)

TimeoutValidation.Validate correctly handles all sentinels: null, TimeSpan.MaxValue, and Timeout.InfiniteTimeSpan are all returned as the caller-specified "no timeout" result before the range check fires. The Timeout.InfiniteTimeSpan == TimeSpan.FromMilliseconds(-1) case is caught before v < TimeSpan.Zero, so the ordering is correct. Applied to all three NatsSubBase timeout fields and ReplyTask. No issues.


Fix: Quiet recoverable server-error logs (#1148)

HandleServerError correctly centralises log dispatch and TrySetObservedException is still called unconditionally (outside the level switch), so reconnect behaviour is unchanged. The ParseError slicing change (from GetOffset(position.Value) - 1 to position.Value then lineWithCR.Length - 1) produces identical results—cleaner two-step strip. No issues.


Fix: KV cap duplicate_window at max_age (#1149)

Straightforward ADR-8 implementation. The 2-minute default constant is applied before the MaxAge cap, and applied on both create and update paths. No issues.


Fix: Durable consumer create when only DurableName is set (#1150)

The name-resolution order (NameDurableName fallback) is correct. ThrowIfInvalidConsumerName is called on the resolved name, so invalid chars in a supplied DurableName are caught on the fallback path. One minor note: if a caller sets both Name (valid) and DurableName (invalid chars), the invalid DurableName is not validated client-side, since Name wins. The server will reject the request, but the exception type will be different from the local ArgumentException. Very edge-case, noted for awareness. No blocking issue.


New API: Service error helpers (#1152)

Overall clean and well-tested. Two observations:

1. IsServiceSuccess(throwOnNoResponders: false) vs GetServiceStatus(false).IsSuccess are inconsistent for the no-responders case.

When throwOnNoResponders: false and msg.HasNoResponders:

  • IsServiceSuccess returns true (header-only check, no error header present → success).
  • GetServiceStatus(false).IsSuccess returns false (IsSuccess = Message is null && !HasNoResponders).

A user who switches from IsServiceSuccess to GetServiceStatus after reading the docs may be surprised. IsServiceSuccess is documented as a header-check so it's technically correct, but the naming implies broader correctness. Callers opting out of the no-responders throw should prefer GetServiceStatus for full fidelity.

2. An empty Nats-Service-Error header value causes EnsureServiceSuccess to throw with an empty message.

LastValueOrEmpty returns "" when the header value is present but empty. NatsSvcStatus.FromError(0, "") sets Message = "" (non-null), so EnsureServiceSuccess throws NatsSvcEndpointException(0, ""). This is technically correct (the header was set = error), but the resulting exception is not helpful. If a server misbehaves and sends a blank error header the caller gets Code=0, Message="" with no diagnostics. A null-or-whitespace guard in LastValueOrEmpty / FromError could improve this, though it's a very edge case.


Summary

All bug fixes are correct and well-tested. The new service-error API is solid; the two observations above are design-level notes rather than correctness bugs. This patch release is ready from a code-review perspective.

@Jarema Jarema left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@mtmk
mtmk merged commit b024736 into main May 27, 2026
30 of 31 checks passed
@mtmk
mtmk deleted the release-2.8.1 branch May 27, 2026 11:30
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