stream: Writable.end(chunk) after end is an error
authorisaacs <i@izs.me>
Sat, 2 Mar 2013 20:25:16 +0000 (12:25 -0800)
committerisaacs <i@izs.me>
Sun, 3 Mar 2013 00:09:16 +0000 (16:09 -0800)
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.

lib/_stream_writable.js
test/simple/test-stream2-writable.js

index 62e07057637bc234a19b484d26f36b0278d5e627..8db4f55a07b46d702419e09b662063a5af04e3e9 100644 (file)
@@ -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,28 @@ Writable.prototype.end = function(chunk, encoding, cb) {
     encoding = null;
   }
 
-  state.ending = true;
   if (chunk)
-    this.write(chunk, encoding, cb);
-  else if (state.length === 0 && !state.finishing && !state.finished) {
+    this.write(chunk, encoding);
+
+  // ignore unnecessary end() calls.
+  if (!state.ending && !state.ended && !state.finished)
+    endWritable(this, state, !!chunk, cb);
+};
+
+function endWritable(stream, state, hadChunk, cb) {
+  state.ending = true;
+  if (!hadChunk &&
+      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;
 };
index efd49021bb7ea53e22e0e363a9b11a3a3d15c412..537660263b2dd25ff9987f890bea83905842a6df 100644 (file)
@@ -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();
+  });
+});