stream: don't flush destroyed writable

[ronag]

It doesn't make much sense (and is a bit weird) to flush a stream which has been destroyed.

I need help creating a relevant test for this.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

NOTE TO SELF: Look into needFinish & stream destroyed. errorOrDestroy added in this PR should preferably be async.

[jasnell]

Hmm.. I'm thinking this is more bug fix than breaking change. What do you think @mcollina?

[mcollina]

This will need a unit test.

I'm happy to land this as a bug fix as long as it does not break anything on CITGM.

[mafintosh]

def bugfix

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/24881/

[ronag]

test added and fixed

[mcollina]

I would still consider this a bugfix, but we should:

1. verify that CITGM passes
2. let it bake for LTS (10.x) for some time before backporting.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/24882/

[mcollina]

CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1932/

[ronag]

@mcollina: Sorry, found further issues.

Consider the updated tests. We now will throw ERR_STREAM_DESTROYED on some tests that assume ERR_STREAM_WRITE_AFTER_END also we now send the err on the write callback and don't emit it as an error on the stream.

How should this be?

[addaleax]

I wouldn’t mind being careful and labelling this semver-major.

[ronag]

Ok, I think everything (expect @mcollina's comment on !err && chinksWritten++) is fixed in a sensical manner.