Skip to content

Commit

Permalink
stream: improve comments regarding end() errors
Browse files Browse the repository at this point in the history
Cleans up comments and TODOs and tries to provide a
more detail description in regards to error behavior
of end().

PR-URL: #32839
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
ronag committed Apr 16, 2020
1 parent 15cc2b6 commit 4a6a5c3
Showing 1 changed file with 7 additions and 10 deletions.
17 changes: 7 additions & 10 deletions lib/_stream_writable.js
Expand Up @@ -598,20 +598,16 @@ Writable.prototype.end = function(chunk, encoding, cb) {
if (typeof cb !== 'function')
cb = nop;

// Ignore unnecessary end() calls.
// TODO(ronag): Compat. Allow end() after destroy().
// 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.
if (!state.errored && !state.ending) {
endWritable(this, state, cb);
} else if (state.finished) {
const err = new ERR_STREAM_ALREADY_FINISHED('end');
process.nextTick(cb, err);
// TODO(ronag): Compat. Don't error the stream.
// errorOrDestroy(this, err, true);
process.nextTick(cb, new ERR_STREAM_ALREADY_FINISHED('end'));
} else if (state.destroyed) {
const err = new ERR_STREAM_DESTROYED('end');
process.nextTick(cb, err);
// TODO(ronag): Compat. Don't error the stream.
// errorOrDestroy(this, err, true);
process.nextTick(cb, new ERR_STREAM_DESTROYED('end'));
} else if (cb !== nop) {
onFinished(this, state, cb);
}
Expand Down Expand Up @@ -720,6 +716,7 @@ function onCorkedFinish(corkReq, state, err) {
state.corkedRequestsFree.next = corkReq;
}

// TODO(ronag): Avoid using events to implement internal logic.
function onFinished(stream, state, cb) {
function onerror(err) {
stream.removeListener('finish', onfinish);
Expand Down

0 comments on commit 4a6a5c3

Please sign in to comment.