From c335929cbadac63cc720e6f4cb3cb48c51c2428b Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 24 May 2017 13:24:19 +0200 Subject: [PATCH] stream: emit finish when using writev and cork In Writable, 'finish' was not emitted when using writev() and cork() in the event of an Error during the write. This commit makes it consistent with the write() path, which emits 'finish'. Fixes: https://github.com/nodejs/node/issues/11121 --- lib/_stream_writable.js | 9 +++- .../test-stream-writable-writev-finish.js | 53 +++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-stream-writable-writev-finish.js diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js index 4e2a19f12c822f..0c7f020bb23c27 100644 --- a/lib/_stream_writable.js +++ b/lib/_stream_writable.js @@ -358,14 +358,19 @@ function doWrite(stream, state, writev, len, chunk, encoding, cb) { function onwriteError(stream, state, sync, er, cb) { --state.pendingcb; if (sync) - process.nextTick(cb, er); + process.nextTick(afterError, stream, state, cb, er); else - cb(er); + afterError(stream, state, cb, er); stream._writableState.errorEmitted = true; stream.emit('error', er); } +function afterError(stream, state, cb, err) { + cb(err); + finishMaybe(stream, state); +} + function onwriteStateUpdate(state) { state.writing = false; state.writecb = null; diff --git a/test/parallel/test-stream-writable-writev-finish.js b/test/parallel/test-stream-writable-writev-finish.js new file mode 100644 index 00000000000000..6f74ca08d2366d --- /dev/null +++ b/test/parallel/test-stream-writable-writev-finish.js @@ -0,0 +1,53 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const stream = require('stream'); + +// ensure consistency between the finish event when using cork() +// and writev and when not using them + +{ + const writable = new stream.Writable(); + + writable._write = (chunks, encoding, cb) => { + cb(new Error('write test error')); + }; + + writable.on('finish', common.mustCall()); + + writable.on('prefinish', common.mustCall()); + + writable.on('error', common.mustCall((er) => { + assert.strictEqual(er.message, 'write test error'); + })); + + writable.end('test'); +} + +{ + const writable = new stream.Writable(); + + writable._write = (chunks, encoding, cb) => { + cb(new Error('write test error')); + }; + + writable._writev = (chunks, cb) => { + cb(new Error('writev test error')); + }; + + writable.on('finish', common.mustCall()); + + writable.on('prefinish', common.mustCall()); + + writable.on('error', common.mustCall((er) => { + assert.strictEqual(er.message, 'writev test error'); + })); + + writable.cork(); + writable.write('test'); + + setImmediate(function() { + writable.end('test'); + }); +}