fix(etl-uvicorn): declare a rate limit retryable and normalize plugin_error.error_reason

CHANGELOG.md

@@ -1,3 +1,25 @@
+## 0.1.1
+
+* **A provider rate limit no longer reaches the wire as terminal.** `plugin_error_of` declared
+  `retryable = false` for the whole legacy `UserError` family, `RateLimitError` included, so a 429
+  a consumer reads off the declared field fails the record on the first throttle instead of backing
+  off, and charges it to the customer. `RateLimitError` now serializes
+  `error_type = "dependency"`, `audience = "user"`, `retryable = true` and an `error_reason` that
+  defaults to `rate_limited`, mirroring the `utic_plugin_base` class of the same name. An explicit
+  `failure_category` still overrides that default reason, as it does for the rest of the family.
+  Every other member of
+  the family stays terminal: terminal is the safe default, so a transient condition declares itself.
+  The `plugins_controller` reads retryability off the HTTP status band today, so its behaviour is
+  unchanged either way.
+* **`plugin_error.error_reason` is lower_snake_case again.** It was assigned straight from
+  `failure_category`, which is a separate SCREAMING_SNAKE vocabulary (`AUTH_PERMISSION_DENIED`), so
+  a plugin that set one put the wrong spelling on a field specified lower_snake_case. The category
+  is now normalized for this field only and still rides the top-level `failure_category` verbatim.
+  Compatibility: the verbatim spelling reached the wire only in 0.1.0, so a consumer written
+  against that release reads `error_reason` change spelling across this patch bump. Nothing is
+  lost, since the top-level `failure_category` is unchanged, but read that field rather than
+  parsing `error_reason` if you need the category verbatim.
+
 ## 0.1.0
 
 * **This package now owns the `/invoke` transport for the reserved fields.**

test/api/test_api.py

@@ -12,6 +12,9 @@
     FileData,
     SourceIdentifiers,
 )
+from unstructured_ingest.error import RateLimitError as IngestRateLimitError
+from unstructured_ingest.error import UserAuthError as IngestUserAuthError
+from unstructured_ingest.error import UserError as IngestUserError
 
 from unstructured_platform_plugins.etl_uvicorn.api_generator import (
     EtlApiException,
@@ -916,3 +919,132 @@ def _string_annotated_precheck(usage: "list") -> "None":
     )
 
     assert client.get("/precheck").json()["status_code"] == 200
+
+
+class _CategorizedUserError(IngestUserError):
+    """A user error that also carries a preflight failure_category, as partitioner's do."""
+
+    failure_category = "AUTH_PERMISSION_DENIED"
+
+
+def test_rate_limit_error_reaches_the_wire_as_retryable():
+    """A provider 429 is transient: terminalizing it fails a record that backing off recovers.
+
+    Mirrors the utic_plugin_base RateLimitError declaration (platform-libs #889), which
+    carries error_type="dependency", error_reason="rate_limited" and retryable=True.
+    """
+
+    def _rate_limited() -> None:
+        raise IngestRateLimitError("provider throttled the request")
+
+    client = TestClient(wrap_in_fastapi(func=_rate_limited, plugin_id="mock_plugin"))
+
+    body = client.post("/invoke").json()
+
+    assert body["status_code"] == 429
+    plugin_error = body["plugin_error"]
+    assert plugin_error["audience"] == "user"
+    assert plugin_error["retryable"] is True
+    assert plugin_error["error_type"] == "dependency"
+    assert plugin_error["error_reason"] == "rate_limited"
+
+
+def test_non_rate_limited_user_errors_stay_terminal():
+    """Terminal is the safe default: only the transient condition declares itself."""
+
+    def _bad_credentials() -> None:
+        raise IngestUserAuthError("credential rejected")
+
+    client = TestClient(wrap_in_fastapi(func=_bad_credentials, plugin_id="mock_plugin"))
+
+    plugin_error = client.post("/invoke").json()["plugin_error"]
+
+    assert plugin_error["retryable"] is False
+    assert plugin_error["error_type"] == "configuration"
+    assert plugin_error["error_reason"] == "invalid_input"
+
+
+def test_rate_limit_failure_category_overrides_the_default_reason():
+    """`rate_limited` is the class DEFAULT reason, not a fixed one.
+
+    A plugin that declares a `failure_category` on a rate limit has said something more
+    specific than the class did, so the category wins the reason -- the same precedence every
+    other member of the family gets. The retryability and typing still come from the class.
+    """
+
+    class _CategorizedRateLimitError(IngestRateLimitError):
+        failure_category = "PROVIDER_RATE_LIMITED"
+
+    def _rate_limited() -> None:
+        raise _CategorizedRateLimitError("provider throttled the request")
+
+    client = TestClient(wrap_in_fastapi(func=_rate_limited, plugin_id="mock_plugin"))
+
+    plugin_error = client.post("/invoke").json()["plugin_error"]
+
+    assert plugin_error["error_reason"] == "provider_rate_limited"
+    assert plugin_error["error_type"] == "dependency"
+    assert plugin_error["retryable"] is True
+
+
+def test_precheck_rate_limit_error_is_retryable():
+    """The precheck envelope carries the same retryability as invoke."""
+
+    def _rate_limited_precheck() -> None:
+        raise IngestRateLimitError("provider throttled the precheck")
+
+    client = TestClient(
+        wrap_in_fastapi(
+            func=_no_params, plugin_id="mock_plugin", precheck_func=_rate_limited_precheck
+        )
+    )
+
+    plugin_error = client.get("/precheck").json()["plugin_error"]
+
+    assert plugin_error["retryable"] is True
+    assert plugin_error["error_reason"] == "rate_limited"
+
+
+def test_error_reason_is_lower_snake_case_on_the_wire():
+    """`error.reason` is lower_snake_case. `failure_category` is a separate
+    SCREAMING_SNAKE vocabulary and keeps its own top-level field."""
+
+    def _categorized() -> None:
+        raise _CategorizedUserError("credential rejected")
+
+    client = TestClient(wrap_in_fastapi(func=_categorized, plugin_id="mock_plugin"))
+
+    body = client.post("/invoke").json()
+
+    assert body["failure_category"] == "AUTH_PERMISSION_DENIED"
+    assert body["plugin_error"]["error_reason"] == "auth_permission_denied"
+
+
+def test_error_reason_normalizes_a_non_snake_failure_category():
+    """failure_category is arbitrary plugin-supplied text, so lowercasing alone does not
+    produce a lower_snake_case token."""
+
+    class _SpacedCategoryError(IngestUserError):
+        failure_category = "Auth Permission-Denied "
+
+    def _spaced() -> None:
+        raise _SpacedCategoryError("credential rejected")
+
+    client = TestClient(wrap_in_fastapi(func=_spaced, plugin_id="mock_plugin"))
+
+    body = client.post("/invoke").json()
+
+    assert body["failure_category"] == "Auth Permission-Denied "
+    assert body["plugin_error"]["error_reason"] == "auth_permission_denied"
+
+
+def test_unusable_failure_category_falls_back_to_the_default_reason():
+    class _PunctuationCategoryError(IngestUserError):
+        failure_category = "///"
+
+    def _punctuation() -> None:
+        raise _PunctuationCategoryError("credential rejected")
+
+    client = TestClient(wrap_in_fastapi(func=_punctuation, plugin_id="mock_plugin"))
+
+    assert client.post("/invoke").json()["plugin_error"]["error_reason"] == "invalid_input"

unstructured_platform_plugins/__version__.py

@@ -1 +1 @@
-__version__ = "0.1.0"  # pragma: no cover
+__version__ = "0.1.1"  # pragma: no cover

unstructured_platform_plugins/etl_uvicorn/api_generator.py

@@ -3,6 +3,7 @@
 import inspect
 import json
 import logging
+import re
 from typing import Any, Callable, Optional, Union, get_origin
 
 from fastapi import FastAPI, HTTPException, status
@@ -12,7 +13,7 @@
 from starlette.responses import RedirectResponse
 from typing_extensions import deprecated
 from unstructured_ingest.data_types.file_data import BatchFileData, FileData, file_data_from_dict
-from unstructured_ingest.error import UnstructuredIngestError, UserError
+from unstructured_ingest.error import RateLimitError, UnstructuredIngestError, UserError
 from uvicorn.config import LOG_LEVELS
 from uvicorn.importer import import_from_string
 
@@ -111,19 +112,52 @@ def failure_category_of(error: BaseException) -> Optional[str]:
     return category if isinstance(category, str) else None
 
 
+def _as_error_reason(category: Optional[str]) -> Optional[str]:
+    """Render a failure_category as an ``error.reason`` token, or None when it cannot be.
+
+    ``failure_category`` and ``error.reason`` are two vocabularies, not one: the preflight
+    categories are SCREAMING_SNAKE (``AUTH_PERMISSION_DENIED``) while ``error.reason`` is
+    specified lower_snake_case. ``failure_category`` still rides its own top-level response
+    field verbatim, so nothing is lost by normalizing the copy that lands here.
+
+    The category is plugin-supplied free text rather than a closed enum, so lowercasing
+    alone would not produce a snake_case token; punctuation and whitespace collapse to
+    single underscores and a category with no usable characters yields None.
+    """
+    if category is None:
+        return None
+    reason = re.sub(r"[^a-z0-9]+", "_", category.lower()).strip("_")
+    return reason or None
+
+
 def plugin_error_of(error: BaseException) -> Optional[PluginErrorMetadata]:
     """Map the legacy UserError family onto the canonical plugin-error envelope.
 
     The ratified ErrorAudience vocabulary owns the wire spelling. A non-user failure remains
     unclassified: orchestrators must not infer actionability from its HTTP status.
+
+    Terminal is the safe default, so a transient condition has to declare itself: a provider
+    rate limit is the one member of this family that clears on its own, and it mirrors the
+    utic_plugin_base ``RateLimitError`` declaration (platform-libs #889) field for field.
+    Retryability is orthogonal to audience -- a throttled request is still the caller's
+    quota, so the audience stays ``user``.
+
+    The class picks the DEFAULT reason; an explicit ``failure_category`` still wins, on this
+    branch as on every other. A plugin that declares a category has said something more
+    specific than the class did (a rate limit raised as ``PROVIDER_RATE_LIMITED``), and
+    dropping it here would discard the more precise of the two.
     """
     if not isinstance(error, UserError):
         return None
+    if isinstance(error, RateLimitError):
+        error_type, default_reason, retryable = "dependency", "rate_limited", True
+    else:
+        error_type, default_reason, retryable = "configuration", "invalid_input", False
     return PluginErrorMetadata(
-        error_type="configuration",
-        error_reason=failure_category_of(error) or "invalid_input",
+        error_type=error_type,
+        error_reason=_as_error_reason(failure_category_of(error)) or default_reason,
         audience=ErrorAudience.USER,
-        retryable=False,
+        retryable=retryable,
     )