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

Don't swallow asyncio.CancelledError #459

Merged
merged 5 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
12 changes: 10 additions & 2 deletions aiohttp_sse/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import asyncio
import io
import re
from contextlib import suppress
import sys
from types import TracebackType
from typing import Any, Mapping, Optional, Type, TypeVar, Union, overload

Expand Down Expand Up @@ -142,8 +142,16 @@
"""
if self._ping_task is None:
raise RuntimeError("Response is not started")
with suppress(asyncio.CancelledError):

try:
await self._ping_task
except asyncio.CancelledError:
if (
sys.version_info >= (3, 11)
and (task := asyncio.current_task())
and task.cancelling()
):
raise

Check warning on line 154 in aiohttp_sse/__init__.py

View check run for this annotation

Codecov / codecov/patch

aiohttp_sse/__init__.py#L154

Added line #L154 was not covered by tests

def stop_streaming(self) -> None:
"""Used in conjunction with ``wait`` could be called from other task
Expand Down
34 changes: 34 additions & 0 deletions tests/test_sse.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import asyncio
import sys
from typing import Awaitable, Callable, List

import pytest
Expand Down Expand Up @@ -525,3 +526,36 @@
streamed_data = await resp.text()

assert streamed_data == "data: foo\r\n\r\n"


@pytest.mark.skipif(
sys.version_info < (3, 11),
reason=".cancelling() missing in older versions",
)
async def test_cancelled_not_swallowed(aiohttp_client: ClientFixture) -> None:
"""Test asyncio.CancelledError is not swallowed by .wait().

Relates to:
https://github.com/aio-libs/aiohttp-sse/issues/458
"""

async def endless_task(sse: EventSourceResponse) -> None:
while True:
await sse.wait()

Check warning on line 544 in tests/test_sse.py

View check run for this annotation

Codecov / codecov/patch

tests/test_sse.py#L542-L544

Added lines #L542 - L544 were not covered by tests

async def handler(request: web.Request) -> EventSourceResponse:
async with sse_response(request) as sse:
task = asyncio.create_task(endless_task(sse))
await asyncio.sleep(0)
task.cancel()
await task

Check warning on line 551 in tests/test_sse.py

View check run for this annotation

Codecov / codecov/patch

tests/test_sse.py#L546-L551

Added lines #L546 - L551 were not covered by tests

return sse # pragma: no cover

app = web.Application()
app.router.add_route("GET", "/", handler)

Check warning on line 556 in tests/test_sse.py

View check run for this annotation

Codecov / codecov/patch

tests/test_sse.py#L555-L556

Added lines #L555 - L556 were not covered by tests

client = await aiohttp_client(app)

Check warning on line 558 in tests/test_sse.py

View check run for this annotation

Codecov / codecov/patch

tests/test_sse.py#L558

Added line #L558 was not covered by tests

async with client.get("/") as response:
assert 200 == response.status

Check warning on line 561 in tests/test_sse.py

View check run for this annotation

Codecov / codecov/patch

tests/test_sse.py#L561

Added line #L561 was not covered by tests
Loading