map_exceptions in request.aclose()

[adamhooper]

Now, clients can catch an exception that occurs during close.

[closes #1463]

[adamhooper]

sigh I'm having a hard time reproducing this error on py36. Is that strictly necessary? I feel the code fix is obviously correct and the unit test is spectacularly difficult to write.

[florimondmanca]

@adamhooper Thanks!

For the unit test — I think we can leverage transport=.... I would suggest writing a custom transport that returns a dummy stream whose .aclose() method raises an httpcore exception, and make sure that an httpx one is raised on the client side. If that makes sense?

[florimondmanca]

Looking very neat!

Just a minor suggestion to increase test coverage slightly?

[florimondmanca]

Am I correct saying this would also reproduce for a regular client.request()? If so, should we update this to…?

Suggested change

	async with client.stream("GET", "http://example.com") as response:
	with pytest.raises(httpx.CloseError):
	await response.aclose()
	with pytest.raises(httpx.CloseError):
	await client.request("GET", "http://example.com")
	with pytest.raises(httpx.CloseError):
	async with client.stream("GET", "http://example.com"):
	pass # Close on exit.

(Also dropped the explicit response.aclose() since the expected behavior is that exiting the stream() async context manager would do that for us, and fail with the proper exception.)

[adamhooper]

You are correct. And the await client.request() test is a worthy addition.

You're the maintainer, so you are welcome to change to nix the aclose() in the test ... but first, I'll explain my motivation.

During debugging, I was surprised by #1464 -- a broken stream raises the "wrong" error on shutdown.

I imagine a reasonable __aexit__() might handle an errno=104 error -- by doing nothing and raising nothing. So I aimed for an explicit test that avoids __aexit__().

The change would be correct. But as a user, I find __axit__() behavior confusing; so I wrote a more-explicit test.

[tomchristie]

Yup, I like the way @adamhooper has approached the test case here, too.

[tomchristie]

Good catch here, yup. Happy with this one.