fix(nodeenv): report a missing node archive instead of a traceback - #412
Merged
Merged
Conversation
A 404 on the node.js archive ended in a raw urllib traceback, without even naming the url when the arm64 build was missing: the x64 retry ran inside the "except HTTPError" block, so the second failure was chained onto the first and the warning with the url was never reached. Collect the attempted urls, retry outside the handler and report every one of them with the http status, plus a hint about "--list" and "--source" when the archive is simply not there. Closes #250
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.
Closes #250
Problem
A 404 on the node.js archive ended in a raw
urllibtraceback:Worse, on arm64 the url was never named at all. The x64 retry ran inside
the
except urllib2.HTTPErrorblock, so a second 404 was raised "duringhandling of the above exception" (hence the doubled traceback in the issue),
and
logger.warning('Failed to download from %s')was never reached.Fix
handler and failures are no longer chained.
--list/--sourcewhen the archive is simply not there.Before:
After:
A non-404 status (500, 403) is reported with its url and status, without the
"try another version" advice.
Tests
Four new tests cover the prebuilt 404, the arm64 fallback that also 404s, the
--source404 and a 500. Five existing tests assertedpytest.raises(HTTPError)and were updated to
SystemExit; their assertions sat inside theraisesblockand never ran, so they were moved out.
Manually verified on darwin-arm64: 22.11.0 installs, 14.21.3 still falls back to
x64, 99.99.99 fails with the message above.