Skip to content

Fix readable stream as body check on safari #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 16, 2025
Merged

Fix readable stream as body check on safari #12

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7b72ec6
fix readable stream as body check on safari
bugii Jun 2, 2025
8f17913
add fetch call again to detect non-native 'fetch' envs and remove
bugii Jun 4, 2025
096ae5d
Merge branch 'main' into fix/readable-stream-support-check-safari
cesarenaldi Jun 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 21 additions & 20 deletions src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,32 +85,33 @@ function createMultipartStream(entries: readonly MultipartEntry[]): MultipartFor
};
}

async function detectStreamSupport(): Promise<boolean> {
async function detectStreamSupport() {
try {
const testStream = new ReadableStream({
start(controller) {
controller.enqueue(new Uint8Array([0])); // Minimal stream chunk
controller.close();
},
});

const request = new Request('data:text/plain;charset=utf-8,42', {
let duplexAccessed = false;
const hasContentType = new Request('https://', {
body: new ReadableStream(),
method: 'POST',
body: testStream,
// Required for streaming request body in some browsers,
// or it will fail and assume it's not supported
// @ts-ignore
duplex: 'half',
});
get duplex() {
duplexAccessed = true;
return 'half';
},
}).headers.has('Content-Type');

// If fails to handle fetch(Request), it's likely not a native implementation of
// Fetch API, so it's not supported.
await fetch(request.clone());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @bugii , thank you for contributing with this great alternative.

There's 2 usages we are trying to assess here, one is browsers and one is Node.js with patched or non-native Fetch API implementations.

This line specifically designed to fail with patched fetch that do not support fetch(Request) and instead just assumes fetch(uri, options). We have seen this in few circumstances one of which was the ElizaOS monorepo.

Copy link
Contributor Author

@bugii bugii Jun 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @cesarenaldi, thank you for the review! Was late at night and I overlooked that code somehow :D
I added the fetch call again.


const body = await request.text();
// if different from '\x00', it's likely not supported
return body === '\x00';
} catch {
const res = await fetch(
new Request('data:text/plain;charset=utf-8,42', {
method: 'POST',
body: new ReadableStream(),
// @ts-ignore
duplex: 'half',
}),
);
const body = await res.text();

return duplexAccessed && !hasContentType && body === '\x00';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent with the original check was to verify that the testStream was consumed by checking its text representation. A buffer with 0 byte in it renders the \x00 char.

Since the 'response' of a Data URI request is the data URI payload itself, would this be a string with '42' in this case and not match the \x00?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take this PR for a spin and see if we can adjust it.

} catch (error) {
return false;
}
}
Expand Down
Loading