Open KhafraDev opened 1 year ago
I think that's the expected behavior, all native async iterators behave like that AFAIK (in browsers as well). The reason is that CreateIterResultObject
requires the result object to inherit from Object.prototype
, and part of resolving the promise includes checking if the .then
property of the result object (see https://tc39.es/ecma262/#sec-promise-resolve-functions step 9) is callable.
The above code snippet rejects in Firefox and Chromium, passes on Safari AFAICT. I think Safari is incorrect here.
Unfortunately there's no other way to read from a ReadableStream/ReadableStream reader, which the fetch spec requires us to do (see: https://streams.spec.whatwg.org/#readablestreamdefaultreader-read-all-bytes). Using its async iterator has the same issue.
The spec says Perform ! [ReadableStreamDefaultReaderRead](https://streams.spec.whatwg.org/#readable-stream-default-reader-read)(reader, readRequest).
, which is exported by lib/internal/webstreams
: https://github.com/nodejs/node/blob/d42628d05ac21a22dbb6cc42a32c7aebc78f40d2/lib/internal/webstreams/readablestream.js#L2124-L2140
If Undici was calling this directly, would that workaround the issue?
probably not, I assume that's being used by .read already edit: yeah, that's in the stack trace
It's being used by read
indeed, but the issue you're having is that read
returns Promise.resolve(anObjectThatInheritsFromObjectPrototype)
, which is what is accessing Object.prototype.then
. Calling readableStreamDefaultReaderRead
directly should workaround the issue (because you don't deal with promises anymore), or what am I missing?
The error is coming from node:internal/webstreams/readablestream:771:27
(the stack trace is in the issue)
The error is coming from
node:internal/webstreams/readablestream:771:27
(the stack trace is in the issue)
That's calling Promise.resolve
though: https://github.com/nodejs/node/blob/d42628d05ac21a22dbb6cc42a32c7aebc78f40d2/lib/internal/webstreams/readablestream.js#L771
unless I'm not understanding something, readableStreamDefaultReaderRead
eventually calls that resolve, which means exposing it won't fix our issue.
here is a better example of the code that's being tested (which browsers pass according to the WPT link):
import { Response } from 'undici'
const hello = new TextEncoder().encode('hello');
const bye = new TextEncoder().encode('bye');
const rs = new ReadableStream({
start (controller) {
controller.enqueue(hello);
controller.close();
}
});
const resp = new Response(rs);
Object.defineProperty(Object.prototype, 'then', {
get () {
throw new Error('..')
}
})
const text = await resp.text();
delete Object.prototype.then;
unless I'm not understanding something,
readableStreamDefaultReaderRead
eventually calls that resolve, which means exposing it won't fix our issue.
It's calling a method of the readRequest
you pass as an argument, so my thinking is that you should be able to override the specific method that's causing an issue.
Version
v19.7.0
Platform
Microsoft Windows NT 10.0.19045.0 x64
Subsystem
web streams
What steps will reproduce the bug?
How often does it reproduce? Is there a required condition?
No response
What is the expected behavior?
No error thrown
What do you see instead?
Additional information
This is a WPT that I discovered working on fetch, https://github.com/web-platform-tests/wpt/blob/master/fetch/api/response/response-stream-with-broken-then.any.js