stream: ensure errorEmitted is always set

nodejs · Aug 9, 2019 · 51d97bf · 51d97bf
1 parent 885c644
commit 51d97bf
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 4 deletions.
diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js
@@ -429,13 +429,11 @@ function onwriteError(stream, state, sync, er, cb) {
     // This can emit finish, and it will always happen
     // after error
     process.nextTick(finishMaybe, stream, state);
-    stream._writableState.errorEmitted = true;
     errorOrDestroy(stream, er);
   } else {
     // The caller expect this to happen before if
     // it is async
     cb(er);
-    stream._writableState.errorEmitted = true;
     errorOrDestroy(stream, er);
     // This can emit finish, but finish must
     // always follow error

diff --git a/lib/internal/streams/destroy.js b/lib/internal/streams/destroy.js
@@ -101,10 +101,14 @@ function errorOrDestroy(stream, err) {
   const rState = stream._readableState;
   const wState = stream._writableState;
 
-  if ((rState && rState.autoDestroy) || (wState && wState.autoDestroy))
+  if ((rState && rState.autoDestroy) || (wState && wState.autoDestroy)) {
     stream.destroy(err);
-  else
+  } else {
+    if (wState) {
+      wState.errorEmitted = true;
+    }
     stream.emit('error', err);
+  }
 }
 
 

diff --git a/test/parallel/test-stream2-writable.js b/test/parallel/test-stream2-writable.js
@@ -402,3 +402,45 @@ const helloWorldBuffer = Buffer.from('hello world');
   w.write(Buffer.allocUnsafe(1));
   w.end(Buffer.allocUnsafe(0));
 }
+
+{
+  // Verify that error is only emitted once when failing in _finish.
+  const w = new W();
+
+  w._final = common.mustCall(function(cb) {
+    cb(new Error('test'));
+  });
+  w._write = function(chunk, e, cb) {
+    process.nextTick(cb);
+  };
+  w.once('error', common.mustCall((err) => {
+    assert.strictEqual(w._writableState.errorEmitted, true);
+    assert.strictEqual(err.message, 'test');
+    w.on('error', common.mustNotCall());
+    w.destroy(new Error());
+  }));
+  w.end();
+}
+
+{
+  // Verify that error is only emitted once when failing in write.
+  const w = new W();
+  w.on('error', common.mustCall((err) => {
+    assert.strictEqual(w._writableState.errorEmitted, true);
+    assert.strictEqual(err.code, 'ERR_STREAM_NULL_VALUES');
+  }));
+  w.write(null);
+  w.destroy(new Error());
+}
+
+{
+  // Verify that error is only emitted once when failing in write after end.
+  const w = new W();
+  w.on('error', common.mustCall((err) => {
+    assert.strictEqual(w._writableState.errorEmitted, true);
+    assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END');
+  }));
+  w.end();
+  w.write('hello');
+  w.destroy(new Error());
+}