Skip to content

Fix consume 503 handling#849

Merged
mtmk merged 8 commits into
mainfrom
fix-consume-503-handling
May 8, 2025
Merged

Fix consume 503 handling#849
mtmk merged 8 commits into
mainfrom
fix-consume-503-handling

Conversation

@mtmk

@mtmk mtmk commented May 7, 2025

Copy link
Copy Markdown
Member

No description provided.

@mtmk
mtmk requested a review from Copilot May 7, 2025 11:31

Copilot AI left a comment

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.

Pull Request Overview

This PR addresses the handling of 503 errors by improving the protocol error handling and notifications when consuming messages. Additionally, it refines the regex parsing for subscription and publication commands and adds new logging events and documentation for JetStream notifications.

  • Updated error-handling logic and notification usage in message consumption.
  • Improved regex patterns for SUB and PUB command parsing in the test mock server.
  • Added extended logging events and documentation for various JetStream notifications.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/NATS.Client.TestUtilities/MockServer.cs Updated logger handling; improved regex for SUB and PUB commands with new default encoding.
tests/NATS.Client.JetStream.Tests/DirectGetTest.cs Added tests for direct get responses with 503 and no reply scenarios.
tests/NATS.Client.JetStream.Tests/ConsumeResponseTest.cs Added tests to verify handling of various error headers including 503 responses.
src/NATS.Client.JetStream/NatsJSOrderedConsumer.cs Revised timeout notification to use a default instance.
src/NATS.Client.JetStream/NatsJSOpts.cs Extended documentation for notifications in consumption options.
src/NATS.Client.JetStream/NatsJSLogEvents.cs Added new logging events for "NoResponders" and "MessageSizeExceedsMaxBytes".
src/NATS.Client.JetStream/Internal/NatsJSFetch.cs Updated timeout notification usage.
src/NATS.Client.JetStream/Internal/NatsJSConsume.cs Revised error handling with reordered header condition checks and added notifications.
src/NATS.Client.JetStream/INatsJSNotification.cs Defined new notification record types with default instances.

Comment thread tests/NATS.Client.TestUtilities/MockServer.cs
Comment thread tests/NATS.Client.TestUtilities/MockServer.cs
Comment thread src/NATS.Client.JetStream/Internal/NatsJSConsume.cs
mtmk and others added 4 commits May 7, 2025 12:55
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mtmk
mtmk requested a review from scottf May 7, 2025 12:56

@scottf scottf left a comment

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.

LGTM

@scottf scottf left a comment

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.

LGTM

@mtmk
mtmk merged commit 7af473c into main May 8, 2025
@mtmk
mtmk deleted the fix-consume-503-handling branch May 8, 2025 15:56
mtmk added a commit that referenced this pull request May 13, 2025
* Fix reconnect socket behavior (#844)
* Fix consume 503 handling (#849)
* NatsMsg add create method (#632) (#798)
* Clean up reconnect logs (#843)
* Fix subscriptions dispose exception handling (#845)
@mtmk mtmk mentioned this pull request May 13, 2025
mtmk added a commit that referenced this pull request May 13, 2025
* Fix reconnect socket behavior (#844)
* Fix consume 503 handling (#849)
* NatsMsg add create method (#632) (#798)
* Clean up reconnect logs (#843)
* Fix subscriptions dispose exception handling (#845)
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.

3 participants