HugoFara · HugoFara · Aug 27, 2026
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,20 @@ ones are marked like "v1.0.0-fork".
 
 ## [Unreleased]
 
+### Fixed
+
+* **A failed API write no longer reports success** (#284). Handlers signal
+  failure by returning an `error` payload rather than by throwing, and the
+  routers passed that straight to `Response::success()` — so the request came
+  back HTTP 200 and the interface, seeing a success, did nothing at all. Such
+  a payload is now recognised and sent as 400. The body is unchanged, so
+  anything reading the message out of it keeps working.
+
+* **API error messages reach the interface** (#284). The API client only ever
+  looked for `message` on a failed request, while the API sends `error` — so
+  even a well-formed error was shown as a bare "HTTP 400: Bad Request". It now
+  reads both.
+
 ## [3.5.0-fork] - 2026-08-27
 
 ### Added

diff --git a/src/backend/Api/V1/Response.php b/src/backend/Api/V1/Response.php
@@ -26,6 +26,16 @@
  */
 class Response
 {
+    /**
+     * Status given to a handler payload that reports a failure.
+     *
+     * 400 rather than a shape-specific code: the payloads carry a message but
+     * no indication of *why* they failed, and guessing 404 from wording would
+     * break as soon as a message is translated. A handler that knows better
+     * should call {@see self::error()} or {@see self::notFound()} directly.
+     */
+    private const FAILURE_STATUS = 400;
+
     /**
      * Create JSON response.
      *
@@ -42,16 +52,64 @@ public static function send(int $status, mixed $data): JsonResponse
     /**
      * Create success response.
      *
+     * Many handlers report a failure by *returning* `['error' => ...]` or
+     * `['success' => false, ...]` rather than by throwing, and every router
+     * hands that return value straight to this method. Sent as-is it becomes
+     * HTTP 200, so a failed write is indistinguishable from a successful one:
+     * `fetch` reports `ok`, the client takes the payload as data, and the UI
+     * silently does nothing (issue #284 — observed on POST /terms/quick, where
+     * the underlying duplicate-key failure of #283 was invisible).
+     *
+     * Rather than rewrite 300-odd return sites, the shape is recognised here
+     * and given a 4xx status. The body is left exactly as the handler built
+     * it, so anything already reading `error` out of the payload keeps working
+     * — only the status becomes honest.
+     *
      * @param mixed $data   Response data
      * @param int   $status HTTP status code (default 200)
      *
      * @return JsonResponse
      */
     public static function success(mixed $data, int $status = 200): JsonResponse
     {
+        // Only the default is promoted: a caller that named a status meant it.
+        if ($status === 200 && self::signalsFailure($data)) {
+            $status = self::FAILURE_STATUS;
+        }
+
         return self::send($status, $data);
     }
 
+    /**
+     * Does this payload report a failure rather than carry a result?
+     *
+     * `'error' => null` is common on the *success* branch of handlers that
+     * always include the key, so presence alone means nothing — the value has
+     * to be a non-empty message (or a bare `true` flag) to count.
+     *
+     * @param mixed $data Payload a handler returned
+     *
+     * @return bool True when the payload describes a failure
+     */
+    private static function signalsFailure(mixed $data): bool
+    {
+        if (!is_array($data)) {
+            return false;
+        }
+
+        if (array_key_exists('success', $data) && $data['success'] === false) {
+            return true;
+        }
+
+        /** @var mixed $error */
+        $error = $data['error'] ?? null;
+        if (is_string($error)) {
+            return trim($error) !== '';
+        }
+
+        return $error === true;
+    }
+
     /**
      * Create error response.
      *

diff --git a/src/frontend/js/shared/api/client.ts b/src/frontend/js/shared/api/client.ts
@@ -481,6 +481,28 @@ async function parseResponse<T>(response: Response): Promise<T> {
   }
 }
 
+/**
+ * Pull a human-readable message out of a failed response.
+ *
+ * The API is not consistent about where the text lives. `Response::error()`
+ * emits `{error: "..."}`, some handlers return `{error: "...", success: false}`
+ * of their own, and the exception handler emits `{error: true, message: "..."}`
+ * — so `error` is only usable as the message when it is a string. Falling all
+ * the way through to the status line keeps a message on screen either way.
+ *
+ * Reading `error` at all is new: every wrapper below used to look only at
+ * `message`, which meant that even a correctly-formed error response arrived
+ * in the UI as a bare "HTTP 400: Bad Request".
+ */
+async function errorMessageFrom(response: Response): Promise<string> {
+  const body = await parseResponse<{ message?: unknown; error?: unknown }>(response);
+  const fromError = typeof body.error === 'string' ? body.error : '';
+  const fromMessage = typeof body.message === 'string' ? body.message : '';
+  return (
+    fromError || fromMessage || `HTTP ${response.status}: ${response.statusText}`
+  );
+}
+
 /**
  * Make a GET request to the API.
  *
@@ -505,12 +527,7 @@ export async function apiGet<T>(
     });
 
     if (!response.ok) {
-      const errorData = await parseResponse<{ message?: string }>(response);
-      return {
-        error:
-          errorData.message ||
-          `HTTP ${response.status}: ${response.statusText}`
-      };
+      return { error: await errorMessageFrom(response) };
     }
 
     const data = await parseResponse<T>(response);
@@ -542,12 +559,7 @@ export async function apiPost<T>(
     });
 
     if (!response.ok) {
-      const errorData = await parseResponse<{ message?: string }>(response);
-      return {
-        error:
-          errorData.message ||
-          `HTTP ${response.status}: ${response.statusText}`
-      };
+      return { error: await errorMessageFrom(response) };
     }
 
     const data = await parseResponse<T>(response);
@@ -579,12 +591,7 @@ export async function apiPut<T>(
     });
 
     if (!response.ok) {
-      const errorData = await parseResponse<{ message?: string }>(response);
-      return {
-        error:
-          errorData.message ||
-          `HTTP ${response.status}: ${response.statusText}`
-      };
+      return { error: await errorMessageFrom(response) };
     }
 
     const data = await parseResponse<T>(response);
@@ -618,12 +625,7 @@ export async function apiDelete<T>(
     const response = await apiFetch(defaultConfig.baseUrl + endpoint, options);
 
     if (!response.ok) {
-      const errorData = await parseResponse<{ message?: string }>(response);
-      return {
-        error:
-          errorData.message ||
-          `HTTP ${response.status}: ${response.statusText}`
-      };
+      return { error: await errorMessageFrom(response) };
     }
 
     const data = await parseResponse<T>(response);
@@ -654,17 +656,7 @@ export async function apiPostMultipart<T>(
     });
 
     if (!response.ok) {
-      const errorData = await parseResponse<{ message?: unknown; error?: unknown }>(response);
-      // An error envelope may carry `error: true` alongside the real text in
-      // `message`, so only a string is usable as the message itself.
-      const fromError = typeof errorData.error === 'string' ? errorData.error : '';
-      const fromMessage = typeof errorData.message === 'string' ? errorData.message : '';
-      return {
-        error:
-          fromError ||
-          fromMessage ||
-          `HTTP ${response.status}: ${response.statusText}`
-      };
+      return { error: await errorMessageFrom(response) };
     }
 
     const data = await parseResponse<T>(response);
@@ -704,12 +696,7 @@ export async function apiPostForm<T>(
     });
 
     if (!response.ok) {
-      const errorData = await parseResponse<{ message?: string }>(response);
-      return {
-        error:
-          errorData.message ||
-          `HTTP ${response.status}: ${response.statusText}`
-      };
+      return { error: await errorMessageFrom(response) };
     }
 
     const respData = await parseResponse<T>(response);

diff --git a/tests/backend/Api/V1/ResponseTest.php b/tests/backend/Api/V1/ResponseTest.php
@@ -61,6 +61,84 @@ public function testSuccessWithCustomStatus(): void
         $this->assertEquals(201, $response->getStatusCode());
     }
 
+    /**
+     * A payload that reports a failure must not be sent as 200.
+     */
+    public function testSuccessPromotesAnErrorPayloadToFourHundred(): void
+    {
+        $data = ['error' => 'Duplicate entry'];
+        $response = Response::success($data);
+
+        $this->assertEquals(400, $response->getStatusCode());
+        // The body is passed through untouched: callers already reading the
+        // message out of the payload keep finding it there.
+        $this->assertEquals($data, $response->getData());
+    }
+
+    /**
+     * The `success => false` convention is recognised too.
+     */
+    public function testSuccessPromotesAnUnsuccessfulPayloadToFourHundred(): void
+    {
+        $response = Response::success(['success' => false, 'error' => 'Nope']);
+
+        $this->assertEquals(400, $response->getStatusCode());
+    }
+
+    /**
+     * `error => null` is what handlers emit on the way *out* of a success
+     * branch, so it must stay a 200.
+     */
+    public function testSuccessKeepsTwoHundredWhenTheErrorKeyIsEmpty(): void
+    {
+        foreach ([null, '', '   '] as $empty) {
+            $response = Response::success(['success' => true, 'error' => $empty]);
+
+            $this->assertEquals(
+                200,
+                $response->getStatusCode(),
+                'error => ' . var_export($empty, true) . ' is not a failure'
+            );
+        }
+    }
+
+    /**
+     * An error envelope may flag failure with `error => true` and carry the
+     * text in `message`.
+     */
+    public function testSuccessPromotesABooleanErrorFlag(): void
+    {
+        $response = Response::success(['error' => true, 'message' => 'Boom']);
+
+        $this->assertEquals(400, $response->getStatusCode());
+    }
+
+    /**
+     * A caller that named its own status meant it.
+     */
+    public function testSuccessLeavesAnExplicitStatusAlone(): void
+    {
+        $response = Response::success(['error' => 'Duplicate entry'], 201);
+
+        $this->assertEquals(201, $response->getStatusCode());
+    }
+
+    /**
+     * Ordinary payloads are untouched, including non-arrays.
+     */
+    public function testSuccessLeavesOrdinaryPayloadsAtTwoHundred(): void
+    {
+        $this->assertEquals(200, Response::success(['id' => 1])->getStatusCode());
+        $this->assertEquals(200, Response::success([])->getStatusCode());
+        $this->assertEquals(200, Response::success('plain')->getStatusCode());
+        $this->assertEquals(200, Response::success(null)->getStatusCode());
+        // "errors" is a result field on batch endpoints, not a failure flag.
+        $this->assertEquals(
+            200,
+            Response::success(['imported' => 5, 'errors' => ['line 3']])->getStatusCode()
+        );
+    }
+
     /**
      * Test error returns JsonResponse with error format.
      */

diff --git a/tests/backend/Modules/Book/Http/BookApiHandlerTest.php b/tests/backend/Modules/Book/Http/BookApiHandlerTest.php
@@ -642,10 +642,14 @@ public function routeDeleteErrorMessageMentionsBookId(): void
     public function routePostHandlesTheCollection(): void
     {
         // No language in the body, so this is the handled-failure path rather
-        // than the trait's old 405 — POST /books is a real endpoint now.
+        // than the trait's old 405 — POST /books is a real endpoint now, and
+        // 400 rather than 405 is what distinguishes the two. It answered 200
+        // until issue #284: the handler reports the missing language by
+        // returning an error payload, which Response::success() used to send
+        // as a success.
         $response = $this->handler->routePost([], []);
 
-        $this->assertSame(200, $response->getStatusCode());
+        $this->assertSame(400, $response->getStatusCode());
     }
 
     // =========================================================================