Skip to content

Conversation

@mtmk
Copy link
Member

@mtmk mtmk commented Nov 17, 2025

Ensure unobserved exceptions are handled properly during connection state transitions.

This pull request introduces a small improvement to exception handling during the initial connection process in NatsConnection.cs. The change ensures that unobserved exceptions from the connection task are suppressed, preventing potential issues with unhandled exceptions surfacing unexpectedly.

Exception handling improvement:

  • In InitialConnectAsync, after attempting to set an exception on _waitForOpenConnection, the code now explicitly accesses the task's Exception property to suppress unobserved exceptions, ensuring cleaner error handling during connection retries. [1] [2]

@mtmk mtmk requested a review from scottf November 17, 2025 13:32
Copy link
Contributor

@scottf scottf left a comment

Choose a reason for hiding this comment

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

LGTM

return source.ToArray().Should();
}

public static GenericCollectionAssertions<T> Should<T>(this ReadOnlySpan<T> source)
Copy link
Member Author

@mtmk mtmk Nov 18, 2025

Choose a reason for hiding this comment

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

This overload was causing stack overflow in tests. and since it wasn't really used by anything I removed them to fix the issue.

The reason for this starting to happen was the release of C# 14 last week where the span overload resolution changed and since we're using the latest language by default and github ubuntu-lates now includes .net 10, and in our global.json we roll to latest major, we end up using the newest stable compiler.

@mtmk
Copy link
Member Author

mtmk commented Nov 18, 2025

hi @DieterSedlmayer are you able to verify this fixes the issue for you? thanks

@DieterSedlmayer
Copy link

LGTM. Waiting for new NuGet release for re-testing.
Btw. Is it the expected behavior that, when using the defaults options with RetryOnInitialConnect = true, the connect attempt never times out?

@mtmk
Copy link
Member Author

mtmk commented Nov 24, 2025

LGTM. Waiting for new NuGet release for re-testing.

thanks, will merge.

Btw. Is it the expected behavior that, when using the defaults options with RetryOnInitialConnect = true, the connect attempt never times out?

Good point. we can add another option for that. wdyt?

@mtmk mtmk merged commit 35a1da0 into main Nov 24, 2025
24 of 25 checks passed
@mtmk mtmk deleted the handle-unobserved-task-exception branch November 24, 2025 10:25
mtmk added a commit that referenced this pull request Dec 11, 2025
* Add prioritized mode (#1011)
* Add Publish Timeout on Disconnect (#1001)
* Fix test flap (#1012)
* Add test to promote mirrored to regular stream (#1008)
* Add Nats-Expected-Last-Subject-Sequence-Subject (#1007)
* Fix keyed NATS clients with configurations (#1006)
* ensure NatsConnectionPool cannot overflow (#1005)
* Fix stream config adjustments on update (#995)
* Handel Unobserved Exceptions During Connection State Transitions (#999)
* Added verification of the consumer sequence number for pull ordered consumers (#981)
* Fix build warnings (#991)
* Reduce closure allocations (#988)
@mtmk mtmk mentioned this pull request Dec 11, 2025
mtmk added a commit that referenced this pull request Dec 11, 2025
* Add prioritized mode (#1011)
* Add Publish Timeout on Disconnect (#1001)
* Fix test flap (#1012)
* Add test to promote mirrored to regular stream (#1008)
* Add Nats-Expected-Last-Subject-Sequence-Subject (#1007)
* Fix keyed NATS clients with configurations (#1006)
* ensure NatsConnectionPool cannot overflow (#1005)
* Fix stream config adjustments on update (#995)
* Handel Unobserved Exceptions During Connection State Transitions (#999)
* Added verification of the consumer sequence number for pull ordered consumers (#981)
* Fix build warnings (#991)
* Reduce closure allocations (#988)
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.

Unobserved task exception when attempting to connect to not existing server

4 participants