diff --git a/doc/api/stream.md b/doc/api/stream.md index 2656fe1af606de..1f0f08a7b86a7f 100644 --- a/doc/api/stream.md +++ b/doc/api/stream.md @@ -2425,7 +2425,8 @@ and `stream.Readable` classes, respectively. The `'finish'` event is emitted after [`stream.end()`][stream-end] is called and all chunks have been processed by [`stream._transform()`][stream-_transform]. The `'end'` event is emitted after all data has been output, which occurs after the callback in -[`transform._flush()`][stream-_flush] has been called. +[`transform._flush()`][stream-_flush] has been called. In the case of an error, +neither `'finish'` nor `'end'` should be emitted. #### transform.\_flush(callback) diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js index e212881c4ac555..4cb3be5c008e9e 100644 --- a/lib/_stream_writable.js +++ b/lib/_stream_writable.js @@ -437,19 +437,12 @@ function onwriteError(stream, state, sync, er, cb) { // Defer the callback if we are being called synchronously // to avoid piling up things on the stack process.nextTick(cb, er); - // This can emit finish, and it will always happen - // after error - process.nextTick(finishMaybe, stream, state); - errorOrDestroy(stream, er); } else { // The caller expect this to happen before if // it is async cb(er); - errorOrDestroy(stream, er); - // This can emit finish, but finish must - // always follow error - finishMaybe(stream, state); } + errorOrDestroy(stream, er); } function onwrite(stream, er) { @@ -618,6 +611,7 @@ Object.defineProperty(Writable.prototype, 'writableLength', { function needFinish(state) { return (state.ending && state.length === 0 && + !state.errorEmitted && state.bufferedRequest === null && !state.finished && !state.writing); diff --git a/test/parallel/test-net-connect-buffer.js b/test/parallel/test-net-connect-buffer.js index 41df6a06662400..749eee519904f5 100644 --- a/test/parallel/test-net-connect-buffer.js +++ b/test/parallel/test-net-connect-buffer.js @@ -51,34 +51,6 @@ tcp.listen(0, common.mustCall(function() { assert.strictEqual(socket.connecting, true); assert.strictEqual(socket.readyState, 'opening'); - // Make sure that anything besides a buffer or a string throws. - common.expectsError(() => socket.write(null), - { - code: 'ERR_STREAM_NULL_VALUES', - type: TypeError, - message: 'May not write null values to stream' - }); - [ - true, - false, - undefined, - 1, - 1.0, - +Infinity, - -Infinity, - [], - {} - ].forEach((value) => { - // We need to check the callback since 'error' will only - // be emitted once per instance. - socket.write(value, common.expectsError({ - code: 'ERR_INVALID_ARG_TYPE', - type: TypeError, - message: 'The "chunk" argument must be one of type string or Buffer. ' + - `Received type ${typeof value}` - })); - }); - // Write a string that contains a multi-byte character sequence to test that // `bytesWritten` is incremented with the # of bytes, not # of characters. const a = "L'État, c'est "; diff --git a/test/parallel/test-net-socket-write-error.js b/test/parallel/test-net-socket-write-error.js index 800d6020da4cbe..178cebe9903cfb 100644 --- a/test/parallel/test-net-socket-write-error.js +++ b/test/parallel/test-net-socket-write-error.js @@ -13,7 +13,7 @@ function connectToServer() { type: TypeError }); - client.end(); + client.destroy(); }) - .on('end', () => server.close()); + .on('close', () => server.close()); } diff --git a/test/parallel/test-net-write-arguments.js b/test/parallel/test-net-write-arguments.js new file mode 100644 index 00000000000000..19b037ee0fc94c --- /dev/null +++ b/test/parallel/test-net-write-arguments.js @@ -0,0 +1,33 @@ +'use strict'; +const common = require('../common'); +const net = require('net'); + +const socket = net.Stream({ highWaterMark: 0 }); + +// Make sure that anything besides a buffer or a string throws. +common.expectsError(() => socket.write(null), + { + code: 'ERR_STREAM_NULL_VALUES', + type: TypeError, + message: 'May not write null values to stream' + }); +[ + true, + false, + undefined, + 1, + 1.0, + +Infinity, + -Infinity, + [], + {} +].forEach((value) => { + // We need to check the callback since 'error' will only + // be emitted once per instance. + socket.write(value, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "chunk" argument must be one of type string or Buffer. ' + + `Received type ${typeof value}` + })); +}); diff --git a/test/parallel/test-stream-writable-write-writev-finish.js b/test/parallel/test-stream-writable-write-writev-finish.js index 4399f1ca503b37..aa43b1490c8600 100644 --- a/test/parallel/test-stream-writable-write-writev-finish.js +++ b/test/parallel/test-stream-writable-write-writev-finish.js @@ -14,16 +14,10 @@ const stream = require('stream'); cb(new Error('write test error')); }; - let firstError = false; - writable.on('finish', common.mustCall(function() { - assert.strictEqual(firstError, true); - })); - - writable.on('prefinish', common.mustCall()); - + writable.on('finish', common.mustNotCall()); + writable.on('prefinish', common.mustNotCall()); writable.on('error', common.mustCall((er) => { assert.strictEqual(er.message, 'write test error'); - firstError = true; })); writable.end('test'); @@ -36,16 +30,10 @@ const stream = require('stream'); setImmediate(cb, new Error('write test error')); }; - let firstError = false; - writable.on('finish', common.mustCall(function() { - assert.strictEqual(firstError, true); - })); - - writable.on('prefinish', common.mustCall()); - + writable.on('finish', common.mustNotCall()); + writable.on('prefinish', common.mustNotCall()); writable.on('error', common.mustCall((er) => { assert.strictEqual(er.message, 'write test error'); - firstError = true; })); writable.end('test'); @@ -62,16 +50,10 @@ const stream = require('stream'); cb(new Error('writev test error')); }; - let firstError = false; - writable.on('finish', common.mustCall(function() { - assert.strictEqual(firstError, true); - })); - - writable.on('prefinish', common.mustCall()); - + writable.on('finish', common.mustNotCall()); + writable.on('prefinish', common.mustNotCall()); writable.on('error', common.mustCall((er) => { assert.strictEqual(er.message, 'writev test error'); - firstError = true; })); writable.cork(); @@ -93,16 +75,10 @@ const stream = require('stream'); setImmediate(cb, new Error('writev test error')); }; - let firstError = false; - writable.on('finish', common.mustCall(function() { - assert.strictEqual(firstError, true); - })); - - writable.on('prefinish', common.mustCall()); - + writable.on('finish', common.mustNotCall()); + writable.on('prefinish', common.mustNotCall()); writable.on('error', common.mustCall((er) => { assert.strictEqual(er.message, 'writev test error'); - firstError = true; })); writable.cork(); @@ -123,14 +99,9 @@ const stream = require('stream'); rs._read = () => {}; const ws = new stream.Writable(); - let firstError = false; - ws.on('finish', common.mustCall(function() { - assert.strictEqual(firstError, true); - })); - ws.on('error', common.mustCall(function() { - firstError = true; - })); + ws.on('finish', common.mustNotCall()); + ws.on('error', common.mustCall()); ws._write = (chunk, encoding, done) => { setImmediate(done, new Error());