Skip to content
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

Reformat HTTP errors #666

Merged
merged 10 commits into from Jun 22, 2020
Merged

Conversation

bhrutledge
Copy link
Contributor

@bhrutledge bhrutledge commented Jun 18, 2020

Fixes #587, in which my main goal was to make the source and potential resolution of errors from Warehouse more obvious, which might have mitigated issues like #577 and #424.

New format:

Screen Shot 2020-06-21 at 5 44 40 AM

Old format:

Screen Shot 2020-06-18 at 8 54 07 AM

This intentionally wraps at 80 characters, but I've also seen reports where it wasn't wrapped, and the Warehouse URL was buried at the end of the line.

twine/__main__.py Outdated Show resolved Hide resolved
tests/test_integration.py Outdated Show resolved Hide resolved
Comment on lines -131 to 133
@pytest.fixture
@pytest.fixture(scope="session")
def sampleproject_dist(tmp_path_factory):
checkout = tmp_path_factory.mktemp("sampleproject", numbered=False)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick fix to avoid a "file exists" error from mktemp when this fixture is used in more than one test.

@bhrutledge bhrutledge added this to In progress in Helpful errors via automation Jun 18, 2020
.coveragerc Show resolved Hide resolved
Copy link
Contributor Author

@bhrutledge bhrutledge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sigmavirus24 I made the requested changes. If you're able, I think it'd be nice to merge today or tomorrow, to include in 3.2.0, but no worries if not.

tests/test_integration.py Outdated Show resolved Hide resolved
Comment on lines 36 to 42
status_code = exc.response.status_code
status_phrase = http.HTTPStatus(status_code).phrase
return (
f"{exc.__class__.__name__} from {exc.response.url}: "
f"{status_code} {status_phrase}\n"
f"{exc.response.reason}"
)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

except exceptions.TwineException as exc:
result = f"{exc.__class__.__name__}: {exc.args[0]}"

return _format_error(result) if isinstance(result, str) else result
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored in 3fd259a to one return and one _format_error to ensure that any current and future string results are treated as an error, which is how they're interpreted by sys.exit.

@bhrutledge bhrutledge added this to the 3.2.0 milestone Jun 20, 2020
twine/__main__.py Outdated Show resolved Hide resolved
@bhrutledge bhrutledge merged commit 33d9611 into pypa:master Jun 22, 2020
Helpful errors automation moved this from In progress to Done Jun 22, 2020
@bhrutledge bhrutledge deleted the 587-http-error-format branch June 22, 2020 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Reformat error message output
3 participants