fix(client): normalize both server error envelopes so err.code / err.fields mean one thing (#3918 follow-up)

.changeset/client-error-envelope-normalisation.md

@@ -0,0 +1,51 @@
+---
+"@objectstack/client": patch
+---
+
+fix(client): normalize both server error envelopes so `err.code` / `err.fields` mean one thing (#3918 follow-up)
+
+Two envelopes are in play and they disagree about where the semantic code and
+the per-field list live:
+
+```
+@objectstack/rest, flat:
+  { error, code: 'VALIDATION_FAILED', fields: [...] }
+
+runtime dispatcher, wrapped:
+  { success: false, error: { message, code: 400,
+      details: { code: 'VALIDATION_FAILED', fields: [...] } } }
+```
+
+`error.code` in the **wrapped** form is the HTTP status, not a semantic code.
+The client read it straight through, so `err.code` was the **number 400** where
+the flat envelope gave `'VALIDATION_FAILED'` — meaning the branch our own docs
+teach,
+
+```js
+if (err.code === 'VALIDATION_FAILED') err.fields.forEach(…)
+```
+
+never matched on a dispatcher-served surface, and the field list (put on the
+wire for those routes by #3918) was unreachable at `err.details.error.details.fields`.
+
+Now normalized at the throw site:
+
+- **`err.code` is always the semantic string.** It is read from the flat
+  `code`, else the wrapped `error.details.code`, else a *string* `error.code` —
+  a numeric value is never reported as a code. The HTTP status is on
+  `err.httpStatus`, where it always was.
+- **`err.fields` is the per-field list** whenever the server sent one, from
+  either envelope. It is left **unset** (not `[]`) when there is none, so
+  `if (err.fields)` is a safe test for "this failure is field-anchored".
+- **`err.details`** prefers a top-level `details` (unchanged), then the wrapped
+  envelope's own `details`, then the whole body. The flat envelope has no
+  top-level `details` and so keeps falling through to the whole body exactly as
+  before — only the wrapped shape changes, and only from "the entire response"
+  to the structured object it actually carries.
+
+**Behaviour change worth noting:** code that read `err.code` from a
+dispatcher-served route previously got a number and now gets a string (or
+`undefined` where the server sent no semantic code). Nothing in this repo did —
+`err.httpStatus` was always the correct source for the status, and remains
+untouched — but a consumer that branched on `err.code === 400` should move to
+`err.httpStatus === 400`.

content/docs/api/client-sdk.mdx

@@ -517,6 +517,34 @@ try {
 }
 ```
 
+`error.code` is always the **semantic** code as a string — the numeric HTTP
+status lives on `error.httpStatus` and nowhere else. This holds regardless of
+which server surface answered: the REST server replies with a flat
+`{ error, code, fields }` body while the runtime dispatcher replies with a
+wrapped `{ success, error: { message, code, details } }` body whose `error.code`
+is the HTTP status, and the client normalizes both before throwing.
+
+### Per-field validation errors
+
+A validation failure additionally carries `error.fields` — one entry per
+offending field, ready to attach to the inputs that produced them:
+
+```typescript
+try {
+  await client.data.create('contact', { email: 'not-an-email' });
+} catch (error) {
+  if (error.code === 'VALIDATION_FAILED') {
+    for (const f of error.fields ?? []) {
+      showFieldError(f.field, f.message);   // 'email', 'email must be a valid email address'
+    }
+  }
+}
+```
+
+`error.fields` is left **unset** (not `[]`) when the server reported no
+per-field detail, so `if (error.fields)` is a safe test for "this failure is
+field-anchored".
+
 ### Error Codes
 
 | Code | HTTP | Category | Retryable | Description |

content/docs/api/error-catalog.mdx

@@ -389,30 +389,49 @@ interface FieldError {
 ## Client-Side Error Handling
 
 <Callout type="warn">
-The `@objectstack/client` SDK's built-in fetch error handling attaches only
-`code`, `category`, `httpStatus`, `retryable`, and `details` to the thrown
-error — not `fieldErrors`, `retryAfter`, or `requestId` directly. If your
-server populates those on the response body, read them from
-`apiError.details` (e.g. `apiError.details?.fieldErrors`) until the client
-surfaces them at the top level.
+The `@objectstack/client` SDK's built-in fetch error handling attaches
+`code`, `category`, `httpStatus`, `retryable`, `details`, and — for validation
+failures — `fields`. It does **not** attach `retryAfter` or `requestId`
+directly; if your server populates those on the response body, read them from
+`apiError.details` until the client surfaces them at the top level.
+
+Two naming details matter here:
+
+- The per-field list is `apiError.fields`, matching the wire (`fields[]`), not
+  the spec contract's `fieldErrors`. Each entry is `{ field, code, message }`.
+  It is left **unset** when the server reported no per-field detail, so
+  `if (apiError.fields)` tests "this failure is field-anchored".
+- `apiError.code` is always the semantic code as a **string**. The numeric HTTP
+  status is on `apiError.httpStatus` and nowhere else. This holds for both wire
+  formats: the flat REST envelope carries the code at the top level, the runtime
+  dispatcher's wrapped envelope carries it in `error.details.code` (its
+  `error.code` is the HTTP status), and the client normalizes both before
+  throwing.
 </Callout>
 
 ### TypeScript Example
 
 ```typescript
-import type { EnhancedApiError } from '@objectstack/spec/api';
+import type { EnhancedApiError, FieldError } from '@objectstack/spec/api';
+
+// The spec type names the per-field list `fieldErrors`; what the client
+// actually attaches at runtime is `fields` (the wire name). Widen the cast
+// until the two names converge.
+type ThrownApiError = EnhancedApiError & { fields?: FieldError[] };
 
 async function handleApiCall() {
   try {
     const result = await client.data.create('task', { title: 'New Task' });
     return result;
   } catch (error) {
-    const apiError = error as EnhancedApiError;
+    const apiError = error as ThrownApiError;
 
     switch (apiError.category) {
       case 'validation':
-        // Show field-level errors to the user
-        apiError.fieldErrors?.forEach(fe => {
+        // Show field-level errors to the user. `fields` is what the client
+        // attaches (the wire name); the spec type calls the same idea
+        // `fieldErrors`.
+        apiError.fields?.forEach(fe => {
           showFieldError(fe.field, fe.message);
         });
         break;

packages/client/src/client.test.ts

@@ -1458,6 +1458,111 @@ describe('HTTP error shaping', () => {
     });
 });
 
+// ---------------------------------------------------------------------------
+// #3918 follow-up — one error shape across both server envelopes.
+//
+// `@objectstack/rest` answers flat (`{ error, code, fields }`); the runtime
+// dispatcher answers wrapped (`{ success, error: { message, code, details } }`)
+// where `error.code` is the HTTP STATUS and the semantic code lives in
+// `details.code`. Reading the wrapped `error.code` straight through handed
+// callers the number 400 where the flat form handed them 'VALIDATION_FAILED',
+// so the branch our own docs teach never matched on a dispatcher-served
+// surface. These pin the normalisation that makes `err.code` / `err.fields`
+// mean the same thing whichever surface answered.
+// ---------------------------------------------------------------------------
+describe('HTTP error shaping — envelope normalisation', () => {
+    const FIELDS = [
+        { field: 'email', code: 'invalid_email', message: 'email must be a valid email address' },
+    ];
+
+    /** What @objectstack/rest's `mapDataError` puts on the wire. */
+    const FLAT = { error: 'Validation failed', code: 'VALIDATION_FAILED', fields: FIELDS };
+
+    /** What the runtime dispatcher puts on the wire since #3918. */
+    const WRAPPED = {
+        success: false,
+        error: {
+            message: 'Validation failed',
+            code: 400,
+            details: { code: 'VALIDATION_FAILED', fields: FIELDS },
+        },
+    };
+
+    it('exposes the SEMANTIC code from the flat envelope', async () => {
+        const { client } = createMockClient(FLAT, 400);
+        const caught: any = await client.data.delete('pm_base', 'rec_1').catch((e) => e);
+
+        expect(caught.code).toBe('VALIDATION_FAILED');
+        expect(caught.httpStatus).toBe(400);
+    });
+
+    it('exposes the SEMANTIC code from the wrapped envelope, not the HTTP status', async () => {
+        // The regression this guards: `caught.code` used to be the number 400.
+        const { client } = createMockClient(WRAPPED, 400);
+        const caught: any = await client.data.delete('pm_base', 'rec_1').catch((e) => e);
+
+        expect(caught.code).toBe('VALIDATION_FAILED');
+        expect(caught.code).not.toBe(400);
+        // The status is still available — it just isn't `code`.
+        expect(caught.httpStatus).toBe(400);
+    });
+
+    it('exposes `fields[]` at the same place for BOTH envelopes', async () => {
+        const flat: any = await createMockClient(FLAT, 400).client.data
+            .delete('pm_base', 'rec_1').catch((e) => e);
+        const wrapped: any = await createMockClient(WRAPPED, 400).client.data
+            .delete('pm_base', 'rec_1').catch((e) => e);
+
+        expect(flat.fields).toEqual(FIELDS);
+        expect(wrapped.fields).toEqual(FIELDS);
+    });
+
+    it('leaves `fields` unset when the server sent none', async () => {
+        // Callers branch on presence; an empty array would be a lie about a
+        // failure that had nothing to do with per-field validation.
+        const { client } = createMockClient({ error: 'nope', code: 'PERMISSION_DENIED' }, 403);
+        const caught: any = await client.data.delete('pm_base', 'rec_1').catch((e) => e);
+
+        expect(caught.fields).toBeUndefined();
+        expect(caught.code).toBe('PERMISSION_DENIED');
+    });
+
+    it('never reports a numeric code, even with no details to fall back on', async () => {
+        // A pre-#3918 dispatcher body: wrapped, but no `details` at all.
+        const { client } = createMockClient(
+            { success: false, error: { message: 'boom', code: 500 } },
+            500,
+        );
+        const caught: any = await client.data.delete('pm_base', 'rec_1').catch((e) => e);
+
+        expect(caught.code).toBeUndefined();
+        expect(caught.message).toBe('boom');
+        expect(caught.httpStatus).toBe(500);
+    });
+
+    it('keeps `details` = the whole body for the flat envelope (unchanged)', async () => {
+        const { client } = createMockClient(FLAT, 400);
+        const caught: any = await client.data.delete('pm_base', 'rec_1').catch((e) => e);
+
+        expect(caught.details).toEqual(FLAT);
+    });
+
+    it('points `details` at the structured object for the wrapped envelope', async () => {
+        const { client } = createMockClient(WRAPPED, 400);
+        const caught: any = await client.data.delete('pm_base', 'rec_1').catch((e) => e);
+
+        expect(caught.details).toEqual({ code: 'VALIDATION_FAILED', fields: FIELDS });
+    });
+
+    it('still honours a top-level `details` when the server sends one', async () => {
+        const body = { message: 'bad', code: 'X', details: { limit: 1000 } };
+        const { client } = createMockClient(body, 400);
+        const caught: any = await client.data.delete('pm_base', 'rec_1').catch((e) => e);
+
+        expect(caught.details).toEqual({ limit: 1000 });
+    });
+});
+
 describe('packages.install', () => {
     const MANIFEST = { id: 'com.acme.crm', name: 'Acme CRM', version: '1.0.0', type: 'app' };
 

packages/client/src/index.ts

@@ -4378,20 +4378,55 @@ export class ObjectStackClient {
           ?? errorBody?.error?.message
           ?? (typeof errorBody?.error === 'string' ? errorBody.error : undefined)
           ?? res.statusText;
-        const errorCode = errorBody?.code || errorBody?.error?.code;
+        // Two server envelopes are in play, and they disagree about where the
+        // SEMANTIC code and the per-field list live:
+        //
+        //   @objectstack/rest, flat:
+        //     { error, code: 'VALIDATION_FAILED', fields: [...] }
+        //   runtime dispatcher, wrapped:
+        //     { success: false, error: { message, code: 400,
+        //         details: { code: 'VALIDATION_FAILED', fields: [...] } } }
+        //
+        // Note `error.code` in the WRAPPED form is the HTTP status, not a
+        // semantic code. Reading it straight into `err.code` handed callers the
+        // number 400 where the flat form handed them 'VALIDATION_FAILED', so the
+        // branch our own docs teach —
+        //   `if (err.code === 'VALIDATION_FAILED') err.fields.forEach(…)`
+        // — simply never matched on a dispatcher-served surface. #3918 put the
+        // field list on the wire for those routes; this is what lets a caller
+        // reach it without knowing which surface answered.
+        //
+        // So: `err.code` is always the semantic STRING (the numeric status is on
+        // `err.httpStatus`, where it always was), and `err.fields` is always the
+        // per-field list when the server sent one.
+        const asSemanticCode = (v: unknown) => (typeof v === 'string' && v ? v : undefined);
+        const errorCode =
+          asSemanticCode(errorBody?.code)
+          ?? asSemanticCode(errorBody?.error?.details?.code)
+          ?? asSemanticCode(errorBody?.error?.code);
+        const fieldErrors =
+          Array.isArray(errorBody?.fields) ? errorBody.fields
+          : Array.isArray(errorBody?.error?.details?.fields) ? errorBody.error.details.fields
+          : undefined;
         // `.message` is what UIs (e.g. the console's error toast) show to end
         // users verbatim, so keep it to the server's human-readable message —
         // no `[ObjectStack]` branding and no `CODE:` prefix. The code stays
         // available programmatically via `error.code`, and the full response
         // body was already logged above for debugging.
         const error = new Error(errorMessage) as any;
-        
+
         // Attach error details for programmatic access
         error.code = errorCode;
         error.category = errorBody?.category;
         error.httpStatus = res.status;
         error.retryable = errorBody?.retryable;
-        error.details = errorBody?.details || errorBody;
+        // Prefer the wrapped envelope's own `details` over the whole body. The
+        // flat envelope has no top-level `details`, so it keeps falling through
+        // to `errorBody` exactly as before — only the wrapped shape changes,
+        // and only from "the entire response" to the structured object it
+        // actually carries.
+        error.details = errorBody?.details ?? errorBody?.error?.details ?? errorBody;
+        if (fieldErrors) error.fields = fieldErrors;
 
         throw error;
     }