-
-
Notifications
You must be signed in to change notification settings - Fork 659
feat: throw error when maxRedirections is used with undici.request() #4311
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
The maxRedirections option was removed in v7 but no error was thrown when users tried to use it, making migration confusing. Now throws a clear error message directing users to use the redirect interceptor instead. Fixes #4310 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matteo Collina <hello@matteocollina.com>
CLAUDE.md
Outdated
**WebAssembly Components:** | ||
- HTTP parser is WebAssembly-based | ||
- Requires Docker to rebuild | ||
- Pre-built binaries included in `lib/llhttp/` |
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.
What's this?
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.
LGTM but some test failures
The maxRedirections option was removed in v7 but no error was thrown when users tried to use it, making migration confusing. Now throws a clear error message directing users to use the redirect interceptor instead. Changes: - Add validation in Request constructor to throw error for maxRedirections > 0 - Allow maxRedirections: 0 for internal fetch usage - Update redirect interceptor to work with new validation - Fix failing tests by removing maxRedirections from request options - Add CLAUDE.md to .gitignore Fixes #4310 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matteo Collina <hello@matteocollina.com>
3d4aa95
to
7cee925
Compare
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@Ethan-Arrowood good spot! |
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.
lgtm
The maxRedirections option was removed in v7 but no error was thrown when users tried to use it, making migration confusing. Now throws a clear error message directing users to use the redirect interceptor instead.
Fixes #4310
🤖 Generated with Claude Code
This relates to...
Rationale
Changes
Features
Bug Fixes
Breaking Changes and Deprecations
Status