stream: don't emit errors after destroy
This one is probably going to be a bit controversial but please give it a chance. It's the continuation on some related but less controversial PR's (e.g. https://github.com/nodejs/node/pull/29197)
Basically it tries to enforce the following rules:
- Emit one error after
destroy(err)
. - Don't emit any error after
destroy()
(including ifdestroy(err)
is called afterwards). - Always pass error to callback in
destroy(err, cb)
ordestroy(null, cb)
.
The idea is to try to avoid unexpectedly crashing the user's application or emit any unexpected errors (even if the user has error listeners registered). Most users (I believe?) assume that after destroy()
no more errors will be emitted, i.e. the object is practically "killed".
In the case where an error is explicitly passed to destroy(err)
, it still makes sense to emit one error. Since the user has explicitly passed one.
If the user wants to handle any errors on the no error case they should pass a callback in destroy(null, cb)
. This would also probably require that the undocumented/internal callback API for destroy()
is documented.
This is semver-major but I don't believe the chances of it breaking anything are very high.
The tests also need a little more polish (I'll do that if there is interest in going forward with this PR).
Checklist
-
make -j4 test
(UNIX), orvcbuild test
(Windows) passes -
tests and/or benchmarks are included -
documentation is changed or added -
commit message follows commit guidelines