-
-
Notifications
You must be signed in to change notification settings - Fork 694
fix(fetch): prevent infinite retry loop on 401 responses #4756
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.
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
Conversation
`isTraversableNavigable()` was returning `true` unconditionally, which caused an infinite retry loop when the server responded with 401. In Node.js environment, there is no traversable navigable that can prompt the user for credentials (unlike browsers). The 401 retry logic at step 14 of HTTP-network-or-cache fetch requires user interaction to provide credentials, which is not possible in Node.js. Returning `false` disables the incomplete 401 retry logic that was causing the infinite loop, while preserving the URL credentials functionality added in nodejs#4747. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes an infinite retry loop that occurs when the server responds with a 401 status code in Node.js environments. The issue was caused by isTraversableNavigable() returning true unconditionally, triggering retry logic designed for browsers that can prompt users for credentials—a capability Node.js lacks.
Changes:
- Modified
isTraversableNavigable()to returnfalseinstead oftrue - Added explanatory comments describing why Node.js environments cannot support traversable navigables
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Stumbled independently upon the same issue so can confirm described issue is real, didn't confirm the fix though. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4756 +/- ##
==========================================
- Coverage 93.22% 93.15% -0.08%
==========================================
Files 109 109
Lines 34024 34026 +2
==========================================
- Hits 31720 31697 -23
- Misses 2304 2329 +25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Ensures that when a server responds with 401 Unauthorized, fetch does not enter an infinite retry loop. This test verifies the fix for isTraversableNavigable() returning false in Node.js environment. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
KhafraDev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't fix the issue, it (essentially) reverts 3388513 and brings back the bug with basic authentication in websockets (node test/web-platform-tests/wpt-runner.mjs run /websockets/basic-auth.any)
isTraversableNavigable()was returningtrueunconditionally, which caused an infinite retry loop when the server responded with 401.In Node.js environment, there is no traversable navigable that can prompt the user for credentials (unlike browsers). The 401 retry logic at step 14 of HTTP-network-or-cache fetch requires user interaction to provide credentials, which is not possible in Node.js.
Returning
falsedisables the incomplete 401 retry logic that was causing the infinite loop, while preserving the URL credentials functionality added in #4747.This relates to...
Rationale
Changes
Features
Bug Fixes
Breaking Changes and Deprecations
Status