Fixes related to JetStream errors being prematurely processed by the core request layer #288
Conversation
… code was set, it turned that into an error. The logic for this was not quite correct one requirement is for the message payload of this type of error be empty and NATS core currently can only expect error of 503 (no responders). Any interpretation of the status code unless a 503, should be delegated - in this case to JetStream which can then make better interpretation of the code (which could simply be a marker for no messages, or a request timeout on requests that have an expiration. [CHANGE] js.pull(stream,dur) now also adds an optional expires - the expires also overrides the request timeout. FIX #273
tests/jetstream_test.ts
Outdated
| expectError( | ||
| 3000, | ||
| ErrorCode.JetStream408RequestTimeout, | ||
| "408 request canceled", |
There was a problem hiding this comment.
So as of server rev (nats-io/nats-server@cadf921), when you reach the maximum number of pull requests, the new request will get a 409 (Exceeded MaxWaiting), and not an old one getting a 408 (Request Canceled). This was incorrect behavior. The server release notes for the upcoming 2.8.2 will state:
### Changed
- JetStream:
- When reaching the max number of pull requests, the server would
evict existing pending requests (with a `408 Request Canceled`)
instead of sending a `409 Exceeded MaxWaiting` to the new request (#3099)
…te enum with the proper name to code symmetry.
nats-base-client/jsutil.ts
Outdated
| // the description can be exceeded max waiting or max ack pending | ||
| return new NatsError( | ||
| description, | ||
| ErrorCode.JetStream409MaxWaitingExceeded, |
There was a problem hiding this comment.
Why name you "409" status JetStream409MaxWaitingExceeded or JetStream409MaxAckPendingExceeded? Should it not be simply JetStream409 or JetStream409Status? The description is then used to get a better sense of what the reason was...
There was a problem hiding this comment.
API breakage --- those are exported, and we added new things. But perhaps we can deprecate the old. Let's do that.
…rrorCodeJetstream409.
…rrorCodeJetstream409.
[FIX] the base client request introspected a message and if an status code was set, it turned that into an error. The logic for this was not quite correct one requirement is for the message payload of this type of error be empty and NATS core currently can only expect error of 503 (no responders). Any interpretation of the status code unless a 503, should be delegated - in this case to JetStream which can then make better interpretation of the code (which could simply be a marker for no messages, or a request timeout on requests that have an expiration.
[CHANGE] js.pull(stream,dur) now also adds an optional expires - the expires also overrides the request timeout.
FIX #273