stream: fix destroy(err, cb) regression

[mcollina]

Fixed a regression that caused the callback passed to destroy()
to not be called if the stream was already destroyed.
This caused a regression on the ws module in CITGM, as discovered
by @refack.

Fixes: websockets/ws#1118

Checklist

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

Affected core subsystem(s)

stream, net

[cjihrig]

Does the missing semicolon pass make lint?

[mcollina]

nope, I have some leftover files with linting errors under bench, and I didn't spot those :(. Updated

[mcollina]

CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/816/
CI: https://ci.nodejs.org/job/node-test-pull-request/8231/

[cjihrig]

LGTM

[cjihrig]

It might be worth doing some assertion on the error received here.

[mcollina]

cc @lpinca

[mcollina]

CI (without linting errors): https://ci.nodejs.org/job/node-test-pull-request/8232/

[refack]

If this statement is important you should assert it (set a field on expected on line 177 and assert it's there in line 175).

[refack]

Some questions.

[refack]

Personally I like it that if a function is used it's a strong indicator that you need this bound.

[mcollina]

This is just a plain old ES5 function. I tend to say the contrary: I use () => {} as a strong indicator that I need this to stay the same.

[refack]

Cool.

[refack]

"

[refack]

IMHO If the cb should stay undocumented don't put it in the signature, extract it from arguments.
My IDE (WebStorm) will intellisense it's presence.