From 384f1be739b747f6589169443ce687b44d434e09 Mon Sep 17 00:00:00 2001 From: isaacs Date: Sat, 2 Mar 2013 12:25:16 -0800 Subject: [PATCH] stream: Writable.end(chunk) after end is an error 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 5222d19a11ed0e29d207da0e8c9c8e0e3b18ad78, but without assuming that the data passed to end() is valid, and thus breaking a bunch of tests. --- lib/_stream_writable.js | 31 ++++++++++++++++------------ test/simple/test-stream2-writable.js | 16 ++++++++++++++ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js index 62e0705763..debbab66de 100644 --- a/lib/_stream_writable.js +++ b/lib/_stream_writable.js @@ -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; @@ -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; -}; +} diff --git a/test/simple/test-stream2-writable.js b/test/simple/test-stream2-writable.js index efd49021bb..537660263b 100644 --- a/test/simple/test-stream2-writable.js +++ b/test/simple/test-stream2-writable.js @@ -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(); + }); +});