Node: Breaking chage with writable stream events

[alexpenev-s]

In some cases writable streams emit an error event after having already emitted a finish event.
This is inconsistent with older versions, and also breaks code that removes all event listeners on the finish event.

Consider the js code below. In version v8.1.1 the output is:
finish
error

In node version v6.9.5 and v4.2.6 the output is:
error

uname -a
Linux xxxx 3.16.0-4-amd64 #1 SMP Debian 3.16.43-2 (2017-04-30) x86_64 GNU/Linux

var stream = require('stream');

var rs = new stream.Readable();
rs.push('ok');
rs.push(null);
rs._read = () => { };

var ws = new stream.Writable();

ws.on('finish', () => { console.log('finish'); });
ws.on('error', () => { console.log('error'); });

ws._write = (chunk, encoding, done) => {
    setImmediate(() => { done(new Error()); });
};
rs.pipe(ws);

[mcollina]

I'm looking into this cc @nodejs/streams

[mcollina]

This was introduced by https://github.com/nodejs/node/commit/b153420fd92755875ae3fb9e75e83465f9469f2b.

@nodejs/lts you might not want to backport that one for now https://github.com/nodejs/LTS/issues/230 cc @gibfahn.

[calvinmetcalf]

just based on the description my first thought is that the actual issue is that in previous versions the error comes back before the finish while in current versions it finishes first

[mcollina]

@calvinmetcalf that was my understanding, but no. In the previous released there was no 'finish' event.

[mcollina]

@calvinmetcalf actually, that was the original problem that I fixed. The breakage comes because 'finish' happens _before_ 'error'. I'm fixing that now.

[mcollina]

Fixed by b443288.