return a JSON clob as text: it was parsed as the envelope

CHANGELOG.md

@@ -15,6 +15,12 @@
 
 ### Fixed
 
+- A large object (`format` `clob` or `blob`) whose body is JSON no longer raises `AttributeError: 'dict' object has
+  no attribute 'decode'`, or a `TypeError` for a blob: the API body handler parsed it as an envelope. A large object
+  is the scraped page itself, so only its compression is undone. Large objects are first resolved in 0.8.18; 0.8.17
+  and earlier return the large-object URL string unresolved, with no warning.
+- A large object the API answers with an error status raises its `ScrapflyError`, or an `HTTPError` when the answer
+  carries no API error, instead of being returned as the page.
 - Brotli webhooks decode with Google's `brotli`, and bodies past about 480 KB are no longer cut short.
 - Every webhook encoding is bounded per decoder call. A few KB of brotli or zstd could allocate gigabytes
   before the size limit applied, and crafted deflate or gzip bodies cost seconds of CPU, all before the

scrapfly/api.py

@@ -24,7 +24,7 @@
 from .errors import *
 from .errors import ContentError
 from .reporter import Reporter
-from .api_response import ResponseBodyHandler
+from .api_response import ApiResponse, LargeObjectApiResponse, ResponseBodyHandler
 from .scrape_config import ScrapeConfig
 from .screenshot_config import ScreenshotConfig
 from .extraction_config import ExtractionConfig
@@ -1292,15 +1292,10 @@ def _large_object_steps(self, callback_url:str, format: Literal['clob', 'blob'])
             params={'key': self.key}
         )
 
-        if self.body_handler.support(headers=response.headers):
-            content = self.body_handler(content=response.content, content_type=response.headers['content-type'])
-        else:
-            content = response.content
-
-        if format == 'clob':
-            return content.decode('utf-8'), 'text'
+        large_object = LargeObjectApiResponse(request=response.request, response=response, format=format, body_handler=self.body_handler)
+        large_object.raise_for_result()
 
-        return BytesIO(content), 'binary'
+        return large_object.content, large_object.content_format
 
     def _scrape_body(self, response: Response, scrape_config: ScrapeConfig):
         if scrape_config.method == 'HEAD':
@@ -1310,26 +1305,7 @@ def _scrape_body(self, response: Response, scrape_config: ScrapeConfig):
             return self.body_handler(content=response.content, content_type=response.headers['content-type'])
 
         # body_handler rejected — content-type not in SUPPORTED_CONTENT_TYPES.
-        # Response may still be compressed (zstd/brotli) if requests did
-        # not transparently decompress. Probe content-encoding and try
-        # the handler's read() anyway before falling back to a tolerant
-        # utf-8 decode.
-        raw = response.content
-        content_encoding = response.headers.get('content-encoding', '').lower()
-        # urllib3 already inflated these: a second decode corrupts some bodies.
-        if content_encoding in ('gzip', 'gz', 'deflate', 'br', 'brotli', 'zstd') \
-                and content_encoding not in urllib3.response.HTTPResponse.CONTENT_DECODERS:
-            try:
-                raw = self.body_handler.read(
-                    content=raw,
-                    content_encoding=content_encoding,
-                    content_type=response.headers.get('content-type', ''),
-                    signature=None,
-                )
-            except Exception:
-                # Fall through to tolerant decode below; don't mask the
-                # real error with a decoder crash.
-                pass
+        raw = ApiResponse(request=response.request, response=response).decompress()
 
         if isinstance(raw, (bytes, bytearray)):
             return raw.decode('utf-8', errors='replace')

scrapfly/api_response.py

@@ -21,8 +21,9 @@
 
 from dateutil.parser import parse
 from requests import Request, Response, HTTPError
-from typing import List, Dict, Optional, Iterable, Iterator, Union, TextIO, Tuple, Callable
+from typing import List, Dict, Literal, Optional, Iterable, Iterator, Union, TextIO, Tuple, Callable
 
+import urllib3
 from requests.structures import CaseInsensitiveDict
 
 from .scrape_config import ScrapeConfig
@@ -349,6 +350,23 @@ def __init__(self, request: Request, response: Response):
         self.request = request
         self.response = response
 
+    def decompress(self) -> bytes:
+        """The body, with a Content-Encoding the HTTP client left in place undone."""
+        content = self.response.content
+        content_encoding = self.response.headers.get('content-encoding', '').lower()
+
+        # urllib3 inflated these already: a second pass corrupts some bodies.
+        if content_encoding in urllib3.response.HTTPResponse.CONTENT_DECODERS:
+            return content
+
+        try:
+            return decompress(content, content_encoding, MAX_DECOMPRESSED_SIZE)
+        except WebhookBodyTooLarge:
+            raise
+        except Exception:
+            # The client advertises only what it decodes: a body that does not decode was decoded already.
+            return content
+
     @property
     def headers(self) -> CaseInsensitiveDict:
         return self.response.headers
@@ -443,6 +461,36 @@ def raise_for_result(
                 raise error_class(**args) from e
 
 
+class LargeObjectApiResponse(ApiResponse):
+    """The clob or blob a scrape result points to: the scraped page itself, never an API envelope."""
+
+    def __init__(self, request: Request, response: Response, format: Literal['clob', 'blob'], body_handler: 'ResponseBodyHandler'):
+        super().__init__(request, response)
+        self.format = format
+        self.error = None
+        # Only an error answer is an API envelope.
+        self.result = {}
+
+        if not response.ok and body_handler.support(headers=response.headers):
+            with suppress(EncoderError):
+                self.result = body_handler(content=response.content, content_type=response.headers['content-type'])
+
+    def raise_for_result(self, raise_on_upstream_error: bool = True, error_class=ApiHttpClientError):
+        super().raise_for_result(raise_on_upstream_error=raise_on_upstream_error, error_class=error_class)
+        # An error answer without an envelope (a proxy's 502 page) is not the page either.
+        self.response.raise_for_status()
+
+    @property
+    def content(self) -> Union[str, BytesIO]:
+        content = self.decompress()
+
+        return content.decode('utf-8') if self.format == 'clob' else BytesIO(content)
+
+    @property
+    def content_format(self) -> Literal['text', 'binary']:
+        return 'text' if self.format == 'clob' else 'binary'
+
+
 class ScrapeApiResponse(ApiResponse):
     scrape_config:ScrapeConfig
     large_object_handler:Callable

tests/test_clients.py

@@ -1,7 +1,9 @@
 """ScrapflyClient and AsyncScrapflyClient run the same operations: every scenario runs on both, against one fake API."""
 import asyncio
+import gzip
 import inspect
 import json
+from io import BytesIO
 from dataclasses import dataclass
 from typing import Callable, Dict, List, Tuple
 from urllib.parse import parse_qs, urlsplit
@@ -11,8 +13,9 @@
 import requests
 
 from scrapfly import AsyncScrapflyClient, ExtractionConfig, ScrapeApiResponse, ScrapeConfig, ScrapflyClient
-from scrapfly import async_client, client
-from scrapfly.errors import CrawlerPromptError, ScrapflyError
+from scrapfly import api_response, async_client, client
+from scrapfly.api_response import ApiResponse
+from scrapfly.errors import ApiHttpClientError, ApiHttpServerError, CrawlerPromptError, ScrapflyError
 
 pytestmark = pytest.mark.unit
 
@@ -212,6 +215,128 @@ async def scenario(scrapfly):
     assert [s.path for s in seen] == ['/scrape', '/scrape/clob/01']
 
 
+@pytest.mark.parametrize('kind', CLIENTS)
+@pytest.mark.parametrize('format,content_type,body,expected', [
+    ('clob', 'application/json', b'{"products": [{"id": 1}]}', '{"products": [{"id": 1}]}'),
+    ('clob', 'text/html', b'<h1>large</h1>', '<h1>large</h1>'),
+    ('blob', 'application/pdf', b'%PDF-\x00\xff', b'%PDF-\x00\xff'),
+    ('blob', 'application/json', b'{"products": [{"id": 1}]}', b'{"products": [{"id": 1}]}'),
+])
+def test_large_object_is_returned_as_is(kind, format, content_type, body, expected):
+    def server(seen: Seen):
+        if seen.path == '/scrape':
+            return json_reply(envelope(HOST + '/scrape/large/01', format=format))
+
+        return 200, {'content-type': content_type}, [body]
+
+    async def scenario(scrapfly):
+        return await resolve(scrapfly.scrape(ScrapeConfig(url='https://web-scraping.dev/')))
+
+    response, _ = run(kind, server, scenario)
+    content = response.scrape_result['content']
+
+    # The envelope around the large object is still the API result.
+    assert response.config['url'] == 'https://web-scraping.dev/product/1'
+    assert response.scrape_result['success'] is True
+
+    if format == 'clob':
+        assert isinstance(content, str) and content == expected
+        assert response.scrape_result['format'] == 'text'
+    else:
+        assert isinstance(content, BytesIO) and content.read() == expected
+        assert response.scrape_result['format'] == 'binary'
+
+
+@pytest.mark.parametrize('kind', CLIENTS)
+@pytest.mark.parametrize('status,headers,body,error', [
+    (404, {'content-type': 'application/json'}, json.dumps({'reason': 'Not Found', 'message': 'Large Object 01 not found', 'error_id': '01', 'http_code': 404}).encode(), ApiHttpClientError),
+    (500, {'content-type': 'application/json'}, json.dumps({'reason': 'Internal Server Error', 'message': 'Internal Error', 'error_id': '01', 'http_code': 500}).encode(), ApiHttpServerError),
+    (502, {'content-type': 'text/html'}, b'<h1>Bad Gateway</h1>', requests.HTTPError),
+])
+@pytest.mark.parametrize('format', ['clob', 'blob'])
+def test_a_large_object_error_is_raised_not_returned_as_the_page(kind, status, headers, body, error, format):
+    def server(seen: Seen):
+        if seen.path == '/scrape':
+            return json_reply(envelope(HOST + '/scrape/large_object/01', format=format))
+
+        return status, headers, [body]
+
+    async def scenario(scrapfly):
+        return await resolve(scrapfly.scrape(ScrapeConfig(url='https://web-scraping.dev/')))
+
+    with pytest.raises(error) as raised:
+        run(kind, server, scenario)
+
+    assert KEY not in str(raised.value)
+
+
+@pytest.mark.parametrize('content_encoding,body,expected', [
+    ('gz', gzip.compress(b'large text'), b'large text'),
+    # The HTTP client inflated it and left the header: the body is returned as it is.
+    ('brotli', b'large text', b'large text'),
+    # A .gz file served gzip: urllib3 inflated one layer, the file is the content.
+    ('gzip', gzip.compress(b'large text'), gzip.compress(b'large text')),
+    ('', b'large text', b'large text'),
+])
+def test_an_api_response_undoes_only_the_encoding_left_in_place(content_encoding, body, expected):
+    response = requests.Response()
+    response.status_code = 200
+    response.headers['content-encoding'] = content_encoding
+    response._content = body
+
+    assert ApiResponse(request=response.request, response=response).decompress() == expected
+
+
+def test_an_api_response_past_the_decompression_limit_raises(monkeypatch):
+    monkeypatch.setattr(api_response, 'MAX_DECOMPRESSED_SIZE', 16)
+    response = requests.Response()
+    response.status_code = 200
+    response.headers['content-encoding'] = 'gz'
+    response._content = gzip.compress(b'x' * 1024)
+
+    with pytest.raises(api_response.WebhookBodyTooLarge):
+        ApiResponse(request=response.request, response=response).decompress()
+
+
+@pytest.mark.parametrize('kind', CLIENTS)
+def test_compressed_json_large_object_stays_text(kind):
+    # `gz` is gzip that urllib3 does not inflate. The SDK must, and must not parse the JSON.
+    page = b'{"products": [{"id": 1}]}'
+
+    def server(seen: Seen):
+        if seen.path == '/scrape':
+            return json_reply(envelope(HOST + '/scrape/clob/02', format='clob'))
+
+        return 200, {'content-type': 'application/json', 'content-encoding': 'gz'}, [gzip.compress(page)]
+
+    async def scenario(scrapfly):
+        return await resolve(scrapfly.scrape(ScrapeConfig(url='https://web-scraping.dev/')))
+
+    response, _ = run(kind, server, scenario)
+
+    assert response.scrape_result['content'] == page.decode()
+    assert response.scrape_result['format'] == 'text'
+    assert response.config['url'] == 'https://web-scraping.dev/product/1'
+    assert response.scrape_result['success'] is True
+
+
+@pytest.mark.parametrize('kind', CLIENTS)
+def test_a_json_scrape_envelope_is_still_parsed(kind):
+    """Sharing decompression with large objects must not skip parsing a real API envelope."""
+    def server(seen: Seen):
+        return json_reply(envelope('{"products": [{"id": 1}]}'))
+
+    async def scenario(scrapfly):
+        return await resolve(scrapfly.scrape(ScrapeConfig(url='https://web-scraping.dev/products.json')))
+
+    response, seen = run(kind, server, scenario)
+
+    assert response.scrape_result['content'] == '{"products": [{"id": 1}]}'
+    assert response.scrape_result['format'] == 'text'
+    assert response.config['url'] == 'https://web-scraping.dev/product/1'
+    assert len(seen) == 1
+
+
 @pytest.mark.parametrize('kind', CLIENTS)
 def test_a_dropped_connection_is_retried(kind, monkeypatch):
     drops = [DROP, DROP]