fix!: ErrorKind.PRECONDITION_FAILED renders as HTTP 400, not 412

docs/agents.md

@@ -248,7 +248,7 @@ once per run inside the application scope and appended to `spec.warmers`.
 | Name | Signature | Notes |
 |---|---|---|
 | `ServiceError` | `ServiceError(detail=None, *, code=None, kind=None, params=None, public=None)` | subclass it and set `kind` / `code` / `public` as class attributes; `code` defaults to the snake_cased class name minus `Error` |
-| `ErrorKind` | `INVALID`, `UNAUTHENTICATED`, `FORBIDDEN`, `NOT_FOUND`, `CONFLICT`, `PRECONDITION_FAILED`, `TOO_MANY_REQUESTS`, `DEADLINE_EXCEEDED`, `UNAVAILABLE`, `NOT_IMPLEMENTED`, `INTERNAL` | 400/401/403/404/409/412/429/504/503/501/500 over HTTP |
+| `ErrorKind` | `INVALID`, `UNAUTHENTICATED`, `FORBIDDEN`, `NOT_FOUND`, `CONFLICT`, `PRECONDITION_FAILED`, `TOO_MANY_REQUESTS`, `DEADLINE_EXCEEDED`, `UNAVAILABLE`, `NOT_IMPLEMENTED`, `INTERNAL` | 400/401/403/404/409/400/429/504/503/501/500 over HTTP (each the `google.rpc.Code` mapping of the gRPC status) |
 | `ErrorInfo` | `kind`, `code`, `detail=None`, `params={}`, `public=True`, `status_override=None`, `headers=None`; `.http_status`; `ErrorInfo.from_service_error(exc)` | the normalized view every renderer takes |
 | `mask_private_error` | `mask_private_error(info) -> ErrorInfo` | `public=False` collapses to a generic `internal_error` 500 |
 | `INTERNAL_ERROR_CODE` | `"internal_error"` | the code every masked error renders as, on both transports |

docs/concepts/errors.md

@@ -60,23 +60,33 @@ except ServiceError as exc:
 | `FORBIDDEN` | 403 | `PERMISSION_DENIED` |
 | `NOT_FOUND` | 404 | `NOT_FOUND` |
 | `CONFLICT` | 409 | `ALREADY_EXISTS` |
-| `PRECONDITION_FAILED` | 412 | `FAILED_PRECONDITION` |
+| `PRECONDITION_FAILED` | 400 | `FAILED_PRECONDITION` |
 | `TOO_MANY_REQUESTS` | 429 | `RESOURCE_EXHAUSTED` |
 | `NOT_IMPLEMENTED` | 501 | `UNIMPLEMENTED` |
 | `UNAVAILABLE` | 503 | `UNAVAILABLE` |
 | `DEADLINE_EXCEEDED` | 504 | `DEADLINE_EXCEEDED` |
 | `INTERNAL` | 500 | `INTERNAL` |
 
-Both columns are injective, so a status maps back to exactly one kind — which is what lets a
-gateway translate between the two without guessing.
+The gRPC column is injective, so a gRPC status maps back to exactly one kind. The HTTP column
+follows from it: every HTTP status is the one `google.rpc.Code` assigns to that gRPC status, so a
+client gets the same answer from the HTTP side as from grpc-gateway or Envoy's transcoder in front
+of the gRPC side.
 
-One row is worth stating out loud, because it has a plausible-looking alternative: `CONFLICT`
-renders as `ALREADY_EXISTS` and not as `ABORTED`, although both mean 409 to a gRPC gateway.
-`ABORTED` is the transaction-conflict code, and the gRPC contract tells clients to retry it at a
-higher level — the wrong advice for "this order was already paid", which will never succeed on a
-retry. `FAILED_PRECONDITION` is spoken for by `PRECONDITION_FAILED`. A client that has to tell a
-duplicate key from a state conflict branches on `x-error-code`, which carries the exact code
-either way, rather than on the status.
+Two rows are worth stating out loud, because each has a plausible-looking alternative.
+
+`CONFLICT` renders as `ALREADY_EXISTS` and not as `ABORTED`, although both mean 409 to a gRPC
+gateway. `ABORTED` is the transaction-conflict code, and the gRPC contract tells clients to retry
+it at a higher level — the wrong advice for "this order was already paid", which will never
+succeed on a retry. `FAILED_PRECONDITION` is spoken for by `PRECONDITION_FAILED`.
+
+`PRECONDITION_FAILED` renders as 400 and not as 412. RFC 9110 reserves 412 for conditional
+request headers (`If-Match`, `If-Unmodified-Since`) evaluating to false, and clients, proxies and
+SDKs read it as "your copy is stale: refetch and retry", the wrong advice for "this order is not
+paid yet". A conditional request your own code rejects with `HTTPException(412)` still answers 412.
+
+So 400 means either `INVALID` or `PRECONDITION_FAILED`, and 409 is either a duplicate key or a
+state conflict. A client that has to tell them apart branches on the error code (`code` in the
+problem document, `x-error-code` over gRPC), which is exact either way, rather than on the status.
 
 ## What the client sees
 
@@ -184,6 +194,25 @@ FastApiEntrypoint(error_renderer=ProblemDetailsRenderer(type_base="https://error
 # → "type": "https://errors.example.com/order_not_found"
 ```
 
+A renderer can also move one kind to another status. Clients written when `PRECONDITION_FAILED`
+answered 412 can keep getting 412 while they migrate:
+
+```python
+import dataclasses
+
+from servicewright import ErrorInfo, ErrorKind, ProblemDetailsRenderer, RenderedError
+
+
+class Precondition412Renderer(ProblemDetailsRenderer):
+    def render(self, info: ErrorInfo) -> RenderedError:
+        if info.kind is ErrorKind.PRECONDITION_FAILED and info.status_override is None:
+            info = dataclasses.replace(info, status_override=412)
+        return super().render(info)
+
+
+FastApiEntrypoint(error_renderer=Precondition412Renderer())
+```
+
 ### Rendering never fails
 
 Rendering an error is the client's last chance to learn what went wrong, so it may not be the

servicewright/core/errors.py

@@ -50,13 +50,19 @@ class ErrorKind(enum.StrEnum):
     INTERNAL = "internal"
 
 
+# Each status is the one google.rpc.Code assigns to the kind's gRPC status, so a
+# service answers the same over HTTP as grpc-gateway or Envoy's transcoder does
+# in front of its gRPC side. That makes PRECONDITION_FAILED (FAILED_PRECONDITION)
+# a 400 shared with INVALID, not 412: RFC 9110 keeps 412 for conditional request
+# headers (If-Match and friends), and clients treat it as a stale ETag. Clients
+# tell the two 400s apart by the error code, not the status.
 HTTP_STATUS_BY_KIND: dict[ErrorKind, int] = {
     ErrorKind.INVALID: 400,
     ErrorKind.UNAUTHENTICATED: 401,
     ErrorKind.FORBIDDEN: 403,
     ErrorKind.NOT_FOUND: 404,
     ErrorKind.CONFLICT: 409,
-    ErrorKind.PRECONDITION_FAILED: 412,
+    ErrorKind.PRECONDITION_FAILED: 400,
     ErrorKind.TOO_MANY_REQUESTS: 429,
     ErrorKind.DEADLINE_EXCEEDED: 504,
     ErrorKind.UNAVAILABLE: 503,

tests/unit/test_error_parity.py

@@ -38,6 +38,22 @@
 
 SECRET = "dsn=postgres://user:pw@db/ledger"
 
+# google.rpc.Code's HTTP mapping (google/rpc/code.proto): what grpc-gateway or
+# Envoy's transcoder answers in front of a gRPC service.
+HTTP_STATUS_BY_GRPC_STATUS = {
+    grpc.StatusCode.INVALID_ARGUMENT: 400,
+    grpc.StatusCode.UNAUTHENTICATED: 401,
+    grpc.StatusCode.PERMISSION_DENIED: 403,
+    grpc.StatusCode.NOT_FOUND: 404,
+    grpc.StatusCode.ALREADY_EXISTS: 409,
+    grpc.StatusCode.FAILED_PRECONDITION: 400,
+    grpc.StatusCode.RESOURCE_EXHAUSTED: 429,
+    grpc.StatusCode.DEADLINE_EXCEEDED: 504,
+    grpc.StatusCode.UNAVAILABLE: 503,
+    grpc.StatusCode.UNIMPLEMENTED: 501,
+    grpc.StatusCode.INTERNAL: 500,
+}
+
 
 class OrderNotFoundError(ServiceError):
     kind = ErrorKind.NOT_FOUND
@@ -64,7 +80,7 @@ class Conclusion:
 
 
 def _kind_of(status: Any, table: Mapping[ErrorKind, Any]) -> ErrorKind:
-    """Reverse the transport's own mapping table; both are injective."""
+    """Reverse the transport's own mapping table (HTTP 400 reverses to ``INVALID``)."""
     return next(kind for kind, mapped in table.items() if mapped == status)
 
 
@@ -183,3 +199,8 @@ async def test__public_service_error__raised_over_either_transport__reaches_the_
     assert conclusion.kind is ErrorKind.NOT_FOUND
     assert conclusion.code == "order_not_found"
     assert conclusion.detail == "no order with id 42"
+
+
+@pytest.mark.parametrize("kind", list(ErrorKind))
+def test__error_kind__http_status__is_what_a_grpc_gateway_answers_for_its_grpc_status(kind: ErrorKind) -> None:
+    assert HTTP_STATUS_BY_KIND[kind] == HTTP_STATUS_BY_GRPC_STATUS[GRPC_STATUS_BY_KIND[kind]]

tests/unit/test_fastapi.py

@@ -505,6 +505,14 @@ async def http404() -> None:
     async def http500() -> None:
         raise HTTPException(status_code=500, detail="boom")
 
+    @router.put("/http-412")
+    async def http412() -> None:
+        raise HTTPException(status_code=412, detail="stale ETag")
+
+    @router.post("/precondition")
+    async def precondition() -> None:
+        raise ServiceError("order 42 is not paid yet", code="order_not_paid", kind=ErrorKind.PRECONDITION_FAILED)
+
     @router.get("/unhandled")
     async def unhandled() -> None:
         raise RuntimeError("kaboom")
@@ -553,6 +561,20 @@ async def test__http_exception_handler__4xx__preserves_the_detail() -> None:
     assert body["detail"] == "not found"
 
 
+async def test__http_exception_handler__412_from_a_conditional_request__keeps_412() -> None:
+    resp = (await _exc_client()).put("/http-412")
+    assert resp.status_code == 412
+    assert resp.json()["detail"] == "stale ETag"
+
+
+async def test__service_error_handler__precondition_failed__answers_400_not_412() -> None:
+    resp = (await _exc_client()).post("/precondition")
+    assert resp.status_code == 400
+    body = resp.json()
+    assert body["status"] == 400
+    assert body["code"] == "order_not_paid"
+
+
 async def test__http_exception_handler__5xx__masks_it_to_a_generic_internal_error() -> None:
     resp = (await _exc_client()).get("/http-500")
     assert resp.status_code == 500