8000 fix(adapter): construct Request with a url and options by jacquesg · Pull Request #6524 · axios/axios · GitHub
[go: up one dir, main page]
Skip to content

fix(adapter): construct Request with a url and options #6524

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

Jump to bottom
Merged
merged 1 commit into from
Aug 25, 2024
Merged

fix(adapter): construct Request with a url and options #6524

merged 1 commit into from
Aug 25, 2024

Conversation

jacquesg
Copy link
Contributor
@jacquesg jacquesg commented Aug 2, 2024

Whilst testing the upload progress features with the new fetch adapter, I've found that when passing a FormData object as the body, the onUploadProgress callback was never triggered.

Debugging this, I found that the getBodyLength function was always returning 0. The problematic line:

(await new Request(body).arrayBuffer()).byteLength

After quite a bit of debugging, I double checked the MDN documentation, which doesn't explicitly state that it is okay to pass a FormData object to the constructor:

input
Defines the resource that you wish to fetch. This can either be:

A string containing the URL of the resource you want to fetch. The URL may be relative to the base URL, which is the document's baseURI in a window context, or WorkerGlobalScope.location in a worker context.
A Request object, effectively creating a copy. Note the following behavioral updates to retain security while making the constructor less likely to throw exceptions:
If this object exists on another origin to the constructor call, the Request.referrer is stripped out.
If this object has a Request.mode of navigate, the mode value is converted to same-origin.

Changing the above to the following fixes it for me (Chrome 126 on macOS)

const _request = new Request(platform.origin, {
  method: 'POST',
  body,
});
await _request.arrayBuffer()).byteLength;

See: https://developer.mozilla.org/en-US/docs/Web/API/Request/Request

@micooz
Copy link
micooz commented Aug 10, 2024

+1, works for me too.

But there seems to be another issue related to the Streams API using Fetch.

axios/lib/adapters/fetch.js

Lines 158 to 168 in c6cce43

request = new Request(url, {
...fetchOptions,
signal: composedSignal,
method: method.toUpperCase(),
headers: headers.normalize().toJSON(),
body: data,
duplex: "half",
credentials: withCredentials
});
let response = await fetch(request);

Calling the last line let response = await fetch(request); will throw an error: net::ERR_H2_OR_QUIC_REQUIRED, which means when request body is ReadableStream(returned by trackStream()), only HTTP2 or HTTP3 is supported.

This is troublesome for local development or environments that do not support HTTP2 or HTTP3.

Any ideas for a solution?

Related issue:

@jacquesg jacquesg force-pushed the fix/adapter-request-construct branch from 5f05c6a to 3ba8c9a Compare August 17, 2024 17:52
@jacquesg jacquesg force-pushed the fix/adapter-request-construct branch from 3ba8c9a to 1552a3d Compare August 23, 2024 17:07
@DigitalBrainJS DigitalBrainJS merged commit 085f568 into axios:v1.x Aug 25, 2024
9 checks passed
@github-actions github-actions bot mentioned this pull request Aug 30, 2024
Merged
@github-actions GitHub Actions
Copy link
Contributor

Hi, @jacquesg! This PR has been published in v1.7.6 release. Thank you for your contribution ❤️!

This was referenced Sep 1, 2024
Merged
Closed
@leonardoGit99 leonardoGit99 mentioned this pull request Nov 5, 2024
Open
@sagarkumar446 sagarkumar446 mentioned this pull request Nov 5, 2024
Merged
@nejidevelops nejidevelops mentioned this pull request Nov 6, 2024
Open
@bhat-abhishek bhat-abhishek mentioned this pull request Nov 6, 2024
Open
@pradumchintamani pradumchintamani mentioned this pull request Nov 6, 2024
Open
@saileshbro saileshbro mentioned this pull request Nov 6, 2024
Merged
@Erenkrs Erenkrs mentioned this pull request Nov 6, 2024
Open
@dpesall dpesall mentioned this pull request Nov 6, 2024
Open
@MikeTeddyOmondi MikeTeddyOmondi mentioned this pull request Nov 6, 2024
Closed
This was referenced Nov 7, 2024
Merged
Merged
@ClovisBN ClovisBN mentioned this pull request Nov 7, 2024
Open
@snyk-io snyk-io bot mentioned this pull request Nov 8, 2024
Closed
@Matas-V Matas-V mentioned this pull request Nov 8, 2024
Open
@snyk-io snyk-io bot mentioned this pull request Nov 9, 2024
Merged
@aurelienfvre aurelienfvre mentioned this pull request Nov 10, 2024
Open
@apute098 apute098 mentioned this pull request Nov 10, 2024
Open
@caniszczyk caniszczyk mentioned this pull request Nov 10, 2024
Closed
@blacklight blacklight A983 mentioned this pull request Nov 11, 2024
Merged
@JasperVanNiekerk JasperVanNiekerk mentioned this pull request Nov 11, 2024
Open
@lunoon lunoon mentioned this pull request Nov 12, 2024
Open
@Niaz-estefaie Niaz-estefaie mentioned this pull request Nov 12, 2024
Open
@Jpalmer95 Jpalmer95 mentioned this pull request Nov 12, 2024
Open
@AhmedHanye AhmedHanye mentioned this pull request Nov 12, 2024
Closed
@pradumchintamani pradumchintamani mentioned this pull request Nov 13, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jacquesg @micooz @DigitalBrainJS
0