fix(app-shell): the record page reads the lock service's 501 silently and asks no more

.changeset/26632-record-lock-501-silent.md

@@ -0,0 +1,19 @@
+---
+'@object-ui/app-shell': patch
+'@object-ui/console': patch
+---
+
+fix(app-shell): a record page on a deployment without the approvals service no longer logs the record-lock read's 501
+
+Clause-②: no
+Metadata-protocol: none
+
+On a deployment that composes no approvals service, such as a fresh `npm create objectstack` scaffold, every record page asks the server whether an approval locks the record, and the server answers `501 NOT_IMPLEMENTED`. That answer is designed: it means there is no lock service, so no approval can lock the record, and the page already treated it that way. But the read went through the SDK client, which logs every non-2xx response as an error before its caller sees it. So each record open printed `HTTP request failed: GET /api/v1/approvals/records/OBJECT/ID/lock -> 501 [NOT_IMPLEMENTED]` in red, which reads like a broken server.
+
+What changes:
+
+- The record page reads the lock directly, the same way it already reads the approval requests, so no client logger runs first. A 501 is read silently.
+- After a 501, the page does not ask again for any record until the page is reloaded. The browser's own network line for that first request still appears once, because the server answers the approval-requests read `200` with an empty list and the page cannot tell the service is missing until it asks.
+- Any other failure of the lock read still logs, through the same logger the SDK client used: `HTTP request failed: GET URL -> STATUS [CODE]`, with a 404 still demoted to a debug line. The next record open asks again.
+
+What to do: nothing. Nothing a flow, page or object declares changes.

ui/packages/app-shell/src/hooks/useRecordApprovals.ts

@@ -33,12 +33,16 @@
  * override actor), so for anyone else an answered empty read cannot tell "no
  * approval is running" from "one is running that you may not see". When the
  * list finds no pending request, the hook also asks the server whether the
- * record is held by a locking approval (`client.approvals.getRecordLock`,
+ * record is held by a locking approval (`GET /approvals/records/:object/:recordId/lock`,
  * answered to every caller who can read the record — objectstack#25934 ruling
- * A) and hands the answer back as `recordLock`.
+ * A) and hands the answer back as `recordLock`. That door has no empty answer:
+ * a composition without the approvals service answers it 501, the designed
+ * "no lock service", which is read silently and never asked again
+ * (objectstack#26632); any other failure still logs.
  */
 
 import { useCallback, useEffect, useMemo, useRef, useState, useSyncExternalStore } from 'react';
+import { createQuietHttpLogger } from '@object-ui/data-objectstack';
 import { useAdapter } from '../providers/AdapterProvider.js';
 import { bearerAuthHeaders } from '../utils/authToken.js';
 import type { DecisionOutputDef } from '../utils/decisionOutputParams.js';
@@ -304,8 +308,8 @@ interface UseRecordApprovalsResult {
    * no answer to give: a pending request was found (it decides, with its own
    * `lock_record`), no adapter is mounted, or the read has never answered — a
    * composition without the approvals service answers it 501, so there the
-   * record is not locked by this signal. A later failed read keeps the last
-   * answer, as the list read does.
+   * record is not locked by this signal, and the read is not sent again. A
+   * later failed read keeps the last answer, as the list read does.
    */
   recordLock: { locked: boolean } | null;
   refresh: () => Promise<void>;
@@ -317,6 +321,7 @@ export function __resetRecordApprovalsStore(): void {
   inFlight.clear();
   listeners.clear();
   latestSeq.clear();
+  recordLockAbsent = false;
 }
 
 function apiBase() {
@@ -500,16 +505,50 @@ function holdEntry(key: string): () => void {
   };
 }
 
-/** Reads the server's record lock for the record a read is for. */
-type RecordLockReader = () => Promise<{ locked: boolean }>;
+/**
+ * Set once the record-lock read answered 501: this deployment has no approvals
+ * service, so no record is held by a locking approval and the read is not sent
+ * again (objectstack#26632). Module-scoped, not per record: the answer is the
+ * deployment's, and the read goes to the one base every read here goes to.
+ */
+let recordLockAbsent = false;
+
+/** Logs a failed lock read exactly as the console's SDK client logs one. */
+const lockReadLogger = createQuietHttpLogger();
+
+/**
+ * The server's record lock (`IApprovalService.getRecordLock`, objectstack#25934).
+ *
+ * Read through `fetchJson`, not `client.approvals.getRecordLock`: the client
+ * logs every non-2xx as an error before its caller sees the answer, and the
+ * 501 of a composition without the approvals service is this read's designed
+ * answer, not a failure (objectstack#26632). So the 501 is read silently and
+ * latched, and every other failure is logged here, through the same logger
+ * the client would have used.
+ */
+async function readRecordLock(objectName: string, recordId: string): Promise<{ locked: boolean }> {
+  const path = `/approvals/records/${encodeURIComponent(objectName)}/${encodeURIComponent(recordId)}/lock`;
+  try {
+    const body = await fetchJson<any>(path);
+    // `{ success: true, data: { locked } }`, the declared envelope.
+    return body && typeof body.success === 'boolean' && 'data' in body ? body.data : body;
+  } catch (err: any) {
+    if (err?.status === 501) {
+      recordLockAbsent = true;
+    } else {
+      lockReadLogger.error('HTTP request failed', undefined, {
+        method: 'GET',
+        url: `${apiBase()}${path}`,
+        status: err?.status,
+        error: { code: err?.code, message: err?.message },
+      });
+    }
+    throw err;
+  }
+}
 
 /** Send the record's read and publish its answer to every consumer of the key. */
-function startRead(
-  key: string,
-  objectName: string,
-  recordId: string,
-  readLock: RecordLockReader | null,
-): Promise<void> {
+function startRead(key: string, objectName: string, recordId: string, readsLock: boolean): Promise<void> {
   const seq = ++readSeq;
   latestSeq.set(key, seq);
   const entry = entries.get(key);
@@ -529,10 +568,10 @@ function startRead(
       // `lock_record`. Without one, ask the record itself (objectstack#25934):
       // the list hides a request from a non-participant, the lock does not.
       let recordLock: { locked: boolean } | null = null;
-      if (!pending && readLock) {
+      if (!pending && readsLock && !recordLockAbsent) {
         recordLock = entries.get(key)?.snapshot.recordLock ?? null;
         try {
-          const lock = await readLock();
+          const lock = await readRecordLock(objectName, recordId);
           recordLock = { locked: lock?.locked === true };
         } catch {
           // A read that never answered (501 without the approvals service)
@@ -599,14 +638,9 @@ export function useRecordApprovals(
   // for the render, mirrored into a ref the mount effect and `refresh` read.
   const [unavailable, setUnavailable] = useState(false);
   const unavailableRef = useRef(false);
-  // The record-lock read goes through the SDK (`client.approvals.getRecordLock`),
-  // so it needs the adapter's client; with no adapter mounted it is skipped.
-  const adapter = useAdapter();
-  const lockReader = useCallback(
-    (object: string, id: string): RecordLockReader | null =>
-      adapter ? () => adapter.getClient().approvals.getRecordLock(object, id) : null,
-    [adapter],
-  );
+  // The record lock is read only where an adapter is mounted — the console's
+  // connection to an ObjectStack server; a host with none never asked it.
+  const readsLock = !!useAdapter();
 
   const subscribe = useCallback((l: () => void) => subscribeKey(readKey, l), [readKey]);
   const getSnapshot = useCallback(
@@ -629,19 +663,19 @@ export function useRecordApprovals(
     // read only when neither exists. The entry may be new while a read is in
     // flight (StrictMode's replayed mount): that read publishes into it.
     if (entry && !entry.read && !inFlight.has(readKey) && !unavailableRef.current) {
-      void startRead(readKey, objectName, recordId, lockReader(objectName, recordId));
+      void startRead(readKey, objectName, recordId, readsLock);
     } else if (entry && inFlight.has(readKey)) {
       entry.read = true;
       if (!entry.snapshot.loading) publish(readKey, { loading: true });
     }
     return release;
-  }, [readKey, objectName, recordId, lockReader]);
+  }, [readKey, objectName, recordId, readsLock]);
 
   const refresh = useCallback(async () => {
     if (!objectName || !recordId || !readKey) return;
     if (unavailableRef.current) return;
-    await startRead(readKey, objectName, recordId, lockReader(objectName, recordId));
-  }, [objectName, recordId, readKey, lockReader]);
+    await startRead(readKey, objectName, recordId, readsLock);
+  }, [objectName, recordId, readKey, readsLock]);
 
   const requests = readKey === null ? EMPTY_ROWS : snapshot.rows;
   const loading = snapshot.loading;

ui/packages/app-shell/src/views/RecordDetailView.approvalLock-25934.test.tsx

@@ -16,8 +16,12 @@
  * `200 { data: [] }` while a `lockRecord: true` approval runs. The page took
  * that for "no approval is running" and offered inline edit, and the save
  * answered `409 RECORD_LOCKED`. The page now asks the record's own lock
- * (`client.approvals.getRecordLock`, answered to every reader) when the list
- * finds no pending request.
+ * (`GET /approvals/records/:object/:recordId/lock`, answered to every reader)
+ * when the list finds no pending request.
+ *
+ * A composition without the approvals service answers that read 501, its
+ * designed "no lock service": the page reads it silently and does not ask
+ * again, while any other failure still logs (objectstack#26632).
  *
  * Same probe as `RecordDetailView.approvalLock-12220`: it prints what the page
  * threads into its `<InlineEditProvider>`. The record carries no
@@ -126,6 +130,7 @@ const pendingRequest = (over: Record<string, unknown> = {}) => ({
 });
 
 let listGets: string[];
+let lockGets: string[];
 
 const jsonResponse = (status: number, body: unknown) =>
   new Response(JSON.stringify(body), { status, headers: { 'content-type': 'application/json' } });
@@ -137,12 +142,31 @@ const jsonResponse = (status: number, body: unknown) =>
  */
 type ListAnswer = { row: Record<string, unknown> | null } | { absent: number };
 
-function stubApprovalsApi(answer: ListAnswer) {
+/**
+ * What the record-lock route answers: `{ locked }` in the declared envelope,
+ * or a refusal carrying the route's status (501 when the approvals service is
+ * absent, the body `respond501` sends).
+ */
+type LockAnswer = { locked: boolean } | { refused: number };
+
+const LOCK_PATH = `/approvals/records/${OBJECT_NAME}/${RECORD_ID}/lock`;
+
+function stubApprovalsApi(answer: ListAnswer, lock: LockAnswer) {
   listGets = [];
+  lockGets = [];
   vi.stubGlobal(
     'fetch',
     vi.fn(async (url: string) => {
       const u = String(url);
+      if (u.endsWith(LOCK_PATH)) {
+        lockGets.push(u);
+        if ('refused' in lock) {
+          return lock.refused === 501
+            ? jsonResponse(501, { code: 'NOT_IMPLEMENTED', message: 'Approvals service is not configured on this deployment' })
+            : jsonResponse(lock.refused, { success: false, error: { code: 'INTERNAL_ERROR', message: 'boom' } });
+        }
+        return jsonResponse(200, { success: true, data: { locked: lock.locked } });
+      }
       if (u.includes('/approvals/requests?object=')) {
         listGets.push(u);
         if ('absent' in answer) return jsonResponse(answer.absent, { code: 'NOT_FOUND' });
@@ -181,24 +205,20 @@ function makeDataSource() {
 }
 
 /**
- * The record-lock read as the SDK answers it: `{ locked }`, or a refusal
- * carrying the route's status (501 when the approvals service is absent).
+ * The SDK's own record-lock read. The page must not use it: the client logs
+ * every non-2xx as an error before the page sees the answer (objectstack#26632).
  */
-type LockAnswer = { locked: boolean } | { refused: number };
 let getRecordLock: ReturnType<typeof vi.fn>;
 
-function makeAdapter(answer: LockAnswer) {
-  getRecordLock = vi.fn(async () => {
-    if ('refused' in answer) throw Object.assign(new Error(`HTTP ${answer.refused}`), { status: answer.refused });
-    return { locked: answer.locked };
-  });
+function makeAdapter() {
+  getRecordLock = vi.fn(async () => ({ locked: false }));
   return { getClient: () => ({ approvals: { getRecordLock } }) } as any;
 }
 
-function renderRecordPage(lock: LockAnswer) {
+function renderRecordPage() {
   return render(
     <MemoryRouter initialEntries={[`/app/crm/${OBJECT_NAME}/${RECORD_ID}`]}>
-      <AdapterCtx.Provider value={makeAdapter(lock)}>
+      <AdapterCtx.Provider value={makeAdapter()}>
       <MetadataCtx.Provider value={METADATA}>
         <RecordDetailView
           dataSource={makeDataSource()}
@@ -216,24 +236,32 @@ function renderRecordPage(lock: LockAnswer) {
 
 const attr = (probe: HTMLElement, name: string) => probe.getAttribute(`data-${name}`);
 
+let consoleError: ReturnType<typeof vi.spyOn>;
+
 beforeEach(() => {
   cleanup();
   __resetRecordApprovalsStore();
   authFetchSpy.mockClear();
+  consoleError = vi.spyOn(console, 'error');
 });
 
 afterEach(() => {
   vi.unstubAllGlobals();
+  consoleError.mockRestore();
 });
 
+/** The `console.error` lines that name the record-lock read. */
+const lockErrorLines = () =>
+  consoleError.mock.calls.filter((args: unknown[]) => args.some((a) => String(a).includes(LOCK_PATH)));
+
 describe('record page lock for a reader outside the approval (objectstack#25934)', () => {
   it('a non-participant on a record held by a locking approval is offered no inline edit', async () => {
-    stubApprovalsApi({ row: null });
-    renderRecordPage({ locked: true });
+    stubApprovalsApi({ row: null }, { locked: true });
+    renderRecordPage();
     const probe = await screen.findByTestId('inline-probe');
 
     await waitFor(() => expect(attr(probe, 'locked')).toBe('true'));
-    expect(getRecordLock).toHaveBeenCalledWith(OBJECT_NAME, RECORD_ID);
+    expect(lockGets).toHaveLength(1);
     expect(attr(probe, 'approval-read-answered')).toBe('true');
     expect(attr(probe, 'approval-pending')).toBe('true');
     expect(attr(probe, 'can-edit')).toBe('false');
@@ -242,37 +270,72 @@ describe('record page lock for a reader outside the approval (objectstack#25934)
   });
 
   it('a record the server reports unlocked opens editable', async () => {
-    stubApprovalsApi({ row: null });
-    renderRecordPage({ locked: false });
+    stubApprovalsApi({ row: null }, { locked: false });
+    renderRecordPage();
     const probe = await screen.findByTestId('inline-probe');
 
-    await waitFor(() => expect(getRecordLock).toHaveBeenCalled());
+    await waitFor(() => expect(lockGets).toHaveLength(1));
     await waitFor(() => expect(attr(probe, 'approval-read-answered')).toBe('true'));
     expect(attr(probe, 'locked')).toBe('false');
     expect(attr(probe, 'approval-pending')).toBe('false');
     await waitFor(() => expect(attr(probe, 'can-edit')).toBe('true'));
   });
 
   it('without the approvals service (lock read 501) nothing changes: an answered empty read is not locked', async () => {
-    stubApprovalsApi({ row: null });
-    renderRecordPage({ refused: 501 });
+    stubApprovalsApi({ row: null }, { refused: 501 });
+    renderRecordPage();
     const probe = await screen.findByTestId('inline-probe');
 
-    await waitFor(() => expect(getRecordLock).toHaveBeenCalled());
+    await waitFor(() => expect(lockGets).toHaveLength(1));
     await waitFor(() => expect(attr(probe, 'approval-read-answered')).toBe('true'));
     expect(attr(probe, 'locked')).toBe('false');
     expect(attr(probe, 'approval-pending')).toBe('false');
     await waitFor(() => expect(attr(probe, 'can-edit')).toBe('true'));
   });
 
+  it('the 501 of a composition without the approvals service logs nothing, and the lock is not read again (objectstack#26632)', async () => {
+    stubApprovalsApi({ row: null }, { refused: 501 });
+    renderRecordPage();
+    const probe = await screen.findByTestId('inline-probe');
+    await waitFor(() => expect(attr(probe, 'approval-read-answered')).toBe('true'));
+    await waitFor(() => expect(lockGets).toHaveLength(1));
+    expect(lockErrorLines()).toEqual([]);
+    expect(getRecordLock).not.toHaveBeenCalled();
+
+    // Open the record again: the list is read again, the lock is not.
+    cleanup();
+    renderRecordPage();
+    await waitFor(() => expect(listGets.length).toBeGreaterThanOrEqual(2));
+    const again = await screen.findByTestId('inline-probe');
+    await waitFor(() => expect(attr(again, 'approval-read-answered')).toBe('true'));
+    expect(attr(again, 'locked')).toBe('false');
+    expect(lockGets).toHaveLength(1);
+    expect(lockErrorLines()).toEqual([]);
+  });
+
+  it('any other lock-read failure still logs, and the next open asks again', async () => {
+    stubApprovalsApi({ row: null }, { refused: 500 });
+    renderRecordPage();
+    const probe = await screen.findByTestId('inline-probe');
+    await waitFor(() => expect(attr(probe, 'approval-read-answered')).toBe('true'));
+    await waitFor(() => expect(lockErrorLines()).toHaveLength(1));
+    expect(String(lockErrorLines()[0][0])).toContain(`GET `);
+    expect(String(lockErrorLines()[0][0])).toContain('-> 500');
+    expect(attr(probe, 'locked')).toBe('false');
+
+    cleanup();
+    renderRecordPage();
+    await waitFor(() => expect(lockGets).toHaveLength(2));
+  });
+
   it('a participant\'s pending request decides, and the record lock is not read', async () => {
-    stubApprovalsApi({ row: pendingRequest({ lock_record: false }) });
-    renderRecordPage({ locked: true });
+    stubApprovalsApi({ row: pendingRequest({ lock_record: false }) }, { locked: true });
+    renderRecordPage();
     const probe = await screen.findByTestId('inline-probe');
 
     await waitFor(() => expect(attr(probe, 'approval-pending')).toBe('true'));
     expect(attr(probe, 'locked')).toBe('false');
     await waitFor(() => expect(attr(probe, 'can-edit')).toBe('true'));
-    expect(getRecordLock).not.toHaveBeenCalled();
+    expect(lockGets).toEqual([]);
   });
 });