Node v16 issue: response is not drained so response never emits 'finish'

[benjie]

To solve this, add a data handler to res:

const res = new MockRes();
res.on("data", () => {});

This issue does not exist on Node v8, v10, v12 or v14.

---

Here's me stepping through some debug stacks to figure out what's going on: https://discord.com/channels/489127045289476126/900390109541703700/900707793445720104 (invite: https://discord.gg/graphile )

[diachedelic]

How can mock-res be changed to fix this issue?

[benjie]

I think just adding a drain internally would fix it - after setting up the Transform, add a drain to it: this.on('data', ()=>{})

I’m not sure if that would break anything though 🤷‍♂️

[diachedelic]

It does appear to emit a "finish" event in Node v16.

$ node
Welcome to Node.js v16.4.1.
Type ".help" for more information.
> const MockServerResponse = require("./index.js")
undefined
> const res = new MockServerResponse(function () {
... console.log("Finish");
... });
undefined
> res.end();
undefined
> Finish

I suspect your code is not calling res.end()?

[benjie]

Thanks for the response, and my apologies for not providing a reproducible example; let me rectify this.

Your example doesn't require draining; to see the issue you need to write enough data to the response that draining is necessary:

$ node
Welcome to Node.js v16.4.0.
Type ".help" for more information.
> const MockServerResponse = require("mock-res")
undefined
> const res = new MockServerResponse(() => {console.log('Finish')});
undefined
> res.writeHead(200, {'Content-Type': 'text/plain'});
undefined
> res.end('All work and no play makes Jack a dull boy.\n'.repeat(1000));
undefined
> console.log('Nothing');
Nothing
undefined
> 

This also occurs if you use .write(LOTS_OF_DATA) and then .end() without arguments:

$ node
Welcome to Node.js v16.4.0.
Type ".help" for more information.
> const MockServerResponse = require("mock-res")
undefined
> const res = new MockServerResponse(() => {console.log('Finish')});
undefined
> res.writeHead(200, {'Content-Type': 'text/plain'});
undefined
> res.write('All work and no play makes Jack a dull boy.\n'.repeat(1000));
false
> res.end()
undefined
> 

Here's a screenshot showing that my code does call .end() (look in the stack trace) and tracking down where the issue lies (namely that the rState.length is large and length is zero, so immediate finish does not occur):

(More screenshots of the debugging experience and the differences between Node 14 and Node 16 can be seen in the Discord thread linked above, if you're interested.)

[diachedelic]

Thanks, this should be fixed in v0.6.0. It was never safe to assume that the 'finish' event would immediately follow the call to res.end().

[benjie]

Thanks, this should be fixed in v0.6.0.

Awesome, thanks!

It was never safe to assume that the 'finish' event would immediately follow the call to res.end().

Indeed, I was merely highlighting where the behaviour of Node v14 and Node v16 differed: in v14 it would always end immediately for this transform, in v16 it doesn't and the this[kCallback] only gets called when the stream is drained, which will never happen if there's no drain. Immediate finish on .end() is neither expected nor required for the 'finish' event to work.

[diachedelic]

Yes, I mean it was never safe for me to make that assumption!