diff --git a/docstrange/processors/cloud_processor.py b/docstrange/processors/cloud_processor.py index 45e25d4..a0c1747 100644 --- a/docstrange/processors/cloud_processor.py +++ b/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..content (or result. 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) diff --git a/tests/test_cloud_deprecation.py b/tests/test_cloud_deprecation.py new file mode 100644 index 0000000..c556f86 --- /dev/null +++ b/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": "

html

"}, + }, + }) + self.assertEqual(content, "

html

") + + 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()