fix(cloud): migrate off deprecated /extract endpoint and stop misreading deprecation 429 as a quota error

docstrange/processors/cloud_processor.py

@@ -53,10 +53,25 @@ def _get_cloud_output(self, output_type: str, specified_fields: Optional[list] =
                     'file': (os.path.basename(self.file_path), file, self.cloud_processor._get_content_type(self.file_path))
                 }
 
+                # The /api/v1/extract/sync endpoint expects `output_format`
+                # (markdown|html|json|csv). Map from the legacy `output_type`
+                # vocabulary; `flat-json` and the specified-* variants map to
+                # the `json` output format on the new API. `output_type` is kept
+                # for backward compatibility with any older endpoint.
+                _fmt_map = {
+                    "markdown": "markdown",
+                    "html": "html",
+                    "csv": "csv",
+                    "flat-json": "json",
+                    "specified-fields": "json",
+                    "specified-json": "json",
+                }
+                output_format = _fmt_map.get(output_type, "markdown")
                 data = {
-                    'output_type': output_type
+                    'output_type': output_type,
+                    'output_format': output_format,
                 }
-                
+
                 # Add model_type if specified
                 if self.cloud_processor.model_type:
                     data['model_type'] = self.cloud_processor.model_type
@@ -82,8 +97,24 @@ def _get_cloud_output(self, output_type: str, specified_fields: Optional[list] =
                     timeout=300
                 )
 
-                # Handle rate limiting (429) specifically
+                # Handle HTTP 429 responses. A 429 does NOT always mean the
+                # rate limit was hit: a deprecated endpoint returns 429 with a
+                # body like {"detail": "The /extract endpoint is deprecated.
+                # Please migrate to /api/v1/extract."}. Inspect the body before
+                # assuming a quota problem so we never surface a misleading
+                # "Rate limit exceeded" error when the real cause is different.
                 if response.status_code == 429:
+                    detail = self.cloud_processor._parse_error_detail(response)
+
+                    if detail and self.cloud_processor._is_deprecation_notice(detail):
+                        error_msg = (
+                            f"The cloud extraction endpoint returned a deprecation "
+                            f"notice: {detail}. Please upgrade docstrange to a version "
+                            f"that targets the current API endpoint."
+                        )
+                        logger.error(error_msg)
+                        raise ConversionError(error_msg)
+
                     if not self.cloud_processor.api_key:
                         error_msg = (
                             "Rate limit exceeded for free tier (limited calls daily). "
@@ -93,13 +124,17 @@ def _get_cloud_output(self, output_type: str, specified_fields: Optional[list] =
                             "  - Python: DocumentExtractor()  # after login (uses cached credentials)\n"
                             "  - Python: DocumentExtractor(api_key='YOUR_API_KEY')  # alternative"
                         )
+                        if detail:
+                            error_msg += f"\nServer said: {detail}"
                         logger.error(error_msg)
                         raise ConversionError(error_msg)
                     else:
                         error_msg = "Rate limit exceeded (10k/month). Please try again later."
+                        if detail:
+                            error_msg += f" Server said: {detail}"
                         logger.error(error_msg)
                         raise ConversionError(error_msg)
-                
+
                 response.raise_for_status()
                 result_data = response.json()
 
@@ -230,8 +265,12 @@ def __init__(self, api_key: Optional[str] = None, output_type: str = None, model
         self.model_type = model_type
         self.specified_fields = specified_fields
         self.json_schema = json_schema
-        self.api_url = "https://extraction-api.nanonets.com/extract"
-
+        # Nanonets deprecated the bare /extract endpoint: it now returns HTTP
+        # 429 with body {"detail": "The /extract endpoint is deprecated. Please
+        # migrate to /api/v1/extract."}. Target the current sync endpoint so a
+        # deprecation notice is not misread as a rate-limit/quota error.
+        self.api_url = "https://extraction-api.nanonets.com/api/v1/extract/sync"
+
         # Don't validate output_type during initialization - it will be validated during processing
         # This prevents warnings during DocumentExtractor initialization
 
@@ -285,17 +324,69 @@ def process(self, file_path: str) -> CloudConversionResult:
             metadata=metadata
         )
 
+    @staticmethod
+    def _parse_error_detail(response: requests.Response) -> str:
+        """Best-effort extraction of a human-readable error message from a response.
+
+        Nanonets error bodies use a JSON ``{"detail": "..."}`` shape; fall back
+        to the raw (truncated) text for anything else.
+        """
+        try:
+            body = response.json()
+        except ValueError:
+            return (response.text or "").strip()[:500]
+        if isinstance(body, dict):
+            for key in ("detail", "message", "error"):
+                value = body.get(key)
+                if isinstance(value, str) and value:
+                    return value
+        return json.dumps(body)[:500]
+
+    @staticmethod
+    def _is_deprecation_notice(detail: str) -> bool:
+        """Return True when an error message describes a deprecated endpoint."""
+        lowered = detail.lower()
+        return "deprecated" in lowered or "migrate to" in lowered
+
     def _extract_content_from_response(self, response_data: Dict[str, Any]) -> str:
-        """Extract content from API response."""
+        """Extract content from API response.
+
+        Supports both the legacy flat shape (top-level ``content``) and the
+        current /api/v1/extract/sync nested shape::
+
+            {"success": true,
+             "result": {"markdown": {"content": "..."}, "html": ..., ...},
+             "output_format": "markdown"}
+        """
         try:
-            # API always returns content in the 'content' field
+            # Legacy shape: top-level 'content' field.
             if 'content' in response_data:
                 return response_data['content']
-
+
+            # Current /api/v1/extract/sync shape: content lives under
+            # result.<format>.content (or result.<format> when it's a string).
+            result = response_data.get('result')
+            if isinstance(result, dict):
+                requested_format = response_data.get('output_format')
+                candidates = []
+                # Prefer the block matching the requested output_format.
+                if requested_format and requested_format in result:
+                    candidates.append(result[requested_format])
+                candidates.extend(
+                    result[key]
+                    for key in ("markdown", "html", "csv", "json")
+                    if key in result and result[key] is not None
+                )
+                for block in candidates:
+                    if isinstance(block, dict) and block.get('content') is not None:
+                        return block['content']
+                    if isinstance(block, str):
+                        return block
+
             # Fallback: return whole response as JSON if no content field
             logger.warning("No 'content' field found in API response, returning full response")
             return json.dumps(response_data, indent=2)
-            
+
         except Exception as e:
             logger.error(f"Failed to extract content from API response: {e}")
             return json.dumps(response_data, indent=2)

tests/test_cloud_deprecation.py

@@ -0,0 +1,176 @@
+#!/usr/bin/env python3
+"""Unit tests for CloudProcessor endpoint migration + 429 body-aware handling.
+
+These tests are fully offline: the HTTP layer and file I/O are mocked, so no
+API key, network access, or sample documents are required.
+
+Regression coverage for the bug where docstrange POSTed to nanonets' deprecated
+`/extract` endpoint (which now returns HTTP 429 with a *deprecation* body) and
+the 429 handler blindly reported "Rate limit exceeded (10k/month)" for any key
+holder — masking the real cause and falsely implying an exhausted quota.
+"""
+
+import io
+import json
+import unittest
+from unittest import mock
+
+from docstrange.exceptions import ConversionError
+from docstrange.processors.cloud_processor import CloudConversionResult, CloudProcessor
+
+
+class _FakeResponse:
+    """Minimal stand-in for requests.Response."""
+
+    def __init__(self, status_code=200, json_body=None, text=""):
+        self.status_code = status_code
+        self._json_body = json_body
+        self.text = text
+
+    def json(self):
+        if self._json_body is None:
+            raise ValueError("no json body")
+        return self._json_body
+
+    def raise_for_status(self):
+        if self.status_code >= 400:
+            raise AssertionError(
+                f"raise_for_status() called for {self.status_code}"
+            )
+
+
+def _make_result(api_key=None):
+    processor = CloudProcessor(api_key=api_key)
+    result = CloudConversionResult(file_path="doc.pdf", cloud_processor=processor)
+    return processor, result
+
+
+class CloudProcessorEndpointTests(unittest.TestCase):
+    def test_uses_current_sync_endpoint(self):
+        processor = CloudProcessor()
+        self.assertEqual(
+            processor.api_url,
+            "https://extraction-api.nanonets.com/api/v1/extract/sync",
+        )
+        self.assertNotIn("/extract\"", processor.api_url)
+
+    def test_request_sends_output_format(self):
+        processor, result = _make_result(api_key="k")
+        captured = {}
+
+        def fake_post(url, headers=None, files=None, data=None, timeout=None):
+            captured["url"] = url
+            captured["data"] = data
+            return _FakeResponse(
+                status_code=200,
+                json_body={
+                    "success": True,
+                    "output_format": "markdown",
+                    "result": {"markdown": {"content": "hello world"}},
+                },
+            )
+
+        with mock.patch("builtins.open", mock.mock_open(read_data=b"x")), \
+                mock.patch("docstrange.processors.cloud_processor.requests.post", fake_post):
+            out = result.extract_markdown()
+
+        self.assertEqual(out, "hello world")
+        self.assertEqual(captured["url"], processor.api_url)
+        # Legacy field kept for backward-compat; new field added.
+        self.assertEqual(captured["data"]["output_type"], "markdown")
+        self.assertEqual(captured["data"]["output_format"], "markdown")
+
+
+class DeprecationAwareRateLimitTests(unittest.TestCase):
+    def test_deprecation_429_is_not_reported_as_quota(self):
+        _, result = _make_result(api_key="k")
+        deprecation = _FakeResponse(
+            status_code=429,
+            json_body={
+                "detail": "The /extract endpoint is deprecated. "
+                          "Please migrate to /api/v1/extract."
+            },
+        )
+
+        with mock.patch("builtins.open", mock.mock_open(read_data=b"x")), \
+                mock.patch("docstrange.processors.cloud_processor.requests.post",
+                           return_value=deprecation):
+            with self.assertRaises(ConversionError) as ctx:
+                result.extract_markdown()
+
+        message = str(ctx.exception)
+        self.assertIn("deprecat", message.lower())
+        self.assertNotIn("10k/month", message)
+        self.assertNotIn("Rate limit exceeded", message)
+
+    def test_real_quota_429_still_reported_with_key(self):
+        _, result = _make_result(api_key="k")
+        quota = _FakeResponse(
+            status_code=429,
+            json_body={"detail": "Monthly quota exceeded."},
+        )
+
+        with mock.patch("builtins.open", mock.mock_open(read_data=b"x")), \
+                mock.patch("docstrange.processors.cloud_processor.requests.post",
+                           return_value=quota):
+            with self.assertRaises(ConversionError) as ctx:
+                result.extract_markdown()
+
+        message = str(ctx.exception)
+        self.assertIn("Rate limit exceeded", message)
+        self.assertIn("Monthly quota exceeded", message)
+
+    def test_free_tier_429_without_key_reports_login_guidance(self):
+        _, result = _make_result(api_key=None)
+        quota = _FakeResponse(
+            status_code=429,
+            json_body={"detail": "Too many requests."},
+        )
+
+        with mock.patch("builtins.open", mock.mock_open(read_data=b"x")), \
+                mock.patch("docstrange.processors.cloud_processor.requests.post",
+                           return_value=quota):
+            with self.assertRaises(ConversionError) as ctx:
+                result.extract_markdown()
+
+        self.assertIn("free tier", str(ctx.exception).lower())
+
+
+class ResponseParsingTests(unittest.TestCase):
+    def test_parses_new_nested_shape(self):
+        processor = CloudProcessor()
+        content = processor._extract_content_from_response({
+            "success": True,
+            "output_format": "markdown",
+            "result": {"markdown": {"content": "# Title"}},
+        })
+        self.assertEqual(content, "# Title")
+
+    def test_backward_compatible_flat_shape(self):
+        processor = CloudProcessor()
+        content = processor._extract_content_from_response({"content": "legacy"})
+        self.assertEqual(content, "legacy")
+
+    def test_prefers_requested_output_format_block(self):
+        processor = CloudProcessor()
+        content = processor._extract_content_from_response({
+            "output_format": "html",
+            "result": {
+                "markdown": {"content": "md"},
+                "html": {"content": "<p>html</p>"},
+            },
+        })
+        self.assertEqual(content, "<p>html</p>")
+
+    def test_parse_error_detail_handles_non_json(self):
+        resp = _FakeResponse(status_code=500, json_body=None, text="Internal Error")
+        self.assertEqual(CloudProcessor._parse_error_detail(resp), "Internal Error")
+
+    def test_is_deprecation_notice(self):
+        self.assertTrue(CloudProcessor._is_deprecation_notice(
+            "The /extract endpoint is deprecated. Please migrate to /api/v1/extract."))
+        self.assertFalse(CloudProcessor._is_deprecation_notice("Monthly quota exceeded."))
+
+
+if __name__ == "__main__":
+    unittest.main()