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 1 commit
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
44 changes: 15 additions & 29 deletions src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,34 +85,20 @@ function createMultipartStream(entries: readonly MultipartEntry[]): MultipartFor
};
}

async function detectStreamSupport(): Promise<boolean> {
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', {
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',
});

// 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 {
return false;
}
function detectStreamSupport() {
let duplexAccessed = false;

const hasContentType = new Request('data:text/plain;charset=utf-8,42', {
Copy link
Member

Choose a reason for hiding this comment

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

Compared to the example in the google page you linked, here we are using a data URI, which includes a mime type. Wondering if this could make a difference, specifically with the hasContentType check.

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.

Sadly I cannot use an empty string as in the example (gives an error in node - which makes sense I guess since relative paths in servers are not really a thing). I think this one is safer, even though I would be very much surprised if the request url had an impact on the content type of the outgoing request before actually fetching it.

body: new ReadableStream(),
method: 'POST',
// @ts-ignore
get duplex() {
duplexAccessed = true;
return 'half';
},
}).headers.has('Content-Type');

return duplexAccessed && !hasContentType;
}

function createFormData(entries: readonly MultipartEntry[]): FormData {
Expand Down Expand Up @@ -163,7 +149,7 @@ export async function createMultipartRequestInit(
method: 'POST' | 'PUT',
entries: readonly MultipartEntry[],
): Promise<RequestInit> {
if (await detectStreamSupport()) {
if (detectStreamSupport()) {
const { stream, boundary } = createMultipartStream(entries);

return {
Expand Down