Refactor body handling to use ReadableStream#32
Conversation
…ddleware Co-authored-by: Copilot <copilot@github.com>
commit: |
Co-authored-by: Copilot <copilot@github.com>
|
|
||
| const middleware = await middlewareLoader(); | ||
|
|
||
| const [bodyForMiddleware, bodyForForward] = internalEvent.body?.tee() ?? [undefined, undefined]; |
There was a problem hiding this comment.
i wonder if this is going to be unnecessary for the majority of apps. as in, if someone isn't going to read the body in the middleware, we're probably going ot tee the stream but never make use of that. i wonder if there's some like lazy tee or something we could do potentially.
There was a problem hiding this comment.
Don't know anything about a lazy tee, and to be fair in most case there will be no body.
I guess we could add an option (or an env variable) to not forward the body to the middleware if not needed
| if (!streamDone) { | ||
| streamDone = true; | ||
| reader.cancel().catch(() => {}); | ||
| } |
There was a problem hiding this comment.
is it worth waiting for the current read to finish before cancelling, if there's on in-progress? something like toggle streamdone, wait for reading to be false, and in the pump below dont do it if streamdone is true, then cancel here after reading is false? WDYT?
There was a problem hiding this comment.
I guess we could do something like that. I don't see any downsides to this at the moment
| .then(({ done, value }) => { | ||
| if (done) { | ||
| streamDone = true; | ||
| reader.releaseLock(); |
There was a problem hiding this comment.
hmm shouldnt the release also happen in the catch as well?
Refactor the handling of request and response bodies to utilize ReadableStream across converters and middleware, improving consistency and performance in body processing.