refactor: add Client::prepare_request and avoid unneeded Result#2124
Closed
katrinafyi wants to merge 7 commits into
Closed
refactor: add Client::prepare_request and avoid unneeded Result#2124katrinafyi wants to merge 7 commits into
katrinafyi wants to merge 7 commits into
Conversation
there was only one Err case and it was extremely rare. now, we just transform it into a Status instead of using `?`.
Member
Author
|
78f4ad0 changes the output for remaps leading to invalid URLs. old, split across log message and link checking error: new: |
Member
Author
|
abandoning because this isn't the right approach. instead of a new function on client, we should make a new struct Remapper/Filter or even out this into the request chain. the unneeded Result refactor is worth landing in a separate pr |
katrinafyi
added a commit
to rina-forks/lychee
that referenced
this pull request
Apr 15, 2026
this means that the Err case of `Client::check` will only happen due to remap failures. this means the error cases we have to handle are much clearer. this commit re-lands this refactor which was previously part of lycheeverse#2124. there's a bit more context in that PR.
thomas-zahner
pushed a commit
to rina-forks/lychee
that referenced
this pull request
Apr 20, 2026
this means that the Err case of `Client::check` will only happen due to remap failures. this means the error cases we have to handle are much clearer. this commit re-lands this refactor which was previously part of lycheeverse#2124. there's a bit more context in that 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
1 participant
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.
First, this PR splits the old Client::check into Client::prepare_request and Client::check_prepared_request (while keeping the old check signature for backwards compat). prepare_request is a new function which centralises any "pre-check transformations". This means remaps can be done once in Client::prepare_request and re-used for both cache key and link checking. This also means we avoid weird cases like "what if cache_key is None" - this should never happen because if cache_key is None, then that link is already invalid.
Second, this PR tightens up some unneeded Result. I found that Client::check would only return Err in two cases: remap failure, and a rare insecure check error. After this change, remap errors are in Client::check_prepared_request, leaving the insecure http error in Client::check_prepared_request. I just tweak this one case so the check_prepared_request can unconditionally return Response. I think this is a bit neater and avoids the need for check_url in check.rs.
Follow-up to #2109 (review). Prompted by #2123 (comment).