Skip to content

Commit 8a2bb7b

Browse files
authored
fix: Keep HttpResponse isinstance checks from consuming streamed responses (#1010)
`HttpResponse` now comes from `typing_extensions`, whose runtime protocol check looks attributes up statically. The `typing` implementation on Python 3.11 calls `hasattr`, which evaluates properties, so `isinstance(response, HttpResponse)` on an unread streaming response either raised or silently buffered the whole body. Adds streaming regression tests (unit and integration) and a parametrized built-in-client fixture they run on. Split out of #1006. Stacked on #1009. *✍️ Drafted by Claude Code*
1 parent 95f1c12 commit 8a2bb7b

7 files changed

Lines changed: 165 additions & 7 deletions

File tree

pyproject.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ dependencies = [
2828
"impit~=0.13.0",
2929
"more_itertools>=10.0.0",
3030
"pydantic[email]>=2.11.0",
31+
# 4.6.0 is the first release whose runtime protocol checks look attributes up statically.
32+
"typing-extensions>=4.6.0",
3133
]
3234

3335
[project.optional-dependencies]

src/apify_client/http_clients/_base.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,14 @@
77
from abc import ABC, abstractmethod
88
from datetime import UTC, datetime, timedelta
99
from importlib import metadata
10-
from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable
10+
from typing import TYPE_CHECKING, Any
1111
from urllib.parse import urlencode
1212

13+
# `Protocol` comes from `typing_extensions`, not `typing`, because its runtime `isinstance` check looks attributes
14+
# up statically. The `typing` implementation on Python 3.11 calls `hasattr`, which evaluates properties. On an
15+
# unread streaming response, that either raises or silently buffers the whole body.
16+
from typing_extensions import Protocol, runtime_checkable
17+
1318
from apify_client._consts import (
1419
DEFAULT_MAX_RETRIES,
1520
DEFAULT_MIN_DELAY_BETWEEN_RETRIES,

tests/integration/test_dataset.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
from apify_client._models import Dataset, DatasetListItem, DatasetStatistics, ListOfDatasets
2222
from apify_client._resource_clients.dataset import DatasetItemsPage
2323
from apify_client.errors import ApifyApiError
24+
from apify_client.http_clients import HttpResponse
2425

2526
if TYPE_CHECKING:
2627
from apify_client import ApifyClient, ApifyClientAsync
@@ -727,7 +728,7 @@ async def get_items() -> DatasetItemsPage:
727728
if is_async:
728729
assert isinstance(stream_ctx, AbstractAsyncContextManager)
729730
async with stream_ctx as response:
730-
assert isinstance(response, impit.Response)
731+
assert isinstance(response, HttpResponse)
731732
assert response.status_code == 200
732733
content = await response.aread()
733734
items = json.loads(content)
@@ -736,7 +737,7 @@ async def get_items() -> DatasetItemsPage:
736737
else:
737738
assert isinstance(stream_ctx, AbstractContextManager)
738739
with stream_ctx as response:
739-
assert isinstance(response, impit.Response)
740+
assert isinstance(response, HttpResponse)
740741
assert response.status_code == 200
741742
content = response.read()
742743
items = json.loads(content)

tests/integration/test_key_value_store.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
)
2222
from apify_client._models import KeyValueStore, KeyValueStoreKey, ListOfKeys, ListOfKeyValueStores
2323
from apify_client.errors import ApifyApiError
24+
from apify_client.http_clients import HttpResponse
2425

2526
if TYPE_CHECKING:
2627
from apify_client import ApifyClient, ApifyClientAsync
@@ -198,14 +199,14 @@ async def test_stream_record_signature(
198199
signature=test_kvs_of_another_user.keys_signature[key],
199200
) as stream: # ty: ignore[invalid-context-manager]
200201
assert isinstance(stream, dict)
201-
value = json.loads(stream['value'].content.decode('utf-8'))
202+
value = json.loads((await stream['value'].aread()).decode('utf-8'))
202203
else:
203204
with kvs.stream_record(
204205
key,
205206
signature=test_kvs_of_another_user.keys_signature[key],
206207
) as stream: # ty: ignore[invalid-context-manager]
207208
assert isinstance(stream, dict)
208-
value = json.loads(stream['value'].content.decode('utf-8'))
209+
value = json.loads(stream['value'].read().decode('utf-8'))
209210

210211
assert test_kvs_of_another_user.expected_content[key] == value
211212

@@ -726,11 +727,15 @@ async def added_record_exists() -> bool:
726727
if is_async:
727728
async with store_client.stream_record('stream-key') as stream: # ty: ignore[invalid-context-manager]
728729
assert isinstance(stream, dict)
729-
value = json.loads(stream['value'].content.decode('utf-8'))
730+
response = stream['value']
731+
assert isinstance(response, HttpResponse)
732+
value = json.loads((await response.aread()).decode('utf-8'))
730733
else:
731734
with store_client.stream_record('stream-key') as stream: # ty: ignore[invalid-context-manager]
732735
assert isinstance(stream, dict)
733-
value = json.loads(stream['value'].content.decode('utf-8'))
736+
response = stream['value']
737+
assert isinstance(response, HttpResponse)
738+
value = json.loads(response.read().decode('utf-8'))
734739

735740
assert value == {'data': 'streamed'}
736741
finally:

tests/unit/conftest.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
import pytest
77
from pytest_httpserver import HTTPServer
88

9+
from apify_client.http_clients import HttpClient, HttpClientAsync, ImpitHttpClient, ImpitHttpClientAsync
10+
911
if TYPE_CHECKING:
1012
from collections.abc import Iterable
1113

@@ -28,3 +30,15 @@ def httpserver(make_httpserver: HTTPServer) -> Iterable[HTTPServer]:
2830
server = make_httpserver
2931
yield server
3032
server.clear()
33+
34+
35+
@pytest.fixture(params=[pytest.param(ImpitHttpClient, id='impit')])
36+
def http_client_class(request: pytest.FixtureRequest) -> type[HttpClient]:
37+
"""Return each built-in synchronous HTTP client class."""
38+
return request.param
39+
40+
41+
@pytest.fixture(params=[pytest.param(ImpitHttpClientAsync, id='impit')])
42+
def http_client_async_class(request: pytest.FixtureRequest) -> type[HttpClientAsync]:
43+
"""Return each built-in asynchronous HTTP client class."""
44+
return request.param
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
from __future__ import annotations
2+
3+
from typing import TYPE_CHECKING
4+
5+
from apify_client import ApifyClient, ApifyClientAsync
6+
from apify_client.http_clients import HttpResponse
7+
8+
if TYPE_CHECKING:
9+
from typing import Any
10+
11+
from pytest_httpserver import HTTPServer
12+
13+
from apify_client.http_clients import HttpClient, HttpClientAsync
14+
15+
16+
DATASET_ID = 'test-dataset-id'
17+
KVS_ID = 'test-kvs-id'
18+
RECORD_KEY = 'test-record-key'
19+
STREAM_CONTENT = b'[{"id": 1}]'
20+
21+
22+
def test_dataset_stream_items_sync(
23+
httpserver: HTTPServer,
24+
http_client_class: type[HttpClient],
25+
) -> None:
26+
"""Dataset streams expose a transport-independent response that can be read synchronously."""
27+
httpserver.expect_request(f'/v2/datasets/{DATASET_ID}/items').respond_with_data(STREAM_CONTENT)
28+
api_url = httpserver.url_for('/').removesuffix('/')
29+
client = ApifyClient.with_custom_http_client(
30+
api_url=api_url,
31+
http_client=http_client_class(),
32+
)
33+
34+
with client.dataset(DATASET_ID).stream_items(item_format='json') as response:
35+
assert isinstance(response, HttpResponse)
36+
assert response.read() == STREAM_CONTENT
37+
38+
39+
async def test_dataset_stream_items_async(
40+
httpserver: HTTPServer,
41+
http_client_async_class: type[HttpClientAsync],
42+
) -> None:
43+
"""Dataset streams expose a transport-independent response that can be read asynchronously."""
44+
httpserver.expect_request(f'/v2/datasets/{DATASET_ID}/items').respond_with_data(STREAM_CONTENT)
45+
api_url = httpserver.url_for('/').removesuffix('/')
46+
client = ApifyClientAsync.with_custom_http_client(
47+
api_url=api_url,
48+
http_client=http_client_async_class(),
49+
)
50+
51+
async with client.dataset(DATASET_ID).stream_items(item_format='json') as response:
52+
assert isinstance(response, HttpResponse)
53+
assert await response.aread() == STREAM_CONTENT
54+
55+
56+
def test_key_value_store_stream_record_sync(
57+
httpserver: HTTPServer,
58+
http_client_class: type[HttpClient],
59+
) -> None:
60+
"""KVS streams require reading the generic response before consuming its content."""
61+
httpserver.expect_request(f'/v2/key-value-stores/{KVS_ID}/records/{RECORD_KEY}').respond_with_data(STREAM_CONTENT)
62+
api_url = httpserver.url_for('/').removesuffix('/')
63+
client = ApifyClient.with_custom_http_client(
64+
api_url=api_url,
65+
http_client=http_client_class(),
66+
)
67+
68+
with client.key_value_store(KVS_ID).stream_record(RECORD_KEY) as record:
69+
assert isinstance(record, dict)
70+
response = record['value']
71+
assert isinstance(response, HttpResponse)
72+
assert response.read() == STREAM_CONTENT
73+
74+
75+
async def test_key_value_store_stream_record_async(
76+
httpserver: HTTPServer,
77+
http_client_async_class: type[HttpClientAsync],
78+
) -> None:
79+
"""KVS streams require asynchronously reading the generic response before consuming its content."""
80+
httpserver.expect_request(f'/v2/key-value-stores/{KVS_ID}/records/{RECORD_KEY}').respond_with_data(STREAM_CONTENT)
81+
api_url = httpserver.url_for('/').removesuffix('/')
82+
client = ApifyClientAsync.with_custom_http_client(
83+
api_url=api_url,
84+
http_client=http_client_async_class(),
85+
)
86+
87+
async with client.key_value_store(KVS_ID).stream_record(RECORD_KEY) as record:
88+
assert isinstance(record, dict)
89+
response = record['value']
90+
assert isinstance(response, HttpResponse)
91+
assert await response.aread() == STREAM_CONTENT
92+
93+
94+
def test_protocol_check_leaves_stream_unread_sync(
95+
httpserver: HTTPServer,
96+
http_client_class: type[HttpClient],
97+
) -> None:
98+
"""Checking a streaming response against the protocol inspects it without pulling the body off the wire."""
99+
httpserver.expect_request(f'/v2/datasets/{DATASET_ID}/items').respond_with_data(STREAM_CONTENT)
100+
api_url = httpserver.url_for('/').removesuffix('/')
101+
client = ApifyClient.with_custom_http_client(
102+
api_url=api_url,
103+
http_client=http_client_class(),
104+
)
105+
106+
with client.dataset(DATASET_ID).stream_items(item_format='json') as response:
107+
assert isinstance(response, HttpResponse)
108+
# `is_stream_consumed` is transport state, not part of the protocol, but the built-in client exposes it.
109+
raw: Any = response
110+
assert raw.is_stream_consumed is False
111+
112+
113+
async def test_protocol_check_leaves_stream_unread_async(
114+
httpserver: HTTPServer,
115+
http_client_async_class: type[HttpClientAsync],
116+
) -> None:
117+
"""Checking a streaming response against the protocol inspects it without pulling the body off the wire."""
118+
httpserver.expect_request(f'/v2/datasets/{DATASET_ID}/items').respond_with_data(STREAM_CONTENT)
119+
api_url = httpserver.url_for('/').removesuffix('/')
120+
client = ApifyClientAsync.with_custom_http_client(
121+
api_url=api_url,
122+
http_client=http_client_async_class(),
123+
)
124+
125+
async with client.dataset(DATASET_ID).stream_items(item_format='json') as response:
126+
assert isinstance(response, HttpResponse)
127+
# `is_stream_consumed` is transport state, not part of the protocol, but the built-in client exposes it.
128+
raw: Any = response
129+
assert raw.is_stream_consumed is False

uv.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)