GeezerVison is showing github.com — Revert "stream: fix .end() error propagation" · nodejs/node@d2a487e
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/streams/writable.js +13-27Lines changed: 13 additions & 27 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 27 | 27 |
| 28 | 28 | const { | 29 | 29 | FunctionPrototype, | 30 | | - Error, | 31 | 30 | ObjectDefineProperty, | 32 | 31 | ObjectDefineProperties, | 33 | 32 | ObjectSetPrototypeOf, | @@ -291,8 +290,8 @@ Writable.prototype.pipe = function() { | 291 | 290 | errorOrDestroy(this, new ERR_STREAM_CANNOT_PIPE()); | 292 | 291 | }; | 293 | 292 |
| 294 | | -function _write(stream, chunk, encoding, cb) { | 295 | | - const state = stream._writableState; | | 293 | +Writable.prototype.write = function(chunk, encoding, cb) { | | 294 | + const state = this._writableState; | 296 | 295 |
| 297 | 296 | if (typeof encoding === 'function') { | 298 | 297 | cb = encoding; | @@ -334,15 +333,11 @@ function _write(stream, chunk, encoding, cb) { | 334 | 333 |
| 335 | 334 | if (err) { | 336 | 335 | process.nextTick(cb, err); | 337 | | - errorOrDestroy(stream, err, true); | 338 | | - return err; | | 336 | + errorOrDestroy(this, err, true); | | 337 | + return false; | 339 | 338 | } | 340 | 339 | state.pendingcb++; | 341 | | - return writeOrBuffer(stream, state, chunk, encoding, cb); | 342 | | -} | 343 | | - | 344 | | -Writable.prototype.write = function(chunk, encoding, cb) { | 345 | | - return _write(this, chunk, encoding, cb) === true; | | 340 | + return writeOrBuffer(this, state, chunk, encoding, cb); | 346 | 341 | }; | 347 | 342 |
| 348 | 343 | Writable.prototype.cork = function() { | @@ -612,30 +607,21 @@ Writable.prototype.end = function(chunk, encoding, cb) { | 612 | 607 | encoding = null; | 613 | 608 | } | 614 | 609 |
| 615 | | - let err; | 616 | | - | 617 | | - if (chunk !== null && chunk !== undefined) { | 618 | | - const ret = _write(this, chunk, encoding); | 619 | | - if (ret instanceof Error) { | 620 | | - err = ret; | 621 | | - } | 622 | | - } | | 610 | + if (chunk !== null && chunk !== undefined) | | 611 | + this.write(chunk, encoding); | 623 | 612 |
| 624 | 613 | // .end() fully uncorks. | 625 | 614 | if (state.corked) { | 626 | 615 | state.corked = 1; | 627 | 616 | this.uncork(); | 628 | 617 | } | 629 | 618 |
| 630 | | - if (err) { | 631 | | - // Do nothing... | 632 | | - } else if (!state.errored && !state.ending) { | 633 | | - // This is forgiving in terms of unnecessary calls to end() and can hide | 634 | | - // logic errors. However, usually such errors are harmless and causing a | 635 | | - // hard error can be disproportionately destructive. It is not always | 636 | | - // trivial for the user to determine whether end() needs to be called | 637 | | - // or not. | 638 | | - | | 619 | + // This is forgiving in terms of unnecessary calls to end() and can hide | | 620 | + // logic errors. However, usually such errors are harmless and causing a | | 621 | + // hard error can be disproportionately destructive. It is not always | | 622 | + // trivial for the user to determine whether end() needs to be called or not. | | 623 | + let err; | | 624 | + if (!state.errored && !state.ending) { | 639 | 625 | state.ending = true; | 640 | 626 | finishMaybe(this, state, true); | 641 | 627 | state.ended = true; | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-stream-writable-end-cb-error.js -35Lines changed: 0 additions & 35 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -46,38 +46,3 @@ const stream = require('stream'); | 46 | 46 | writable.emit('error', new Error('kaboom')); | 47 | 47 | })); | 48 | 48 | } | 49 | | - | 50 | | -{ | 51 | | - const w = new stream.Writable({ | 52 | | - write(chunk, encoding, callback) { | 53 | | - setImmediate(callback); | 54 | | - }, | 55 | | - finish(callback) { | 56 | | - setImmediate(callback); | 57 | | - } | 58 | | - }); | 59 | | - w.end('testing ended state', common.mustCall((err) => { | 60 | | - // This errors since .destroy(err), which is invoked by errors | 61 | | - // in same tick below, will error all pending callbacks. | 62 | | - // Does this make sense? Not sure. | 63 | | - assert.strictEqual(err.code, 'ERR_STREAM_DESTROYED'); | 64 | | - })); | 65 | | - assert.strictEqual(w.destroyed, false); | 66 | | - assert.strictEqual(w.writableEnded, true); | 67 | | - w.end(common.mustCall((err) => { | 68 | | - // This errors since .destroy(err), which is invoked by errors | 69 | | - // in same tick below, will error all pending callbacks. | 70 | | - // Does this make sense? Not sure. | 71 | | - assert.strictEqual(err.code, 'ERR_STREAM_DESTROYED'); | 72 | | - })); | 73 | | - assert.strictEqual(w.destroyed, false); | 74 | | - assert.strictEqual(w.writableEnded, true); | 75 | | - w.end('end', common.mustCall((err) => { | 76 | | - assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END'); | 77 | | - })); | 78 | | - assert.strictEqual(w.destroyed, true); | 79 | | - w.on('error', common.mustCall((err) => { | 80 | | - assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END'); | 81 | | - })); | 82 | | - w.on('finish', common.mustNotCall()); | 83 | | -} |
|