Adding support for the no_responders feature.

[stebet]

This solves #258

• Throws a new NatsNoRespondersException if triggered via. RequestAsync, but not if triggered via PublishAsync.
• A normal subscriber that receives a message due to no_responders, in case the client is using his own request-reply mechanism through PubSub and reply_to, does NOT throw an exception but the subscriber receives an empty message with msg.Headers.Code equal to 503.
  • We could also have this throw an exception but I felt it didn't make sense outside of RequestAsync but that's open for discussion. It could even be another configuration value for NatsOpts (bool ThrowOnNoResponders) so let's discuss that.
• Added tests for both RequestAsync and custom PubSub to trigger the behavior.
• Also ran dotnet format which formatted some unrelated files.

[mtmk]

A normal subscriber that receives a message due to no_responders, in case the client is using his own request-reply mechanism through PubSub and reply_to, does NOT throw an exception but the subscriber receives an empty message with msg.Headers.Code equal to 503.

• We could also have this throw an exception but I felt it didn't make sense outside of RequestAsync but that's open for discussion. It could even be another configuration value for NatsOpts (bool ThrowOnNoResponders) so let's discuss that.

Maybe we should make it a property on the message itself? (Rust client seems to be doing that)

Also looking at Go and Rust clients there isn't an option to set no_responders. If the client is supporting headers no_responders is implied.

[stebet]

Maybe we should make it a property on the message itself? (Rust client seems to be doing that)

That's possible. I still think an exception is more intuitive though from a C# convention perspective (compared to the NatsNoReplyException for timeouts), unless explicitly documented, since another side effect is that the Msg.Data property will be a default value (null for objects, 0 for ints etc.)

Also looking at Go and Rust clients there isn't an option to set no_responders. If the client is supporting headers no_responders is implied.

In that case we should default the NatsOpts.NoResponders property to true, so it can rather be explicitly disabled for clients if they want, since this client does support headers. Or should it just not be configurable at all?

[mtmk]

Maybe we should make it a property on the message itself? (Rust client seems to be doing that)

That's possible. I still think an exception is more intuitive though from a C# convention perspective (compared to the NatsNoReplyException for timeouts), unless explicitly documented, since another side effect is that the Msg.Data property will be a default value (null for objects, 0 for ints etc.)

right, I think exception is the right thing to do. I was suggesting the way we detect no responders can be a calculated property on the message object.

Also looking at Go and Rust clients there isn't an option to set no_responders. If the client is supporting headers no_responders is implied.

In that case we should default the NatsOpts.NoResponders property to true, so it can rather be explicitly disabled for clients if they want, since this client does support headers. Or should it just not be configurable at all?

I'm thinking less options the better but if you say there may be a use case for it we could leave it in the options but yes I think it should default to true if we left it as an option.

[stebet]

@mtmk

• Removed the NoResponders option (don't really see a use-case for disabling it).
• Added a bool IsNoRespondersError property to NatsMsg for easy checking.

[mtmk]

Excellent. thank you @stebet! One last thing, should we have the check in RequestMany as well for completeness? Is that something you've seen being used?

[mtmk]

@stebet don't worry about tests. there may be quite a few failing I can fix them later.

[stebet]

@stebet don't worry about tests. there may be quite a few failing I can fix them later.

Fixed the ones that my changes broke :) which were counting on timeouts when they were received NoResponders instead. We could also just drop the NatsNoRespondersException and just use the existing NatsNoReplyException with a clear "No responders" message since it's implied that you will never get a reply if there is no responder?

[mtmk]

@stebet don't worry about tests. there may be quite a few failing I can fix them later.

Fixed the ones that my changes broke :) which were counting on timeouts when they were received NoResponders instead. We could also just drop the NatsNoRespondersException and just use the existing NatsNoReplyException with a clear "No responders" message since it's implied that you will never get a reply if there is no responder?

Thank you :-) yes good point. would there be a need to distinguish the two? e.g. trying to pick up the fastest responder in a set of responders?

[stebet]

Thank you :-) yes good point. would there be a need to distinguish the two? e.g. trying to pick up the fastest responder in a set of responders?

Not apart from the exception message I think. They're sort of equivalent, one will just be returned pretty much instantly (no responders) since there is no responder that will ever answer, or they simply took too long and the request timed out (exceeded the NatsOpts.RequestTimeout threshold)

We can also just keep them separate for clarity to avoid confusion.

[caleblloyd]

I don't know if it makes sense to offer just 1 public IsNoRespondersError here. If we do one, we should probably enumerate every single error code. I don't know if using Go or Rust as examples is great. Those languages don't have exceptions.

[mtmk]

Not a standard error very specific message from the server. Other errors should still be check from the headers. we can't rely throw exceptions at this level without destroying the subscription.

⚠️ we should also check that payload is empty as other clients are doing. We don't have a reliable way of doing that (so ignore it for this issue). I'll create a separate issue for that.

[caleblloyd]

I am pretty sure that Go will return a No Responders error in any subscription if it receives one, not just in the Request-Reply APIs:

https://github.com/nats-io/nats.go/blob/48e070d968fc25ce61883c47bf4cba8a677ba0ff/nats.go#L4682-L4684

Also, I do think they should be separate Exceptions. NatsNoRespondersException is immediate and indicates that no service is listneing. Wheras NatsNoReplyException indicates that there was interest in the subject at the time it was sent, but a client-side timeout ocurred. So I like the separate Exceptions here.

[mtmk]

I am pretty sure that Go will return a No Responders error in any subscription if it receives one, not just in the Request-Reply APIs:

We can't throw exception per message. We'd need to end the subscription.

[mtmk]

see also #262

[mtmk]

I somewhat disagree with handling no-responders at subscription level having said that I also can't see a nice way of doing that. But maybe I'm missing something.

[stebet]

No responders at a subscription level is an edge case imo so I don't think it warrants an exception. It should never happen unless the client is explicitly doing his own response-request mechanism, since it's only returned to the reply_to subject if the subscriotion is on the same connection as the one making the publish. It's not sent if the connection listening to the reply_to subject is different from the one making the publish.

In that case it's good to have the property to do a quick check. On that note, a class with constant values for possible status codes might be a good idea for a later PR to make it clearer in the code what each status code means (similar to the StatusCodes constants in the ASP.NET Core libraries).

[caleblloyd]

I am pretty sure that Go will return a No Responders error in any subscription if it receives one, not just in the Request-Reply APIs:

We can't throw exception per message. We'd need to end the subscription.

Ah good point. Then it's probably fine just to throw in the Request-Reply patterns

a class with constant values for possible status codes might be a good idea for a later PR to make it clearer in the code what each status code means (similar to the StatusCodes constants in the ASP.NET Core libraries).

That sounds like a good idea!

[mtmk]

@stebet please resolve conflict then we can merge. thanks!

[stebet]

@stebet please resolve conflict then we can merge. thanks!

Rebased on latest main

[mtmk]

LGTM thanks @stebet!