Reformat error message output

[bhrutledge]

Currently, Twine displays error messages like this:

$ twine upload --repository testpypi -u __token__ -p pypi-token dist/*
Uploading distributions to https://test.pypi.org/legacy/
Uploading example-pkg-bhrutledge-0.0.4a3.tar.gz
100%|██████████████████████████████████████| 4.53k/4.53k [00:00<00:00, 19.0kB/s]
NOTE: Try --verbose to see response content.
HTTPError: 403 Client Error: Invalid or non-existent authentication information. See https://test.pypi.org/help/#invalid-auth for details for url: https://test.pypi.org/legacy/

I think something like this would be more helpful (though I'm not committed to this exact format):

Uploading distributions to https://test.pypi.org/legacy/
Uploading example-pkg-bhrutledge-0.0.4a3.tar.gz
100%|██████████████████████████████████████| 4.53k/4.53k [00:00<00:00, 19.0kB/s]

ERROR from https://test.pypi.org/legacy/: 403 Forbidden 
Invalid or non-existent authentication information. See https://test.pypi.org/help/#invalid-auth for details.
NOTE: Try --verbose to see response content.

For errors like this one from Warehouse, I think this makes the source and potential resolution more obvious, and might have mitigated issues like #577 and #424.

This might be a significant restructuring, since it seems like "expected" errors are displayed by a catch-all except:

twine/twine/__main__.py

Lines 23 to 27 in f7402e0

	def main():
	try:
	return cli.dispatch(sys.argv[1:])
	except (exceptions.TwineException, requests.HTTPError) as exc:
	return "{}: {}".format(exc.__class__.__name__, exc.args[0])

Related:

• Refactor/normalize exceptions · Issue #586
• Use red text when printing errors on command line · Issue #511

[sigmavirus24]

Any error can be formatted this way including things we might raise that halt upload or anything else.we choose to let bubble up. That means we'd have to start branching for each potential exception to add special formatting such that main looks like

try:
     return cli.dispatcy(sys.argv[1:])
except requests.HTTPError as exc:
     return """ERROR from ...
...
...
""".format(exc)
except exceptions.TwineException as exc:
     return ...
except SomeOtherError as exc:
     return

And my experience with highly specific chains like that is that contributors find a new exception path and try to shove it into some existing path.

My other concern is that people are checking the output of twine in automation (GitHub Actions, CI releases, etc.) and that changing it here will break their automation making this something of a breaking change.

[bhrutledge]

I do like the relative simplicity of the current handler in main(). One option I was thinking about was catching requests.HTTPError lower in the stack, and wrapping it in a TwineException, to give us more control of the formatting.

That said, it seems to me already have a bit of a special case for requests.HTTPError via the existing except (exceptions.TwineException, requests.HTTPError). Since these are the "expected" exceptions, splitting those up unto discrete except clauses doesn't feel excessive to me. All other "unexpected" exceptions would continue to bubble up, as they do now. And, we require (via style guide, or PR review) that new "expected" exceptions subclass TwineException.

I also wondered about relying on logging instead of exceptions for error reporting, but that would be a huge change.

My other concern is that people are checking the output of twine in automation

Yeah, that occurred to me, and I can see this change being released as Twine 4.0. I don't have stats on interactive vs. automated use, but it seems to me that the former is an important audience, and that making error messages easier to read would help users resolve issues on their own.

[sigmavirus24]

I also wondered about relying on logging instead of exceptions for error reporting

As we make Twine into more of a library, logging errors is completely inaccessible to users of the library. Further logging imposes formatting requirements that are unnecessary and far from user-friendly. Flake8 uses logging for debugging purposes but generally speaking only the maintainers are patient enough to read those when debugging an issue with a user.