Fail loud in the app shell; alarm on client_check_failed

backend/e2e/canonical-capability-behavior.spec.js

@@ -179,7 +179,9 @@ function observeBrowserErrors(page) {
       message: message.text(),
     });
   });
-  page.on('requestfailed', (request) => entries.push({
+  // The shell reports deliberate failures to /api/client-errors best-effort; a
+  // report cut off by navigation is telemetry, not a page failure.
+  page.on('requestfailed', (request) => request.url().endsWith('/api/client-errors') || entries.push({
     kind: 'requestfailed',
     url: request.url(),
     failure: request.failure()?.errorText || 'unknown',

backend/e2e/crm-startup.spec.js

@@ -103,7 +103,7 @@ function watchRequests(page) {
     });
   });
   page.on('requestfailed', (request) => {
-    if (!started.has(request)) return;
+    if (!started.has(request) || request.url().endsWith('/api/client-errors')) return;
     failed.push({
       method: request.method(),
       path: shortUrl(request.url()),

backend/e2e/docs-outage.spec.js

@@ -162,6 +162,12 @@ function observeErrors(page) {
  * @param {Array<object>} entries collected observer entries
  * @param {(entry: object) => boolean} [alsoAttributable] extra declared responses
  */
+// The shell reports each deliberate failure to /api/client-errors on a
+// best-effort basis. Those reports are a declared consequence of the outage: a
+// report may be cut off by navigation or throttled by the per-IP budget that
+// every test in a file shares.
+const isClientErrorReport = (entry) => new URL(entry.url).pathname === '/api/client-errors';
+
 function expectOnlyDeliberateDocsOutage(entries, alsoAttributable = () => false) {
   const docs503s = entries.filter(
     (entry) => entry.kind === 'response' && entry.status === 503 && isDocsUrl(entry.url),
@@ -174,12 +180,13 @@ function expectOnlyDeliberateDocsOutage(entries, alsoAttributable = () => false)
   const attributable = new Set([
     ...outageResponses,
     ...entries.filter((entry) => entry.kind === 'console' && outageUrls.has(entry.url)),
+    ...entries.filter(isClientErrorReport),
   ]);
   const unexpected = entries.filter((entry) => !attributable.has(entry)).map(describeEntry);
   expect(unexpected).toEqual([]);
   expect(docs503s.length).toBeGreaterThan(0);
   expect(entries.filter((entry) => entry.kind === 'pageerror').map(describeEntry)).toEqual([]);
-  expect(entries.filter((entry) => entry.kind === 'requestfailed').map(describeEntry)).toEqual([]);
+  expect(entries.filter((entry) => entry.kind === 'requestfailed' && !isClientErrorReport(entry)).map(describeEntry)).toEqual([]);
 }
 
 async function portalContext(browser, server, options = {}) {

backend/src/routes/clientErrors.ts

@@ -62,5 +62,7 @@ export function handleClientErrorRoute(event: LambdaEvent): LambdaResponse {
     page: field(body.page).split(/[?#]/)[0],
     userAgent: headerValue(event.headers, 'user-agent').slice(0, MAX_FIELD_LENGTH),
   }));
-  return noStoreJson(204, {});
+  // 200, not 204: Chromium reports a POST answered with 204 as net::ERR_ABORTED,
+  // which surfaces as a failed request in the browser and in e2e observers.
+  return noStoreJson(200, {});
 }

backend/tests/client-errors.test.ts

@@ -24,10 +24,14 @@ describe('client error reports', () => {
 
   it('logs one structured ERROR line without query strings', () => {
     let status = 0;
+    let body: string | undefined;
     const lines = captureErrors(() => {
-      status = handleClientErrorRoute(event({ surface: 'sponsor-doc', check: 'GET /api/sponsor-docs', status: 503, message: 'x', page: '/sponsor-docs/sdoc-1?code=SECRET' }, '198.51.100.1')).statusCode;
+      const response = handleClientErrorRoute(event({ surface: 'sponsor-doc', check: 'GET /api/sponsor-docs', status: 503, message: 'x', page: '/sponsor-docs/sdoc-1?code=SECRET' }, '198.51.100.1'));
+      status = response.statusCode;
+      body = response.body;
     });
-    assert.equal(status, 204);
+    assert.equal(status, 200, 'not 204: Chromium reports a 204 POST as ERR_ABORTED');
+    assert.equal(body, '{}');
     assert.equal(lines.length, 1);
     const entry = JSON.parse(lines[0]);
     assert.equal(entry.event, 'client_check_failed');

frontend/src/shell/api.js

@@ -1,3 +1,14 @@
+import {
+  failureMessage,
+  reportClientFailure,
+} from "../core/guarded-fetch.js";
+
+// Shell reads can sit behind a cold Lambda start and a large listing, so they
+// get longer than the anonymous pages' 20s; writes get the full Lambda budget.
+const READ_TIMEOUT_MS = 30000;
+const WRITE_TIMEOUT_MS = 120000;
+const MAX_REPORTS_PER_PAGE = 5;
+
 export function resolveApiBase({ documentRef, windowRef }) {
   const meta = documentRef
     .querySelector('meta[name="api-base"]')
@@ -9,7 +20,12 @@ export function resolveApiBase({ documentRef, windowRef }) {
   return "";
 }
 
-export function createApiClient({ apiBase, fetchImpl, storage }) {
+export function createApiClient({
+  apiBase,
+  fetchImpl,
+  storage,
+  timeoutMs = READ_TIMEOUT_MS,
+}) {
   function apiUrl(path) {
     return new URL(path, apiBase);
   }
@@ -19,6 +35,17 @@ export function createApiClient({ apiBase, fetchImpl, storage }) {
   // and blob downloads never coalesce.
   const inflightGets = new Map();
 
+  // One failing page load fires many failing requests. Report each distinct
+  // (check, status) once, capped per page, so the server's per-IP budget is
+  // not spent on duplicates and no 429s show up as browser errors.
+  const reported = new Set();
+  function reportOnce(report) {
+    const key = `${report.check} ${report.status ?? report.message}`;
+    if (reported.has(key) || reported.size >= MAX_REPORTS_PER_PAGE) return;
+    reported.add(key);
+    reportClientFailure(report, { fetchImpl });
+  }
+
   async function request(url, options = {}) {
     const method = String(options.method || "GET").toUpperCase();
     const coalesceKey =
@@ -53,10 +80,35 @@ export function createApiClient({ apiBase, fetchImpl, storage }) {
       headers.Authorization = `Bearer ${token}`;
     }
     const { responseType, ...fetchOptions } = options;
-    const response = await fetchImpl(url, {
-      ...fetchOptions,
-      headers,
-    });
+    // Reads fail fast; writes and downloads may legitimately run as long as the
+    // Lambda (120s) before the visitor is told it failed.
+    const deadlineMs =
+      String(fetchOptions.method || "GET").toUpperCase() === "GET" &&
+      responseType !== "blob"
+        ? timeoutMs
+        : Math.max(timeoutMs, WRITE_TIMEOUT_MS);
+    const checkPath = new URL(String(url), apiBase || "http://localhost").pathname;
+    const what = `load ${checkPath}`;
+    // Every shell request has a deadline and a visible failure: a hung API call
+    // must reject with a message the surface renders, never spin forever.
+    let response;
+    try {
+      response = await fetchImpl(url, {
+        ...fetchOptions,
+        headers,
+        signal: fetchOptions.signal || AbortSignal.timeout(deadlineMs),
+      });
+    } catch (cause) {
+      if (fetchOptions.signal?.aborted && cause?.name === "AbortError") throw cause;
+      const status =
+        cause?.name === "TimeoutError" || cause?.name === "AbortError"
+          ? "timeout"
+          : "network";
+      reportOnce({ surface: "app-shell", check: checkPath, status: null, message: status });
+      const error = new Error(failureMessage(what, status));
+      error.status = status;
+      throw error;
+    }
 
     if (response.ok && responseType === "blob") return response.blob();
     const text = await response.text();
@@ -82,6 +134,15 @@ export function createApiClient({ apiBase, fetchImpl, storage }) {
       throw error;
     }
 
+    if (response.status >= 500) {
+      reportOnce({
+        surface: "app-shell",
+        check: checkPath,
+        status: response.status,
+        message: String(payload?.error || ""),
+      });
+    }
+
     if (!response.ok) {
       const fallback = `HTTP ${response.status}${
         response.statusText ? ` ${response.statusText}` : ""

frontend/test/api-client.test.mjs

@@ -1,5 +1,5 @@
 import assert from "node:assert/strict";
-import { describe, test } from "node:test";
+import { after, describe, test } from "node:test";
 
 import { createApiClient } from "../src/shell/api.js";
 
@@ -67,3 +67,84 @@ describe("createApiClient request coalescing", () => {
     assert.equal(calls.length, 2);
   });
 });
+
+describe("createApiClient fail-loud", () => {
+  function client(fetchImpl, timeoutMs = 20) {
+    return createApiClient({
+      apiBase: "https://ops.example.invalid",
+      storage: { getItem: () => null },
+      fetchImpl,
+      timeoutMs,
+    });
+  }
+
+  test("a hung request times out with a visible message and is reported", async () => {
+    const reports = [];
+    const { request } = client((url, options) => {
+      if (String(url).endsWith("/api/client-errors")) {
+        reports.push(JSON.parse(options.body));
+        return Promise.resolve({});
+      }
+      return new Promise((_, reject) =>
+        options.signal.addEventListener("abort", () => reject(options.signal.reason)),
+      );
+    });
+    // AbortSignal.timeout timers are unref'd in Node; hold the loop open until it fires.
+    const keepAlive = setTimeout(() => {}, 5000);
+    after(() => clearTimeout(keepAlive));
+    await assert.rejects(request(new URL("https://ops.example.invalid/api/me?x=1")), (error) => {
+      assert.equal(error.status, "timeout");
+      assert.match(error.message, /Could not load \/api\/me: it took too long/);
+      return true;
+    });
+    assert.equal(reports[0].surface, "app-shell");
+    assert.equal(reports[0].check, "/api/me");
+  });
+
+  test("a 5xx is reported and still rejects with the server error", async () => {
+    const reports = [];
+    const { request } = client((url, options) => {
+      if (String(url).endsWith("/api/client-errors")) {
+        reports.push(JSON.parse(options.body));
+        return Promise.resolve({});
+      }
+      return Promise.resolve({
+        ok: false,
+        status: 503,
+        statusText: "x",
+        text: async () => JSON.stringify({ error: "down" }),
+      });
+    });
+    await assert.rejects(request(new URL("https://ops.example.invalid/api/users")), /down/);
+    assert.equal(reports[0].status, 503);
+  });
+
+  test("a 4xx is not reported", async () => {
+    const reports = [];
+    const { request } = client((url, options) => {
+      if (String(url).endsWith("/api/client-errors")) reports.push(options.body);
+      return Promise.resolve({ ok: false, status: 403, statusText: "", text: async () => "{}" });
+    });
+    await assert.rejects(request(new URL("https://ops.example.invalid/api/users")));
+    assert.equal(reports.length, 0);
+  });
+
+  test("repeated failures are reported once per check and capped per page", async () => {
+    const reports = [];
+    const { request } = client((url, options) => {
+      if (String(url).endsWith("/api/client-errors")) {
+        reports.push(JSON.parse(options.body));
+        return Promise.resolve({});
+      }
+      return Promise.resolve({ ok: false, status: 503, statusText: "", text: async () => "{}" });
+    });
+    for (let i = 0; i < 4; i += 1) {
+      await assert.rejects(request(new URL("https://ops.example.invalid/api/same"), { method: "POST" }));
+    }
+    for (let i = 0; i < 10; i += 1) {
+      await assert.rejects(request(new URL(`https://ops.example.invalid/api/many-${i}`)));
+    }
+    assert.equal(reports.filter((report) => report.check === "/api/same").length, 1);
+    assert.equal(reports.length, 5);
+  });
+});

infra/template.full.yaml

@@ -1645,6 +1645,36 @@ Resources:
       TreatMissingData: notBreaching
       AlarmActions: [!Ref ConversationalRolloutAlarmTopic]
 
+  # Browser-reported failures (anonymous pages and the app shell post to
+  # /api/client-errors, which logs one ERROR client_check_failed line). Paged
+  # on the shared operator topic: this stack's own alarm topic has no
+  # subscribers, so alarms on it reach nobody.
+  ClientCheckFailedMetricFilter:
+    Type: AWS::Logs::MetricFilter
+    Properties:
+      LogGroupName: !Ref ConversationalBackendLogGroup
+      FilterPattern: '{ $.event = "client_check_failed" }'
+      MetricTransformations:
+        - MetricNamespace: DataOps/Client
+          MetricName: ClientCheckFailed
+          MetricValue: "1"
+          DefaultValue: 0
+
+  ClientCheckFailedAlarm:
+    Type: AWS::CloudWatch::Alarm
+    Properties:
+      AlarmName: !Sub ${AWS::StackName}-client-check-failed
+      AlarmDescription: A browser reported a failed or timed-out page/API check (client_check_failed). See the backend log group.
+      Namespace: DataOps/Client
+      MetricName: ClientCheckFailed
+      Statistic: Sum
+      Period: 300
+      EvaluationPeriods: 1
+      Threshold: 1
+      ComparisonOperator: GreaterThanOrEqualToThreshold
+      TreatMissingData: notBreaching
+      AlarmActions: [!Ref DynamoPitrAlertTopicArn]
+
 Outputs:
   ConversationalRolloutAlarmTopicArn:
     Value: !Ref ConversationalRolloutAlarmTopic