Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stream: always invoke end callback #28687

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -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()`.

<a id="ERR_STREAM_ALREADY_FINISHED"></a>
### ERR_STREAM_ALREADY_FINISHED

A stream method was called that cannot complete because the stream was
finished.

<a id="ERR_STREAM_NULL_VALUES"></a>
### ERR_STREAM_NULL_VALUES

Expand Down
8 changes: 8 additions & 0 deletions lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand Down
8 changes: 8 additions & 0 deletions lib/_stream_writable.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
ronag marked this conversation as resolved.
Show resolved Hide resolved
} else {
cb(new ERR_STREAM_ALREADY_FINISHED('end'));
}
}

return this;
};
Expand Down
3 changes: 3 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
27 changes: 27 additions & 0 deletions test/parallel/test-http-outgoing-end-multiple.js
Original file line number Diff line number Diff line change
@@ -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();
}));
20 changes: 20 additions & 0 deletions test/parallel/test-stream-writable-end-multiple.js
Original file line number Diff line number Diff line change
@@ -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');
}));
}));