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

ASGI: Wait for response to complete before sending disconnect message #919

Merged
merged 6 commits into from
May 12, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions httpx/_concurrency.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import typing
JayH5 marked this conversation as resolved.
Show resolved Hide resolved

import sniffio

if typing.TYPE_CHECKING: # pragma: no cover
try:
from typing import Protocol
except ImportError:
from typing_extensions import Protocol # type: ignore

class Event(Protocol):
def set(self) -> None:
...

# asyncio wait() returns True, but Trio returns None: ignore the return value.
async def wait(self) -> typing.Any:
...

def is_set(self) -> bool:
...


def create_event() -> "Event":
if sniffio.current_async_library() == "trio":
import trio

return trio.Event()
else:
import asyncio

return asyncio.Event()
17 changes: 11 additions & 6 deletions httpx/_dispatch/asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import httpcore

from .._concurrency import create_event
from .._content_streams import ByteStream


Expand Down Expand Up @@ -76,23 +77,27 @@ async def request(
status_code = None
response_headers = None
body_parts = []
request_complete = False
response_started = False
response_complete = False
response_complete = create_event()

headers = [] if headers is None else headers
stream = ByteStream(b"") if stream is None else stream

request_body_chunks = stream.__aiter__()

async def receive() -> dict:
nonlocal response_complete
nonlocal request_complete, response_complete

if response_complete:
if request_complete:
# Simulate blocking until the response is complete and then disconnect
Copy link
Member

Choose a reason for hiding this comment

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

I'd tweak this comment. We're not "simulating" blocking until the response is complete, we are blocking until the response is complete. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Took out "streaming" and then found the comment a bit redundant 🤔

await response_complete.wait()
return {"type": "http.disconnect"}

try:
body = await request_body_chunks.__anext__()
except StopAsyncIteration:
request_complete = True
return {"type": "http.request", "body": b"", "more_body": False}
return {"type": "http.request", "body": body, "more_body": True}

Expand All @@ -108,23 +113,23 @@ async def send(message: dict) -> None:
response_started = True

elif message["type"] == "http.response.body":
assert not response_complete
assert not response_complete.is_set()
body = message.get("body", b"")
more_body = message.get("more_body", False)

if body and method != b"HEAD":
body_parts.append(body)

if not more_body:
response_complete = True
response_complete.set()

try:
await self.app(scope, receive, send)
except Exception:
if self.raise_app_exceptions or not response_complete:
raise

assert response_complete
assert response_complete.is_set()
assert status_code is not None
assert response_headers is not None

Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pytest-asyncio
pytest-trio
pytest-cov
trio
trio-typing
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to add this dependency?

Copy link
Member Author

Choose a reason for hiding this comment

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

For any trio types to be type-checked this must be installed (the only type being checked is trio.Event). This package also has a mypy plugin that can be enabled for extra functionality, which I did enable in encode/httpcore#69, but I didn't bother here.

trustme
uvicorn
seed-isort-config
Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ def get_packages(package):
"idna==2.*",
"rfc3986>=1.3,<2",
"httpcore>=0.8.1",
"sniffio",
],
classifiers=[
"Development Status :: 4 - Beta",
Expand Down
3 changes: 1 addition & 2 deletions tests/test_asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ async def test_asgi_exc_after_response():
await client.get("http://www.example.org/")


@pytest.mark.asyncio
async def test_asgi_disconnect_after_response_complete():
async def test_asgi_disconnect_after_response_complete(async_environment):
disconnect = False

async def read_body(scope, receive, send):
Expand Down