From f224b24cbc6e50452ba25414a3dbc1877b8396d4 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 14 Jul 2019 22:11:06 +0200 Subject: [PATCH] stream: always invoke end callback --- doc/api/errors.md | 6 +++++ lib/_http_outgoing.js | 8 ++++++ lib/_stream_writable.js | 8 ++++++ lib/internal/errors.js | 3 +++ .../test-http-outgoing-end-multiple.js | 27 +++++++++++++++++++ .../test-stream-writable-end-multiple.js | 20 ++++++++++++++ 6 files changed, 72 insertions(+) create mode 100644 test/parallel/test-http-outgoing-end-multiple.js create mode 100644 test/parallel/test-stream-writable-end-multiple.js diff --git a/doc/api/errors.md b/doc/api/errors.md index e809b26f6d0d9c..a812d74c8f26ea 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1703,6 +1703,12 @@ An attempt was made to call [`stream.pipe()`][] on a [`Writable`][] stream. A stream method was called that cannot complete because the stream was destroyed using `stream.destroy()`. + +### ERR_STREAM_ALREADY_FINISHED + +A stream method was called that cannot complete because the stream was +finished. + ### ERR_STREAM_NULL_VALUES diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index ef7c8c65772b9d..42a971ea1b3471 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -45,6 +45,7 @@ const { ERR_INVALID_CHAR, ERR_METHOD_NOT_IMPLEMENTED, ERR_STREAM_CANNOT_PIPE, + ERR_STREAM_ALREADY_FINISHED, ERR_STREAM_WRITE_AFTER_END }, hideStackFrames @@ -668,6 +669,13 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) { } if (this.finished) { + if (typeof callback === 'function') { + if (!this.writableFinished) { + this.on('finish', callback); + } else { + callback(new ERR_STREAM_ALREADY_FINISHED('end')); + } + } return this; } diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js index bf9abeeed81d45..09f22442d7e2bb 100644 --- a/lib/_stream_writable.js +++ b/lib/_stream_writable.js @@ -41,6 +41,7 @@ const { ERR_MULTIPLE_CALLBACK, ERR_STREAM_CANNOT_PIPE, ERR_STREAM_DESTROYED, + ERR_STREAM_ALREADY_FINISHED, ERR_STREAM_NULL_VALUES, ERR_STREAM_WRITE_AFTER_END, ERR_UNKNOWN_ENCODING @@ -591,6 +592,13 @@ Writable.prototype.end = function(chunk, encoding, cb) { // Ignore unnecessary end() calls. if (!state.ending) endWritable(this, state, cb); + else if (typeof cb === 'function') { + if (!state.finished) { + this.once('finish', cb); + } else { + cb(new ERR_STREAM_ALREADY_FINISHED('end')); + } + } return this; }; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 6e3bfb29c03f27..fba18a13679e3f 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1128,6 +1128,9 @@ E('ERR_SOCKET_DGRAM_NOT_RUNNING', 'Not running', Error); E('ERR_SRI_PARSE', 'Subresource Integrity string %s had an unexpected at %d', SyntaxError); +E('ERR_STREAM_ALREADY_FINISHED', + 'Cannot call %s after a stream was finished', + Error); E('ERR_STREAM_CANNOT_PIPE', 'Cannot pipe, not readable', Error); E('ERR_STREAM_DESTROYED', 'Cannot call %s after a stream was destroyed', Error); E('ERR_STREAM_NULL_VALUES', 'May not write null values to stream', TypeError); diff --git a/test/parallel/test-http-outgoing-end-multiple.js b/test/parallel/test-http-outgoing-end-multiple.js new file mode 100644 index 00000000000000..7c43e1f59d5849 --- /dev/null +++ b/test/parallel/test-http-outgoing-end-multiple.js @@ -0,0 +1,27 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +const server = http.createServer(common.mustCall(function(req, res) { + res.end('testing ended state', common.mustCall()); + res.end(common.mustCall()); + res.on('finish', common.mustCall(() => { + res.end(common.mustCall((err) => { + assert.strictEqual(err.code, 'ERR_STREAM_ALREADY_FINISHED'); + server.close(); + })); + })); +})); + +server.listen(0); + +server.on('listening', common.mustCall(function() { + http + .request({ + port: server.address().port, + method: 'GET', + path: '/' + }) + .end(); +})); diff --git a/test/parallel/test-stream-writable-end-multiple.js b/test/parallel/test-stream-writable-end-multiple.js new file mode 100644 index 00000000000000..a94676ab8ab366 --- /dev/null +++ b/test/parallel/test-stream-writable-end-multiple.js @@ -0,0 +1,20 @@ +'use strict'; + +const common = require('../common'); + +const assert = require('assert'); +const stream = require('stream'); + +const writable = new stream.Writable(); + +writable._write = (chunk, encoding, cb) => { + setTimeout(() => cb(), 10); +}; + +writable.end('testing ended state', common.mustCall()); +writable.end(common.mustCall()); +writable.on('finish', common.mustCall(() => { + writable.end(common.mustCall((err) => { + assert.strictEqual(err.code, 'ERR_STREAM_ALREADY_FINISHED'); + })); +}));