ReadableStream.from({}) returns confusing error

[rotu]

Version

v22.4.1

Platform

Darwin Hypothesis.local 24.0.0 Darwin Kernel Version 24.0.0: Mon Jul  1 21:58:28 PDT 2024; root:xnu-11215.0.132.501.1~1/RELEASE_ARM64_T8103 arm64

Subsystem

No response

What steps will reproduce the bug?

ReadableStream.from({})

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior? Why is that the expected behavior?

It should show a more descriptive error message. Maybe "value is not iterable".

What do you see instead?

> ReadableStream.from({})
Uncaught TypeError: FunctionPrototypeCall is not a function
    at getIterator (node:internal/webstreams/util:246:20)
    at getIterator (node:internal/webstreams/util:238:36)
    at readableStreamFromIterable (node:internal/webstreams/readablestream:1315:26)
    at ReadableStream.from (node:internal/webstreams/readablestream:311:12)

Additional information

No response

[rotu]

Note there is another apparent bug here. For some reason, FunctionPrototypeCall is reporting its own name instead of its first argument. That is, the faulting line should probably print an error that says: "'method' is not a function"

https://github.com/nodejs/node/blob/fc233627ed447be0bb893d8cba8e3ff1eba58658/lib/internal/webstreams/util.js#L246C3-L246C55

  const iterator = FunctionPrototypeCall(method, obj);

Edit: reported issue in V8 https://issues.chromium.org/issues/352528966

[RedYetiDev]

The way I see it, the error message content displayed is a V8 bug, but the fact that it even got to that error (and not a validation error) may be an issue with Node.js.

[jakecastelli]

Had a quick look, I think something like this would prevent it:

diff --git a/lib/internal/webstreams/util.js b/lib/internal/webstreams/util.js
index cce67d5b04..2b043f987c 100644
--- a/lib/internal/webstreams/util.js
+++ b/lib/internal/webstreams/util.js
@@ -243,6 +243,10 @@ function getIterator(obj, kind = 'sync', method) {
     }
   }
 
+  if (method === undefined) {
+    throw new ERR_INVALID_STATE.TypeError('The object is not iterable');
+  }
+
   const iterator = FunctionPrototypeCall(method, obj);

@debadree25 any idea?

[debadree25]

I believe that method is a 1-1 mapping of https://tc39.es/ecma262/#sec-getiterator and we are missing Step 1.b.ii and Step 3. so i think you diff should solve it @jakecastelli

[jakecastelli]

Thanks! I am not too familiar with webstream and wpt (roughly know the idea) do we need to add a test for this in our test/parallel or should this test be ideally in wpt upstream.

[debadree25]

I think test/parallel should be fine

[rotu]

I believe that method is a 1-1 mapping of https://tc39.es/ecma262/#sec-getiterator and we are missing Step 1.b.ii and Step 3. so i think you diff should solve it

It seems to have started from GetIterator but it has an extra parameter and seems to have absorbed GetIteratorFromMethod. The recursive call here of getIterator seems to be a slight code smell.