handle no responders in NatsSubBase

[caleblloyd]

Handles No Responders in NatsSubBase, pre-de-serialization

Also handles StopOnEmptyMsg

This prevents the library from ever needing to check msg.Data for an Empty value

[caleblloyd]

@mtmk let me know if you prefer this approach - then we can fix/add tests here

[stebet]

Does this impþementation kill the subscription? It's important in my opinion that the subscription be left intact for custom implementations of request-reply. My use case for example needs the subscription to stay intact for performance reasons (avoids the overhead of a new inbox + subscription for every request).

[caleblloyd]

Does this impþementation kill the subscription?

Yes. But you can explicitly set NatsSubOpts.ThrowIfNoResponders=false and perform the check for a 503 code in the headers yourself.

[caleblloyd]

Note that Request-Reply uses the muxed Inbox Subscription by default, which muxes all subscriptions over a single persistent NATS Subscription. So there's no additional NATS Subscription overhead on Inbox Subscriptions (except for the first one established).

[mtmk]

I'm mainly concerned about auto unsubs. especially empty message check silently unsubs which could be source of hard to find bugs. also no-responders check may be OK but that only happens on reply inbox subscriptions and not all subscriptions.

[mtmk]

If you chose to implement this feel free to get rid of #275

[caleblloyd]

I'm mainly concerned about auto unsubs. especially empty message check silently unsubs which could be source of hard to find bugs. also no-responders check may be OK but that only happens on reply inbox subscriptions and not all subscriptions.

Both StopOnEmptyMsg? and ThrowIfNoResponders? are opt-in on NatsSubOpts. So nothing changes on normal subscriptions. For Request-Reply, ThrowIfNoResponders is set to true if not explicitly set to false, and if RequestMany is used then StopOnEmptyMsg is set to true if not explicitly set to false.

I think this matches all of the previous behavior.

[caleblloyd]

Should be ready for review

[mtmk]

a few nits

[mtmk]

LGTM

[mtmk]

Thank you @caleblloyd! was there any other change you wanted to make before merge?

[caleblloyd]

Nope- just merged!