Skip to content

Conversation

@fantix
Copy link

@fantix fantix commented Dec 13, 2019

So that on the server side, http.disconnect can be received when timed out.

@fantix fantix force-pushed the testclient-timeout branch from fd1d0aa to 6888895 Compare December 19, 2019 00:05
@fantix fantix force-pushed the testclient-timeout branch from 6888895 to ce54e9a Compare December 19, 2019 00:08
@blueyed
Copy link
Contributor

blueyed commented Jan 6, 2020

Looks interesting. I've quickly tried if it would abort an await asyncio.sleep(5), but that does not appear to be the case. Would it possible to handle this? (i.e. that the timeout for client.get would ensure that it takes no longer than that)

@fantix
Copy link
Author

fantix commented Jan 9, 2020

Oh, this patch only added timeout feature to the TestClient and the requests-powered ASGI "server". That means, on timeout, the TestClient will "disconnect" the connection, and an http.disconnect event is raised in the ASGI "server". However, it is totally up to the ASGI application to handle the http.disconnect event - whether abort the current handling task or whatever else. Please see an example here where we abort any pending request handlers when http.disconnect is detected.

@Kludex
Copy link
Owner

Kludex commented Jan 29, 2022

Thanks for the PR @fantix ! Sorry for taking so long to act here 😅

I think this feature is extremely useful. It's going to be useful to test StreamingResponse, a bug we have on BaseHTTPMiddleware and also another PR that is coming around that needs a test when the client disconnects.

I'll be closing this issue, and proceed with it on #1446. I've co-authored yourself, as I've just adapted this code.

@Kludex Kludex closed this Jan 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants