fix(fetch): warn when httpVersion is set in the Fetch adapter#10570
Open
JivinSardine wants to merge 1 commit intoaxios:v1.xfrom
Open
fix(fetch): warn when httpVersion is set in the Fetch adapter#10570JivinSardine wants to merge 1 commit intoaxios:v1.xfrom
JivinSardine wants to merge 1 commit intoaxios:v1.xfrom
Conversation
Contributor
There was a problem hiding this comment.
No issues found across 1 file
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Add a console warning when httpVersion is set in environments using the Fetch adapter (e.g., Bun, browser), since the Fetch API does not support the httpVersion option. Fixes axios#7535 When using Bun.js or browser environments, axios silently ignores the httpVersion option, which can cause confusion when users expect HTTP/2 behavior. This change issues a clear warning to inform users that httpVersion is not supported by the Fetch adapter.
b977437 to
7b74905
Compare
Collaborator
Actually, HTTP2 is supported by browsers' fetch, but it's done automatically under the hood, without the ability to select the protocol version manually. As we can see, in the opinion of the fetch spec designers, adding an option to select exactly the protocol version wasn't their priority, so I think the protocol version isn't critical enough for adding a warning to the frontend code, where code size should be reduced at almost any cost. It would be better to highlight this behaviour in the docs rather than adding a notice to the runtime. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add a console warning when
httpVersionis set in environments using the Fetch adapter (e.g., Bun.js, browser), since the Fetch API does not support thehttpVersionoption.Fixes #7535
Problem
When using Bun.js or other browser-like environments, axios uses the Fetch adapter. The
httpVersionoption is silently ignored in this adapter, which can cause confusion when users expect HTTP/2 behavior.For example, this config causes requests to hang in Bun.js without any error or warning:
Solution
Issue a
console.warnwhenhttpVersionis set to a non-default value in the Fetch adapter:Note: The Node.js HTTP adapter (
lib/adapters/http.js) already properly handleshttpVersionand supports both HTTP/1.1 and HTTP/2.Checklist
CHANGELOG.mdhas been updated with a relevant entry