Skip to content
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
16 changes: 11 additions & 5 deletions src/apify/scrapy/requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,18 +146,24 @@ def to_apify_request(scrapy_request: ScrapyRequest, spider: Spider) -> ApifyRequ
request_kwargs['user_data'] = user_data if isinstance(user_data, dict) else {}

# Store an Apify-platform view of the headers. The authoritative copy with exact bytes travels in
# the serialized scrapy_request below, so non-UTF-8 headers (which make `to_unicode_dict()` raise) are
# tolerated rather than dropping the whole request.
# the serialized scrapy_request below, so if any header is not valid UTF-8, the headers are left out here and
# the request still converts.
#
# Trade-off: with `use_extended_unique_key=True` the unique key includes the headers, so when non-UTF-8
# headers are omitted here two requests differing only in those headers share a unique key and one is
# deduplicated away. This is rare (header values are normally ASCII/UTF-8) and still strictly better than
# the old behavior, which dropped such requests entirely.
if isinstance(scrapy_request.headers, Headers):
try:
# `to_unicode_dict()` yields str keys and values, but Scrapy types it as `UserDict[str | bytes, Any]`,
# so the mapping goes through the model's validator instead of a cast.
request_kwargs['headers'] = HttpHeaders.model_validate(scrapy_request.headers.to_unicode_dict())
# Decoded strictly, so a non-UTF-8 value raises. Multiple values of one header are joined with ','.
# Once Scrapy < 2.15 is dropped, `Headers.to_tuple_list()` could replace this, but it decodes with
# `errors='replace'` and returns one tuple per value.
headers = scrapy_request.headers
header_dict = {
key.decode(headers.encoding): b','.join(values).decode(headers.encoding)
for key, values in headers.items()
}
request_kwargs['headers'] = HttpHeaders(header_dict)
except UnicodeDecodeError:
logger.warning(
'Could not represent Scrapy request headers as Apify request headers (non-UTF-8 values); '
Expand Down
37 changes: 29 additions & 8 deletions tests/unit/scrapy/requests/test_to_apify_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ def spider() -> DummySpider:

def test_creates_simple_request(spider: Spider) -> None:
"""A simple Scrapy request converts to an Apify request carrying the serialized `scrapy_request`."""
scrapy_request = Request(url='https://example.com')
scrapy_request = Request(url='https://example.com/')

apify_request = to_apify_request(scrapy_request, spider)
assert apify_request is not None
assert apify_request.url == 'https://example.com'
assert apify_request.url == 'https://example.com/'

user_data = apify_request.user_data
assert 'scrapy_request' in user_data
Expand All @@ -45,22 +45,43 @@ def test_handles_headers(spider: Spider) -> None:
apify_request = to_apify_request(scrapy_request, spider)

assert apify_request is not None
expected_headers = cast('dict[str, str]', dict(scrapy_request_headers.to_unicode_dict()))
assert apify_request.headers == HttpHeaders(expected_headers)
assert apify_request.headers == HttpHeaders({'Authorization': 'Bearer access_token'})


def test_joins_multi_value_headers(spider: Spider) -> None:
"""A header with several values is carried onto the Apify request with the values joined by a comma."""
scrapy_request = Request(url='https://example.com', headers={'Accept': ['text/html', 'application/json']})

apify_request = to_apify_request(scrapy_request, spider)

assert apify_request is not None
assert apify_request.headers == HttpHeaders({'Accept': 'text/html,application/json'})


def test_non_utf8_headers_are_left_out(spider: Spider, caplog: pytest.LogCaptureFixture) -> None:
"""A non-UTF-8 header leaves all headers off the Apify request with a warning, while the request still converts."""
scrapy_request = Request(url='https://example.com', headers={'Accept': 'text/html', 'X-Raw': b'\xff'})

with caplog.at_level(logging.WARNING, logger='apify.scrapy.requests'):
apify_request = to_apify_request(scrapy_request, spider)

assert apify_request is not None
assert apify_request.headers == HttpHeaders()
assert 'non-UTF-8 values' in caplog.text


def test_without_id_and_unique_key(spider: Spider) -> None:
"""A request without an id or unique key converts, preserving its user data."""
scrapy_request = Request(
url='https://example.com',
url='https://example.com/',
method='GET',
meta={'userData': {'some_user_data': 'test'}},
)

apify_request = to_apify_request(scrapy_request, spider)

assert apify_request is not None
assert apify_request.url == 'https://example.com'
assert apify_request.url == 'https://example.com/'
assert apify_request.method == 'GET'

user_data = apify_request.user_data
Expand All @@ -73,7 +94,7 @@ def test_without_id_and_unique_key(spider: Spider) -> None:
def test_with_id_and_unique_key(spider: Spider) -> None:
"""An explicit `apify_request_unique_key` in `meta` becomes the Apify request's unique key."""
scrapy_request = Request(
url='https://example.com',
url='https://example.com/',
method='GET',
meta={
'apify_request_unique_key': 'https://example.com',
Expand All @@ -84,7 +105,7 @@ def test_with_id_and_unique_key(spider: Spider) -> None:
apify_request = to_apify_request(scrapy_request, spider)
assert apify_request is not None

assert apify_request.url == 'https://example.com'
assert apify_request.url == 'https://example.com/'
assert apify_request.method == 'GET'
assert apify_request.unique_key == 'https://example.com'

Expand Down
12 changes: 6 additions & 6 deletions tests/unit/scrapy/requests/test_to_scrapy_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ def spider() -> DummySpider:
return DummySpider()


# A JSON-encoded (pickle-free) Scrapy request for a GET request to https://apify.com, encoded exactly
# A JSON-encoded (pickle-free) Scrapy request for a GET request to https://apify.com/, encoded exactly
# the way `to_apify_request` stores it under `user_data['scrapy_request']` (JSON, no outer base64).
_SCRAPY_REQUEST_JSON_ENCODED = encode_to_json(
{
'url': 'https://apify.com',
'url': 'https://apify.com/',
'callback': None,
'errback': None,
'headers': {
Expand All @@ -54,7 +54,7 @@ def spider() -> DummySpider:
def test_without_reconstruction(spider: Spider) -> None:
"""An Apify request without a stored `scrapy_request` becomes a fresh Scrapy request (no reconstruction)."""
apify_request = ApifyRequest(
url='https://example.com',
url='https://example.com/',
method='GET',
unique_key='https://example.com',
user_data={},
Expand Down Expand Up @@ -86,7 +86,7 @@ def test_unique_key_is_stamped_together_with_its_fingerprint(spider: Spider) ->
def test_without_reconstruction_with_optional_fields(spider: Spider) -> None:
"""The without-reconstruction path also carries optional headers and user data to the Scrapy request."""
apify_request = ApifyRequest(
url='https://crawlee.dev',
url='https://crawlee.dev/',
method='GET',
unique_key='https://crawlee.dev',
headers=HttpHeaders({'Authorization': 'Bearer access_token'}),
Expand All @@ -109,7 +109,7 @@ def test_without_reconstruction_with_optional_fields(spider: Spider) -> None:
def test_with_reconstruction(spider: Spider) -> None:
"""An Apify request with a stored `scrapy_request` is reconstructed from its JSON payload."""
apify_request = ApifyRequest(
url='https://apify.com',
url='https://apify.com/',
method='GET',
unique_key='https://apify.com',
user_data={
Expand All @@ -129,7 +129,7 @@ def test_with_reconstruction(spider: Spider) -> None:
def test_with_reconstruction_with_optional_fields(spider: Spider) -> None:
"""Reconstruction from the stored JSON payload also restores optional headers and user data."""
apify_request = ApifyRequest(
url='https://apify.com',
url='https://apify.com/',
method='GET',
unique_key='https://apify.com',
headers=HttpHeaders({'Authorization': 'Bearer access_token'}),
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/scrapy/test_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ def test_next_request_skips_request_that_fails_to_convert(
def test_next_request_returns_converted_request(scheduler: ApifyScheduler, rq: mock.AsyncMock) -> None:
"""A valid RQ entry is reconstructed into a Scrapy request and left unhandled until Scrapy is done."""
apify_request = ApifyRequest(
url='https://example.com',
url='https://example.com/',
method='GET',
unique_key='https://example.com',
user_data={},
Expand Down
Loading
Loading