Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Commit

Permalink
stream: Writable.end(chunk) after end is an error
Browse files Browse the repository at this point in the history
Calling end(data) calls write(data).  Doing this after end should
raise a 'write after end' error.

However, because end() calls were previously ignored on already
ended streams, this error was confusingly suppressed, even though the
data never is written, and cannot get to the other side.

This is a re-hash of 5222d19, but
without assuming that the data passed to end() is valid, and thus
breaking a bunch of tests.
  • Loading branch information
isaacs committed Mar 4, 2013
1 parent 0c57b31 commit 384f1be
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 13 deletions.
31 changes: 18 additions & 13 deletions lib/_stream_writable.js
Expand Up @@ -304,10 +304,6 @@ Writable.prototype._write = function(chunk, cb) {
Writable.prototype.end = function(chunk, encoding, cb) {
var state = this._writableState;

// ignore unnecessary end() calls.
if (state.ending || state.ended || state.finished)
return;

if (typeof chunk === 'function') {
cb = chunk;
chunk = null;
Expand All @@ -317,17 +313,26 @@ Writable.prototype.end = function(chunk, encoding, cb) {
encoding = null;
}

if (typeof chunk !== 'undefined' && chunk !== null)
this.write(chunk, encoding);

// ignore unnecessary end() calls.
if (!state.ending && !state.ended && !state.finished)
endWritable(this, state, cb);
};

function endWritable(stream, state, cb) {
state.ending = true;
if (chunk)
this.write(chunk, encoding, cb);
else if (state.length === 0 && !state.finishing && !state.finished) {
if (state.length === 0 && !state.finishing) {
state.finishing = true;
this.emit('finish');
stream.emit('finish');
state.finished = true;
if (cb) process.nextTick(cb);
} else if (cb) {
this.once('finish', cb);
}

if (cb) {
if (state.finished || state.finishing)
process.nextTick(cb);
else
stream.once('finish', cb);
}
state.ended = true;
};
}
16 changes: 16 additions & 0 deletions test/simple/test-stream2-writable.js
Expand Up @@ -311,3 +311,19 @@ test('duplexes are pipable', function(t) {
assert(!gotError);
t.end();
});

test('end(chunk) two times is an error', function(t) {
var w = new W();
w._write = function() {};
var gotError = false;
w.on('error', function(er) {
gotError = true;
t.equal(er.message, 'write after end');
});
w.end('this is the end');
w.end('and so is this');
process.nextTick(function() {
assert(gotError);
t.end();
});
});

0 comments on commit 384f1be

Please sign in to comment.