Rust: move the redirect option onto the builder — 3.7.0 was breaking - #122
Merged
Conversation
I shipped RequestInit.follow_redirects in 3.7.0 as a minor. Adding a public
field to a struct consumers construct is a BREAKING change in Rust semver, and
the evidence was immediate: building the SmooAI monorepo against 3.7.0 fails
with error[E0063] in 129 exhaustive `RequestInit { .. }` constructors across
about forty crates.
I had that evidence an hour earlier and misread it. Adding the field broke the
fetch repo's own tests; I patched them and treated it as test churn rather than
the semver signal it was.
The option is now FetchBuilder::with_follow_redirects, which is the shape Go
and .NET already use, and RequestInit is back to its 3.6.2 fields. This is not
just damage control — reqwest's redirect policy is per-Client rather than
per-request, so the builder was the correct home from the start and the struct
field was working against the grain.
Kept fully additive so 3.6.2 -> 3.7.1 needs no consumer changes:
client::fetch keeps its exact signature and delegates to a new
client::fetch_with_redirect_policy that takes the extra argument.
The wiring bug this repeats is worth naming. with_follow_redirects stored a
value that reached FetchClient and then went nowhere, because both fetch
methods still called the old entry point — the setter compiled, the option did
nothing. That is the same failure as .NET's SmooFetchBuilder.Build() dropping
FollowRedirects during its field-by-field copy, and in both languages only an
end-to-end test that actually watched for the redirect hop caught it. A unit
test asserting "the builder stored the flag" would have passed in both.
3.7.0 is yanked from crates.io. The other four languages are unaffected: Python
added a defaulted dataclass field, Go and .NET added builder methods, and
TypeScript's change was to stop overriding a caller's existing option.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 85b0158 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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
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.
What went wrong in 3.7.0
I added
RequestInit.follow_redirectsand shipped it as a minor. Adding a public field to a struct consumers construct is a breaking change in Rust semver.Building the SmooAI monorepo against 3.7.0:
129 exhaustive constructors across ~40 crates.
I had this evidence an hour before publishing — adding the field broke this repo's own tests, and I patched them and treated it as test churn rather than the semver signal it was.
The fix
The option moves to
FetchBuilder::with_follow_redirects, matching what Go and .NET already do, andRequestInitreturns to its 3.6.2 fields.This isn't only damage control. reqwest's redirect policy is per-Client, not per-request, so the builder was the correct home from the start — the struct field was working against the grain of the underlying library.
Kept fully additive
client::fetchkeeps its exact signature and delegates to a newclient::fetch_with_redirect_policythat takes the extra argument. So3.6.2 → 3.7.1requires no consumer changes at all.The wiring bug worth naming
with_follow_redirectsstored a value that reachedFetchClientand then went nowhere — bothfetchmethods still called the old entry point. The setter compiled; the option did nothing.That is the same failure as .NET's
SmooFetchBuilder.Build()droppingFollowRedirectsduring its field-by-field copy. In both languages, only an end-to-end test that actually watched for the redirect hop caught it. A unit test asserting "the builder stored the flag" would have passed in both.Verification
cargo fmt --checkandclippy -D warningscleanRequestInithas nofollow_redirectsfield — only a doc comment pointing at the builder3.7.0 will be yanked from crates.io once this publishes. The other four languages are unaffected: Python added a defaulted dataclass field, Go and .NET added builder methods, TypeScript's change was to stop overriding a caller's existing option.
Pearl: th-86dc77
🤖 Generated with Claude Code