Open elucidsoft opened 1 year ago
I added an express version to the repo, so you can run them from same repo to see that express behaves as you would expect, and nestjs is not.
btw you forgot to call app.use(bodyParser.json())
in the Express version. But yeah, it does triggers the 'close' event.
I just tested your repro with v8 and v9. None of them emits the 'close' nor the 'end' event on that req
object when bodyParse
is not false
:thinking: Even when we use our own body-parser middleware
after investigating that a bit, I found this weird(?) behavior:
adding const onClose = () => console.log('----------'); req.on('close', onClose)
right before L149, it will make that onClose
be called. But when we move that line to after L149, onClose
won't get called. Maybe that's due to how the event loop works or there is something wrong with RouterExecutionContext#createPipesFn
. Note that this only happens when the body-parser middleware is evaluated
I just tested something: if we remove the await
from inside handler
's returns by making that fnApplyPipes
sync, the 'close' event got emitted in controller's method. So I guess that this has something to do on how the event loops works. But the 'end' event still didn't got emitted
Here's an up-to-date reproduction: https://gitlab.com/micalevisk/nestjs-issue-10389
@elucidsoft Were you able to find a workaround for this issue?
another thing that I observed:
if we drop the await
from L149 and make createPipesFn
return a synchronous function, it will work as we expect. But when we add a new line right after L149 using an await like this: await new Promise((resolve) => setTimeout(resolve, 0));
it will work just the same as having await
on L149. No ideia why is this tho
Is there an existing issue for this?
Current behavior
The req.on('close') event is not emitting when the content-type: applicaiton/json header is set. This makes no sense, the content-type header should not change the behavior when the connection is closed. This use to work, the behavior has somehow changed in either upgrade 7 or 8. We just observed this behavior, but we have done several nestjs upgrades and we are not sure when the behavior started. This took an extremely long time to track down, and is rather critical to our workflow, can please advise a temp workaround for us would be very helpful!!
Minimum reproduction code
https://github.com/elucidsoft/nestjs-broken-middleware
Steps to reproduce
Working without Header
content-type:
header NOT set.Broken with application/json Content-Type header
content-type: application/json
header set.Expected behavior
Event should emit no matter the content-type set.
Package
Other package
No response
NestJS version
9.1.4
Packages versions
I also verified this is broken in 8.x
Node.js version
16.14.2
In which operating systems have you tested?
Other
I verified this behavior on multiple platforms of Linux, inside docker, and outside docker, with multiple REST clients. All observed same behavior.