fix(rest): render batch entry failures through the single-resource error mapping

crates/rest/Cargo.toml

@@ -133,6 +133,12 @@ helios-observability = { path = "../observability" }
 # Key generation for the JWE round-trip tests
 rand = "0.8"
 
+# `SearchProvider::search_param_registry` returns a `parking_lot::RwLock`, and
+# the batch unit tests' mock storage has to satisfy that trait since #478
+# widened `process_batch`'s bound. Test-only: the crate's own code reaches the
+# registry through `helios-persistence` and never names the lock type.
+parking_lot = "0.12"
+
 # Temp files
 tempfile = "3"
 

crates/rest/README.md

@@ -508,6 +508,39 @@ version — so a lowercase `"post"` is invalid instance data and is refused with
 `400`, not silently accepted. `GET`, `POST`, `PUT` and `DELETE` dispatch; `PATCH`
 and `HEAD` are refused as described under Current Limitations.
 
+### Per-entry outcomes
+
+A failed entry's `response.outcome` carries the **same issue code the equivalent
+single-resource request would return**, because both are rendered by one mapping
+(`RestError::client_outcome`). A batch `GET Patient/missing` and
+`GET [base]/Patient/missing` produce byte-identical OperationOutcomes.
+
+| Entry failure | Status | `issue.code` |
+|---|---|---|
+| `request` absent | 400 | `required` |
+| `request.method` absent | 400 | `required` |
+| `request.method` not an `http-verb` code | 400 | `value` |
+| `request.url` absent | 400 | `required` |
+| `request.url` names nothing | 400 | `value` |
+| `resource` absent on `POST`/`PUT` | 400 | `invalid` |
+| `PUT`/`DELETE` URL names no instance | 400 | `value` |
+| conditional criteria in the URL | 400 | `not-supported` |
+| insufficient scope | 403 | `forbidden` |
+| target not found | 404 | `not-found` |
+| `HEAD` | 405 | `not-supported` |
+| `ifMatch` precondition failed | 412 | `conflict` |
+| write validation failed | 422 | the validator's own issues |
+| `PATCH` | 501 | `not-supported` |
+| storage error | per class | `deleted`, `conflict`, `multiple-matches`, `transient`, `timeout`, `exception`, … |
+
+An entry that fails enforce-mode write validation carries the validator's **full
+multi-issue outcome** — per-issue `code`, `severity` and `expression` (the
+FHIRPath location of the failing element) — exactly as `POST [base]/[type]` does.
+
+`required` and `value` are both children of `invalid` in the `issue-type`
+hierarchy, and `invalid` is used where the distinction between an absent element
+and an unusable value does not apply.
+
 ### Conditional Operations in Bundles
 
 - `ifMatch` — **supported.** ETag for optimistic locking on `PUT` **and `DELETE`**
@@ -522,7 +555,8 @@ and `HEAD` are refused as described under Current Limitations.
 Conditional interactions expressed in the entry URL (`PUT [type]?[criteria]`,
 `DELETE [type]?[criteria]`) are **not resolved**:
 
-- In a `batch`, such an entry is refused per-entry with `400`; nothing is written.
+- In a `batch`, such an entry is refused per-entry with `400 not-supported`;
+  nothing is written. Both arms agree on the status **and** the issue code.
 - In a `transaction`, any non-`GET` entry whose URL carries a query string
   declines the whole bundle with `400 not-supported` before anything executes,
   because the backends parse entry URLs query-blind and would otherwise commit
@@ -537,10 +571,12 @@ endpoints and **not** for bundle entries; reconciling the two is #511.
 The following FHIR transaction features are not yet implemented:
 - **Conditional interactions in bundle entries** - `[type]?[criteria]` URLs are refused rather than resolved (#511)
 - **Conditional reference resolution** - References like `Patient?identifier=12345` are not resolved
-- **PATCH method** - PATCH operations in bundles return 501 Not Implemented, in both `batch` (per entry) and `transaction` (whole bundle). Send the patch to the instance endpoint instead
-- **HEAD entries** - refused with 405. `HEAD` is a legal `http-verb` code and is served on the instance-read route, but not inside a Bundle
+- **PATCH method** - PATCH operations in bundles return `501 not-supported`, in both `batch` (per entry) and `transaction` (whole bundle). Send the patch to the instance endpoint instead
+- **HEAD entries** - refused with `405 not-supported`. `HEAD` is a legal `http-verb` code and is served on the instance-read route, but not inside a Bundle
 - **Prefer header** - `return=minimal` and `return=OperationOutcome` not honored
 - **Duplicate detection** - Same resource appearing twice in a transaction is not detected
+- **Transaction entry failures after dispatch** - a transaction entry that fails once the backend is executing it is still collapsed to `400 processing`, with the real status stringified into the message (`Entry failed with status 404`). The backends discard the entry result at their `status >= 400` guard and return `TransactionError::BundleError`, which carries neither. Tracked separately; the per-entry codes above are the `batch` arm and the transaction refusals raised *before* dispatch
+- **Bare type-level `GET`** - `GET Patient` in a batch entry is read as an instance read with an empty id and answers `404 not-found` with the message `Resource Patient/ not found`. Executing it as a search is #478
 
 ## HTTP Headers
 
@@ -574,6 +610,11 @@ All errors are returned as FHIR OperationOutcome resources:
 }
 ```
 
+The same shape is used for a failed Bundle entry, placed at
+`Bundle.entry.response.outcome` — see [Per-entry outcomes](#per-entry-outcomes).
+Both are built by `RestError::client_outcome`, so an error is described
+identically wherever it surfaces.
+
 ## Testing
 
 Tests use a JSON-driven specification format:

crates/rest/src/error.rs

@@ -39,6 +39,15 @@
 //! spec-defined parameters/features that the server explicitly refuses;
 //! [`RestError::NotImplemented`] (501 + `not-supported`) signals work that
 //! has not yet been wired up.
+//!
+//! Three variants share `400` and differ only in how precisely they classify
+//! the fault, using the `issue-type` hierarchy rather than three shades of the
+//! same code: [`RestError::MissingElement`] (`required`) for an absent
+//! mandatory element, [`RestError::InvalidElementValue`] (`value`) for one
+//! that is present but unusable, and [`RestError::BadRequest`] (`invalid`,
+//! their common parent) for everything else. Prefer a child where the
+//! distinction is real — `OperationOutcome.issue.code` is bound `required` to
+//! `issue-type` and its ElementDefinition asks for the most applicable code.
 
 use axum::{
     Json,
@@ -120,6 +129,45 @@ pub enum RestError {
         message: String,
     },
 
+    /// A mandatory element is absent (HTTP 400 + `required`).
+    ///
+    /// Split from [`RestError::BadRequest`] so the outcome can carry
+    /// `required` — "A required element is missing." — rather than its is-a
+    /// parent `invalid`. Nothing in this crate could say `required` before:
+    /// every handler reported an absent element as `BadRequest`, and the
+    /// Bundle arms were the first place the distinction mattered enough to
+    /// notice (#504).
+    ///
+    /// Use it only where the StructureDefinition gives `min=1` or a named
+    /// invariant makes the element mandatory — `Bundle.entry.request`
+    /// (`bdl-3` in R4/R4B, transitively `bdl-3c` in R5/R6),
+    /// `Bundle.entry.request.method` (1..1) and `Bundle.entry.request.url`
+    /// (1..1). It is deliberately **not** used for an absent
+    /// `Bundle.entry.resource`: that element is 0..1 and only R5/R6's
+    /// `bdl-3c` requires it for POST/PUT/PATCH, so claiming `required` there
+    /// would assert a rule R4 and R4B do not have.
+    MissingElement {
+        /// Message naming the absent element and the entry it belongs to.
+        message: String,
+    },
+
+    /// An element is present but its value cannot be used (HTTP 400 + `value`).
+    ///
+    /// Split from [`RestError::BadRequest`] for the same reason as
+    /// [`RestError::MissingElement`], one level down the other branch:
+    /// `value` — "An element or header value is invalid." — is a child of
+    /// `invalid`, and `OperationOutcome.issue.code`'s ElementDefinition
+    /// requires the most applicable code rather than an ancestor that happens
+    /// to be true.
+    ///
+    /// The distinction against `MissingElement` is absent-versus-unusable, and
+    /// it is the one a client acts on differently: a missing element is added,
+    /// an invalid value is corrected.
+    InvalidElementValue {
+        /// Message naming the element and why its value cannot be used.
+        message: String,
+    },
+
     /// Unsupported media type (HTTP 415).
     UnsupportedMediaType {
         /// The unsupported content type.
@@ -284,6 +332,12 @@ impl fmt::Display for RestError {
             RestError::BadRequest { message } => {
                 write!(f, "Bad request: {}", message)
             }
+            RestError::MissingElement { message } => {
+                write!(f, "Missing element: {}", message)
+            }
+            RestError::InvalidElementValue { message } => {
+                write!(f, "Invalid element value: {}", message)
+            }
             RestError::UnsupportedMediaType { content_type } => {
                 write!(f, "Unsupported media type: {}", content_type)
             }
@@ -393,6 +447,16 @@ impl RestError {
             RestError::BadRequest { message } => {
                 (StatusCode::BAD_REQUEST, "invalid", message.clone())
             }
+            // `required` and `value` are both children of `invalid` in the
+            // `issue-type` hierarchy (verified identical in R4/R4B/R5/R6 at
+            // `crates/fhir-gen/resources/*/valuesets.json`), so these two
+            // refine `BadRequest` rather than contradicting it.
+            RestError::MissingElement { message } => {
+                (StatusCode::BAD_REQUEST, "required", message.clone())
+            }
+            RestError::InvalidElementValue { message } => {
+                (StatusCode::BAD_REQUEST, "value", message.clone())
+            }
             RestError::UnsupportedMediaType { content_type } => (
                 StatusCode::UNSUPPORTED_MEDIA_TYPE,
                 "not-supported",
@@ -406,6 +470,11 @@ impl RestError {
                 "processing",
                 message.clone(),
             ),
+            // Unreachable from either renderer since #504: both go through
+            // [`Self::client_outcome`], which intercepts `ValidationFailed`
+            // above this table and surfaces the validator's own multi-issue
+            // outcome. Kept so the match stays exhaustive, and because a
+            // caller wanting only a summary line is still entitled to one.
             RestError::ValidationFailed { .. } => (
                 StatusCode::UNPROCESSABLE_ENTITY,
                 "processing",
@@ -487,18 +556,43 @@ impl RestError {
             ),
         }
     }
+
+    /// The client-facing `(status, OperationOutcome)` for this error.
+    ///
+    /// **This is the only place a [`RestError`] becomes an OperationOutcome.**
+    /// [`IntoResponse`] renders the pair as an HTTP response body and
+    /// `handlers::batch` renders it as a `Bundle.entry.response.outcome`;
+    /// neither builds its own. That is the point. Before #504 the batch
+    /// handler had a second renderer that hardcoded `"code": "processing"`,
+    /// so the identical failure carried `forbidden` at `GET [base]/Patient/1`
+    /// and `processing` for the same read inside a Bundle entry — and
+    /// [`Self::client_response`]'s promise one doc comment above, that it is
+    /// "shared by `IntoResponse` and the batch/transaction handler so both
+    /// sanitize identically", was not true.
+    ///
+    /// `ValidationFailed` is surfaced verbatim rather than collapsed, because
+    /// it already carries a fully-formed multi-issue outcome from the
+    /// write-path validator — per-issue `code`, `severity` and `expression`.
+    /// The interception has to happen **here** rather than at any call site:
+    /// [`Self::client_response`]'s own `ValidationFailed` arm returns
+    /// `(422, "processing", "Resource validation failed")`, so a caller that
+    /// reached the code table first would re-flatten it.
+    pub(crate) fn client_outcome(&self) -> (StatusCode, serde_json::Value) {
+        if let RestError::ValidationFailed { outcome } = self {
+            return (StatusCode::UNPROCESSABLE_ENTITY, outcome.clone());
+        }
+        let (status, code, details) = self.client_response();
+        (status, create_operation_outcome("error", code, &details))
+    }
 }
 
 impl IntoResponse for RestError {
     fn into_response(self) -> Response {
-        // ValidationFailed carries a fully-formed OperationOutcome (potentially
-        // many issues from the write-path validator); surface it verbatim
-        // rather than collapsing it to the generic single-issue shape.
-        if let RestError::ValidationFailed { outcome } = &self {
-            return (StatusCode::UNPROCESSABLE_ENTITY, Json(outcome.clone())).into_response();
-        }
-        let (status, code, details) = self.client_response();
-        let operation_outcome = create_operation_outcome("error", code, &details);
+        // Both the HTTP body and a Bundle entry's `response.outcome` are
+        // rendered by `client_outcome`, so the two cannot describe the same
+        // failure differently (#504). It also carries the `ValidationFailed`
+        // pass-through that used to live here.
+        let (status, operation_outcome) = self.client_outcome();
 
         // Unauthorized additionally carries a Bearer challenge in the
         // WWW-Authenticate header.
@@ -557,7 +651,11 @@ impl IntoResponse for RestError {
 /// * `severity` - The issue severity (fatal, error, warning, information)
 /// * `code` - The FHIR issue code
 /// * `details` - Human-readable details
-fn create_operation_outcome(severity: &str, code: &str, details: &str) -> serde_json::Value {
+pub(crate) fn create_operation_outcome(
+    severity: &str,
+    code: &str,
+    details: &str,
+) -> serde_json::Value {
     serde_json::json!({
         "resourceType": "OperationOutcome",
         "issue": [{