diff --git a/doc/api/stream.md b/doc/api/stream.md index 7bc1874c803cae..517a807745c126 100644 --- a/doc/api/stream.md +++ b/doc/api/stream.md @@ -369,7 +369,7 @@ to be processed. However, use of `writable.cork()` without implementing See also: [`writable.uncork()`][], [`writable._writev()`][stream-_writev]. -##### `writable.destroy([error])` +##### `writable.destroy([error][, callback])` @@ -389,6 +389,11 @@ the `'drain'` event before destroying the stream. Once `destroy()` has been called any further calls will be a noop and no further errors except from `_destroy` may be emitted as `'error'`. +If passed `callback`; it will be invoked once the stream destrution has +completed. If an error has occured it will be passed as the first argument to +the callback and no `uncaughtException` error will occur even if no `'error'` +listener has been registered on the stream. + Implementors should not override this method, but instead implement [`writable._destroy()`][writable-_destroy]. @@ -2936,6 +2941,6 @@ contain multi-byte characters. [stream-write]: #stream_writable_write_chunk_encoding_callback [Stream Three States]: #stream_three_states [writable-_destroy]: #stream_writable_destroy_err_callback -[writable-destroy]: #stream_writable_destroy_error +[writable-destroy]: #stream_writable_destroy_error_callback [writable-new]: #stream_constructor_new_stream_writable_options [zlib]: zlib.html diff --git a/lib/internal/streams/destroy.js b/lib/internal/streams/destroy.js index 3a953afd445649..2cb077f5d81eef 100644 --- a/lib/internal/streams/destroy.js +++ b/lib/internal/streams/destroy.js @@ -1,18 +1,28 @@ 'use strict'; -// Undocumented cb() API, needed for core, not for public API. -// The cb() will be invoked synchronously if _destroy is synchronous. -// If cb is passed no 'error' event will be emitted. +const { once } = require('internal/util'); +let eos; + function destroy(err, cb) { const r = this._readableState; const w = this._writableState; - if ((w && w.destroyed) || (r && r.destroyed)) { - if (typeof cb === 'function') { - // TODO(ronag): Invoke with `'close'`/`'error'`. - cb(); - } + if (typeof err === 'function') { + cb = err; + err = null; + } + if (typeof cb === 'function') { + // TODO(ronag): Remove once cb is invoked only through eos. + cb = once(cb); + + if (!eos) eos = require('internal/streams/end-of-stream'); + eos(this, (err) => { + cb(err && err.code !== 'ERR_STREAM_PREMATURE_CLOSE' ? err : undefined); + }); + } + + if ((w && w.destroyed) || (r && r.destroyed)) { return this; } @@ -52,6 +62,12 @@ function destroy(err, cb) { r.closed = true; } + // TODO(ronag): Remove this and always use eos + // in order to ensure the same order relative + // to events regardless whether this is the first + // call to destroy(cb) or not. + // Revisit once https://github.com/nodejs/node/pull/29179 + // is closed. if (typeof cb === 'function') { cb(err); } diff --git a/test/parallel/test-stream-writable-destroy.js b/test/parallel/test-stream-writable-destroy.js index 706847a8582d0c..c4a2d62261ae8e 100644 --- a/test/parallel/test-stream-writable-destroy.js +++ b/test/parallel/test-stream-writable-destroy.js @@ -248,9 +248,33 @@ const assert = require('assert'); const expected = new Error('kaboom'); + let ticked = false; + write.destroy(expected, common.mustCall((err) => { + assert.strictEqual(err, undefined); + assert.strictEqual(ticked, true); + let ticked2 = false; + write.destroy(expected, common.mustCall((err) => { + assert.strictEqual(err, undefined); + assert.strictEqual(ticked2, true); + })); + ticked2 = true; + })); + ticked = true; + + // Destroy already destroyed. + + ticked = false; write.destroy(expected, common.mustCall((err) => { assert.strictEqual(err, undefined); + assert.strictEqual(ticked, true); + let ticked2 = false; + write.destroy(expected, common.mustCall((err) => { + assert.strictEqual(err, undefined); + assert.strictEqual(ticked2, true); + })); + ticked2 = true; })); + ticked = true; } {