Skip to content

Commit

Permalink
stream: always invoke end callback
Browse files Browse the repository at this point in the history
  • Loading branch information
ronag committed Aug 9, 2019
1 parent 885c644 commit f224b24
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 0 deletions.
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);
} 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');
}));
}));

0 comments on commit f224b24

Please sign in to comment.