fix: handle terminal middleware without mutating chains#3534
Conversation
| */ | ||
| export interface StreamMiddleware { | ||
| (stream: Stream, connection: Connection, next: (stream: Stream, connection: Connection) => void): void | Promise<void> | ||
| (stream: Stream, connection: Connection, next: (stream: Stream, connection: Connection) => void): void | false | Promise<void | false> |
There was a problem hiding this comment.
should this be void | boolean | Promise<void | boolean>?
There was a problem hiding this comment.
should be false - kind of want false to be an explicit option to cancel the middleware... or could make it true & void do the same thing which feels a bit off
There was a problem hiding this comment.
thinking about cases where the function returns some boolean logic. would have to coerce to void or false to stop or not stop the middleware while also fitting the type constraint.
There was a problem hiding this comment.
The semantics get a bit awkward because you need to choose what the boolean means - e.g. isAuthorized() vs isBlocked()
| /** | ||
| * Stream middleware allows accessing stream data outside of the stream handler | ||
| * | ||
| * Return false to stop the middleware chain without aborting the stream. |
There was a problem hiding this comment.
could said returning true or void lets the middleware continue.
2 participants
Description
Fixes two stream middleware issues:
Adds regression coverage for repeated inbound streams, terminal sync/async middleware, outbound terminal middleware, and throw/reject abort behaviour.
Change checklist