Fix UnboundLocalError in fetch_v2fly_domains when domain file fails to parse - #290
Open
msarg44 wants to merge 1 commit into
Open
Conversation
…o parse The try/except in fetch_v2fly_domains only assigned 'domains' inside the try block. If parse_v2fly_domain_file raised (e.g. the v2fly repo clone fails or a company file is missing/unreadable), the except branch appended the error but 'domains' was never bound, so 'sorted(list(domains))' on the final line raised UnboundLocalError and crashed the update regardless of the (correctly) collected error. Initialize 'domains = set()' before the try so the except path returns empty domains plus the error instead of crashing. Adds a regression test verifying graceful handling when the domain file cannot be parsed.
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.
Summary
BaseProvider.fetch_v2fly_domains()incloudcheck/providers/base.pyonly assigns the localdomainsvariable inside thetryblock:If
parse_v2fly_domain_file()raises (e.g. the v2fly community repo clone fails, or the company'sdata/<company>file is missing/unreadable), theexceptbranch correctly appends the error, butdomainswas never bound. The finalreturn sorted(list(domains)), errorsthen raisesUnboundLocalError, crashing the whole domain-update pass for the provider — even though a perfectly good error message was already collected.Change
Initialize
domains = set()before thetryblock so the failure path returns the collected error plus empty domains instead of crashing:This matches the documented contract (
fetch_v2fly_domains()returns(domains, errors)) — downstream callers inupdate_domains()already handle per-provider errors gracefully.Testing
Added a regression test
test_v2fly_domains_missing_fileincloudcheck_update/test_cloudcheck_update.pythat patchesparse_v2fly_domain_fileto raise, then asserts the call returns([], [error])rather than crashing withUnboundLocalError.