stream: add isDisturbed helper

Adds a helper util used to determine whether a stream has been disturbed (read or cancelled). Refs: #39627 PR-URL: #39628 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Backport-PR-URL: #39819
nodejs · ronag · Aug 2, 2021 · Aug 22, 2021 · Aug 20, 2021 · cd6f6c86024e46d5b9a598e5d5347606d3a4070e
commit cd6f6c86024e46d5b9a598e5d5347606d3a4070e
diff --git a/doc/api/stream.md b/doc/api/stream.md
@@ -1259,16 +1259,27 @@ added: v11.4.0
 Is `true` if it is safe to call [`readable.read()`][stream-read], which means
 the stream has not been destroyed or emitted `'error'` or `'end'`.
 
+##### `readable.readableAborted`
+<!-- YAML
+added: REPLACEME
+-->
+
+> Stability: 1 - Experimental
+
+* {boolean}
+
+Returns whether the stream was destroyed or errored before emitting `'end'`.
+
 ##### `readable.readableDidRead`
 <!-- YAML
 added: v16.7.0
 -->
 
+> Stability: 1 - Experimental
+
 * {boolean}
 
-Allows determining if the stream has been or is about to be read.
-Returns true if `'data'`, `'end'`, `'error'` or `'close'` has been
-emitted.
+Returns whether `'data'` has been emitted.
 
 ##### `readable.readableEncoding`
 <!-- YAML
@@ -1943,6 +1954,18 @@ Calling `Readable.from(string)` or `Readable.from(buffer)` will not have
 the strings or buffers be iterated to match the other streams semantics
 for performance reasons.
 
+### `stream.Readable.isDisturbed(stream)`
+<!-- YAML
+added: REPLACEME
+-->
+
+> Stability: 1 - Experimental
+
+* `stream` {stream.Readable|ReadableStream}
+* Returns: `boolean`
+
+Returns whether the stream has been read from or cancelled.
+
 ### `stream.addAbortSignal(signal, stream)`
 <!-- YAML
 added: v15.4.0

diff --git a/lib/internal/streams/readable.js b/lib/internal/streams/readable.js
@@ -1191,12 +1191,15 @@ ObjectDefineProperties(Readable.prototype, {
   readableDidRead: {
     enumerable: false,
     get: function() {
-      return (
-        this._readableState.dataEmitted ||
-        this._readableState.endEmitted ||
-        this._readableState.errorEmitted ||
-        this._readableState.closeEmitted
-      );
+      return this._readableState.dataEmitted;
+    }
+  },
+
+  readableAborted: {
+    enumerable: false,
+    get: function() {
+      return !!(this._readableState.destroyed || this._readableState.errored) &&
+        !this._readableState.endEmitted;
     }
   },
 

diff --git a/lib/internal/streams/utils.js b/lib/internal/streams/utils.js
@@ -1,10 +1,13 @@
 'use strict';
 
 const {
+  Symbol,
   SymbolAsyncIterator,
   SymbolIterator,
 } = primordials;
 
+const kIsDisturbed = Symbol('kIsDisturbed');
+
 function isReadable(obj) {
   return !!(obj && typeof obj.pipe === 'function' &&
             typeof obj.on === 'function');
@@ -27,7 +30,16 @@ function isIterable(obj, isAsync) {
     typeof obj[SymbolIterator] === 'function';
 }
 
+function isDisturbed(stream) {
+  return !!(stream && (
+    stream.readableDidRead ||
+    stream.readableAborted ||
+    stream[kIsDisturbed]
+  ));
+}
+
 module.exports = {
+  isDisturbed,
   isIterable,
   isReadable,
   isStream,

diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js
@@ -80,6 +80,10 @@ const {
   queueMicrotask,
 } = require('internal/process/task_queues');
 
+const {
+  kIsDisturbed,
+} = require('internal/streams/utils');
+
 const {
   ArrayBufferViewGetBuffer,
   ArrayBufferViewGetByteLength,
@@ -200,6 +204,7 @@ class ReadableStream {
         promise: undefined,
       }
     };
+
     // The spec requires handling of the strategy first
     // here. Specifically, if getting the size and
     // highWaterMark from the strategy fail, that has
@@ -232,6 +237,10 @@ class ReadableStream {
     return makeTransferable(this);
   }
 
+  get [kIsDisturbed]() {
+    return this[kState].disturbed;
+  }
+
   /**
    * @readonly
    * @type {boolean}

diff --git a/lib/stream.js b/lib/stream.js
@@ -36,6 +36,7 @@ const internalBuffer = require('internal/buffer');
 const promises = require('stream/promises');
 
 const Stream = module.exports = require('internal/streams/legacy').Stream;
+Stream.isDisturbed = require('internal/streams/utils').isDisturbed;
 Stream.Readable = require('internal/streams/readable');
 Stream.Writable = require('internal/streams/writable');
 Stream.Duplex = require('internal/streams/duplex');

diff --git a/test/parallel/test-stream-readable-aborted.js b/test/parallel/test-stream-readable-aborted.js
@@ -0,0 +1,57 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { Readable } = require('stream');
+
+{
+  const readable = new Readable({
+    read() {
+    }
+  });
+  assert.strictEqual(readable.readableAborted, false);
+  readable.destroy();
+  assert.strictEqual(readable.readableAborted, true);
+}
+
+{
+  const readable = new Readable({
+    read() {
+    }
+  });
+  assert.strictEqual(readable.readableAborted, false);
+  readable.push(null);
+  readable.destroy();
+  assert.strictEqual(readable.readableAborted, true);
+}
+
+{
+  const readable = new Readable({
+    read() {
+    }
+  });
+  assert.strictEqual(readable.readableAborted, false);
+  readable.push('asd');
+  readable.destroy();
+  assert.strictEqual(readable.readableAborted, true);
+}
+
+{
+  const readable = new Readable({
+    read() {
+    }
+  });
+  assert.strictEqual(readable.readableAborted, false);
+  readable.push('asd');
+  readable.push(null);
+  assert.strictEqual(readable.readableAborted, false);
+  readable.on('end', common.mustCall(() => {
+    assert.strictEqual(readable.readableAborted, false);
+    readable.destroy();
+    assert.strictEqual(readable.readableAborted, false);
+    queueMicrotask(() => {
+      assert.strictEqual(readable.readableAborted, false);
+    });
+  }));
+  readable.resume();
+}
diff --git a/test/parallel/test-stream-readable-didRead.js b/test/parallel/test-stream-readable-didRead.js
@@ -1,12 +1,13 @@
 'use strict';
 const common = require('../common');
 const assert = require('assert');
-const Readable = require('stream').Readable;
+const { isDisturbed, Readable } = require('stream');
 
 function noop() {}
 
 function check(readable, data, fn) {
   assert.strictEqual(readable.readableDidRead, false);
+  assert.strictEqual(isDisturbed(readable), false);
   if (data === -1) {
     readable.on('error', common.mustCall());
     readable.on('data', common.mustNotCall());
@@ -27,7 +28,10 @@ function check(readable, data, fn) {
   readable.on('close', common.mustCall());
   fn();
   setImmediate(() => {
-    assert.strictEqual(readable.readableDidRead, true);
+    assert.strictEqual(readable.readableDidRead, data > 0);
+    if (data > 0) {
+      assert.strictEqual(isDisturbed(readable), true);
+    }
   });
 }
 

diff --git a/test/parallel/test-whatwg-readablestream.js b/test/parallel/test-whatwg-readablestream.js
@@ -2,6 +2,7 @@
 'use strict';
 
 const common = require('../common');
+const { isDisturbed } = require('stream');
 const assert = require('assert');
 const {
   isPromise,
@@ -1520,3 +1521,52 @@ class Source {
   readableByteStreamControllerClose(controller);
   readableByteStreamControllerEnqueue(controller);
 }
+
+{
+  const stream = new ReadableStream({
+    start(controller) {
+      controller.enqueue('a');
+      controller.close();
+    },
+    pull: common.mustNotCall(),
+  });
+
+  const reader = stream.getReader();
+  (async () => {
+    isDisturbed(stream, false);
+    await reader.read();
+    isDisturbed(stream, true);
+  })().then(common.mustCall());
+}
+
+{
+  const stream = new ReadableStream({
+    start(controller) {
+      controller.close();
+    },
+    pull: common.mustNotCall(),
+  });
+
+  const reader = stream.getReader();
+  (async () => {
+    isDisturbed(stream, false);
+    await reader.read();
+    isDisturbed(stream, true);
+  })().then(common.mustCall());
+}
+
+{
+  const stream = new ReadableStream({
+    start(controller) {
+    },
+    pull: common.mustNotCall(),
+  });
+  stream.cancel();
+
+  const reader = stream.getReader();
+  (async () => {
+    isDisturbed(stream, false);
+    await reader.read();
+    isDisturbed(stream, true);
+  })().then(common.mustCall());
+}