diff --git a/lib/internal/streams/writable.js b/lib/internal/streams/writable.js index 2510398d999fe1..97fe1d28ef1f3a 100644 --- a/lib/internal/streams/writable.js +++ b/lib/internal/streams/writable.js @@ -27,7 +27,6 @@ const { FunctionPrototype, - Error, ObjectDefineProperty, ObjectDefineProperties, ObjectSetPrototypeOf, @@ -291,8 +290,8 @@ Writable.prototype.pipe = function() { errorOrDestroy(this, new ERR_STREAM_CANNOT_PIPE()); }; -function _write(stream, chunk, encoding, cb) { - const state = stream._writableState; +Writable.prototype.write = function(chunk, encoding, cb) { + const state = this._writableState; if (typeof encoding === 'function') { cb = encoding; @@ -334,15 +333,11 @@ function _write(stream, chunk, encoding, cb) { if (err) { process.nextTick(cb, err); - errorOrDestroy(stream, err, true); - return err; + errorOrDestroy(this, err, true); + return false; } state.pendingcb++; - return writeOrBuffer(stream, state, chunk, encoding, cb); -} - -Writable.prototype.write = function(chunk, encoding, cb) { - return _write(this, chunk, encoding, cb) === true; + return writeOrBuffer(this, state, chunk, encoding, cb); }; Writable.prototype.cork = function() { @@ -612,14 +607,8 @@ Writable.prototype.end = function(chunk, encoding, cb) { encoding = null; } - let err; - - if (chunk !== null && chunk !== undefined) { - const ret = _write(this, chunk, encoding); - if (ret instanceof Error) { - err = ret; - } - } + if (chunk !== null && chunk !== undefined) + this.write(chunk, encoding); // .end() fully uncorks. if (state.corked) { @@ -627,15 +616,12 @@ Writable.prototype.end = function(chunk, encoding, cb) { this.uncork(); } - if (err) { - // Do nothing... - } else if (!state.errored && !state.ending) { - // This is forgiving in terms of unnecessary calls to end() and can hide - // logic errors. However, usually such errors are harmless and causing a - // hard error can be disproportionately destructive. It is not always - // trivial for the user to determine whether end() needs to be called - // or not. - + // This is forgiving in terms of unnecessary calls to end() and can hide + // logic errors. However, usually such errors are harmless and causing a + // hard error can be disproportionately destructive. It is not always + // trivial for the user to determine whether end() needs to be called or not. + let err; + if (!state.errored && !state.ending) { state.ending = true; finishMaybe(this, state, true); state.ended = true; diff --git a/test/parallel/test-stream-writable-end-cb-error.js b/test/parallel/test-stream-writable-end-cb-error.js index 20428f1777fd17..8f6d209954436f 100644 --- a/test/parallel/test-stream-writable-end-cb-error.js +++ b/test/parallel/test-stream-writable-end-cb-error.js @@ -46,38 +46,3 @@ const stream = require('stream'); writable.emit('error', new Error('kaboom')); })); } - -{ - const w = new stream.Writable({ - write(chunk, encoding, callback) { - setImmediate(callback); - }, - finish(callback) { - setImmediate(callback); - } - }); - w.end('testing ended state', common.mustCall((err) => { - // This errors since .destroy(err), which is invoked by errors - // in same tick below, will error all pending callbacks. - // Does this make sense? Not sure. - assert.strictEqual(err.code, 'ERR_STREAM_DESTROYED'); - })); - assert.strictEqual(w.destroyed, false); - assert.strictEqual(w.writableEnded, true); - w.end(common.mustCall((err) => { - // This errors since .destroy(err), which is invoked by errors - // in same tick below, will error all pending callbacks. - // Does this make sense? Not sure. - assert.strictEqual(err.code, 'ERR_STREAM_DESTROYED'); - })); - assert.strictEqual(w.destroyed, false); - assert.strictEqual(w.writableEnded, true); - w.end('end', common.mustCall((err) => { - assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END'); - })); - assert.strictEqual(w.destroyed, true); - w.on('error', common.mustCall((err) => { - assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END'); - })); - w.on('finish', common.mustNotCall()); -}