Rename HTTPError to RequestError, introduce HTTPStatusError

[sileht]

feat: introduce HTTPStatusError

Fixes #867

feat: rename HTTPError to RequestError

Unlike HTTPStatusError, HTTPError name is not always related to HTTP status code >= 400.

The base class is renamed RequestError, to avoid confusion with
HTTPStatusError.

Related #867

[sileht]

I don't see any automated CI here, there is plan to add one ? Can I help ? I just see other pr have travis-ci

[florimondmanca]

@sileht Seems like Travis is under scheduled maintenance right now:

We will be undergoing scheduled maintenance. During this time, your builds triggered via GitHub will still be accepted, but all public services (including api.travis-ci.org) cannot be reached. Any scheduled or running builds will be canceled and re-enqueued to begin running after the maintenance window.

It seems your build should start automatically once maintenance is over. :-)

[florimondmanca]

Thanks! This is looking great to me already.

Any places in the docs that need updating?

Also going to ask a review from @tomchristie on this API change. :)

[florimondmanca]

Going to try and trigger the Travis CI build...

[sileht]

Any places in the docs that need updating?

That's surprise me, but I didn't find any reference of HTTPError in the documentation.

[florimondmanca]

LGTM. :-) I also think this is definitely an improvement over the ambiguity of HTTPError as the base exception class in HTTPX.

Waiting for some more feedback before merging…

Possible follow-up work might be adding documenting exception classes that are exposed on httpx.[...] to the docs, since I believe we've been considering them as public API for a while.

[sileht]

I added a commit with some basic doc.

[florimondmanca]

Thanks!

I added a commit with some basic doc.

Ideally, can we move this to a separate PR? In general we try to have one PR do one thing, but this one PR already does two things… :-)

This doc addition is small, but I've got a bunch of copy nits, and it'd be better not to mix with this feature PR (which is ready to be merged without the docs addition).

(I'm also wondering if exceptions docs wouldn't be better served by adding an Exceptions section in the Developer Interface, with info generated from exception docstrings...)

[sileht]

I have split it.

[florimondmanca]

A quick suggestion about something I hadn't clicked about before :-)

[florimondmanca]

Another improvement suggestion… Sorry for submitting this in pieces!

[florimondmanca]

Since HTTPStatusError will in practice always both have a .request and a .response attached, we should probably ensure this by overriding the constructor?

This will help type checkers understand that these attributes aren't Optional in this case. (Otherwise, accessing eg exc.response.status_code would raise a type checking error like Item "None" of "Optional[Response]" has no attribute "status_code".)

Suggested change

	def __init__(self, *args: typing.Any, response: "Response") -> None:
	super().__init__(*args)
	self.request: "Request" = response.request
	self.response: "Response" = response

(The type comments are important to override the annotations from the base class.)

[florimondmanca]

I'm going to close this for now — there's been some changes and improvements since March in the realm of the exception hierarchy, and I think we might need to properly discuss and agree on the changes proposed here beforehand. Filed #1064.

Either way, @sileht thank you so much for putting time into this. This is definitely super valuable and I'm sure we'll be able to reuse this very soon!