Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/29782-missing-object-list-read.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
'@object-ui/data-objectstack': patch
---

fix(data-objectstack): a plain list read of an object the server does not know rejects with its 404 code and sentence instead of reading as an empty list

Clause-②: no
Metadata-protocol: none

`ObjectStackAdapter.find` on its plain path (no `$expand`, no `$search`) used to turn every 404 into `{ data: [], total: 0 }` and remember the resource, so every later read of it answered empty without asking the server. That memo exists for optional platform collections such as `sys_presence` and `sys_activity`, whose absence means the deployment does not install the feature. It also swallowed the 404 `OBJECT_NOT_FOUND` of a list's own object, so an object that failed to register read as "no data yet".

The empty answer and the memo now apply only to names in the platform registry (`isPlatformProvidedObjectName` from `@objectstack/spec`). A 404 on any other object is rethrown with its code, status and message, and is not remembered, so lists, boards, widgets and pickers over a missing object show their load-failure state. A dashboard figure over a missing object no longer reads 0. Enable-block denials (`OBJECT_API_DISABLED`, `OBJECT_API_METHOD_NOT_ALLOWED`) are rethrown as before.
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ function makeAdapter(status: number, body: unknown) {
);
});
const ds: any = new ObjectStackAdapter({ baseUrl: 'http://test.local', fetch: fetchImpl });
ds.__fetch = fetchImpl;
return ds;
}

Expand Down Expand Up @@ -154,3 +155,53 @@ describe('object list · an OBJECT_API_DISABLED 404 is not an empty list (#4408)
expect(container.querySelector('[data-testid="list-error-state"]')).toBeNull();
});
});

/**
* objectstack#29782 — the same masking one code over, on the list's plain
* read path. An object that failed to register answers 404
* `OBJECT_NOT_FOUND`. The adapter's plain `find()` (no `$expand`, no
* `$search`) degraded that to zero rows and memoised it, so the list said
* "no data yet" about an object that does not exist, and #29704's panel
* (the server's sentence and code) was reachable only on the raw path. The
* empty answer now belongs to platform collections alone
* (`isPlatformProvidedObjectName`); `sys_jwks` above is one, which is why the
* bare-404 control there still reads empty.
*/
describe('object list · a 404 OBJECT_NOT_FOUND on the plain read path is not an empty list (objectstack#29782)', () => {
const missingSchema: ListViewSchema = {
type: 'list-view',
objectName: 'crm_invoice',
fields: ['name'],
} as ListViewSchema;

it("shows the server's sentence and code, not the empty state", async () => {
const ds = makeAdapter(404, {
success: false,
error: { code: 'OBJECT_NOT_FOUND', message: "Object 'crm_invoice' is not registered" },
});
const { container } = render(
<SchemaRendererProvider dataSource={ds}>
<ListView schema={missingSchema} dataSource={ds} />
</SchemaRendererProvider>,
);

const panel = await waitFor(() => {
const el = container.querySelector('[data-testid="list-error-state"]');
expect(el).not.toBeNull();
return el as HTMLElement;
});

expect(panel.textContent).toContain("Object 'crm_invoice' is not registered");
expect(panel.querySelector('[data-testid="list-error-code"]')?.textContent).toBe('OBJECT_NOT_FOUND');
expect(panel.textContent).not.toMatch(/connection/i);
expect(container.querySelector('[data-testid="empty-state"]')).toBeNull();

// Every data read the list made took the plain path: the masking lived
// there, so a read that went raw would make this pass for the wrong reason.
const dataCalls = (ds.__fetch.mock.calls as unknown[][])
.map(([url]) => String(url))
.filter((u) => u.includes('/data/'));
expect(dataCalls.length).toBeGreaterThan(0);
expect(dataCalls.every((u) => !/populate=|[$]?search=/.test(decodeURIComponent(u)))).toBe(true);
});
});
5 changes: 4 additions & 1 deletion ui/packages/data-objectstack/src/apiAccessDenied.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,13 @@ describe('ObjectStackDataSource.find — enable-block denials are surfaced, not
});

it('CONTROL: a 404 RECORD_NOT_FOUND is not an enable-block denial', async () => {
// On a platform collection, so the 404 takes the optional-collection
// branch. A non-registry object's 404 is rethrown instead
// (findMissingObject-29782.test.ts).
const find = vi.fn().mockRejectedValue(clientError('No such record', 404, 'RECORD_NOT_FOUND'));
const ds = makeDS(find);

await expect(ds.find('account')).resolves.toMatchObject({ data: [], total: 0 });
await expect(ds.find('sys_comment')).resolves.toMatchObject({ data: [], total: 0 });
});

it('CONTROL: a 2xx with zero rows resolves as an ordinary empty result', async () => {
Expand Down
164 changes: 164 additions & 0 deletions ui/packages/data-objectstack/src/findMissingObject-29782.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* A plain list read of an object the server does not know must reach the
* surface as a failure, not as "no data yet".
*
* `find()`'s SDK path (no `$expand`, no `$search`) degraded every 404 to
* `{ data: [], total: 0 }` and memoised the resource in `missingResources`.
* That memo exists for OPTIONAL platform collections (`sys_presence`,
* `sys_activity`, …), whose absence means "this deployment does not install
* the feature". It also swallowed the 404 of a list's OWN object: an object
* that failed to register answered 404 `OBJECT_NOT_FOUND`, the list showed its
* empty state, and every later read of that object short-circuited to empty
* without asking again.
*
* The error code cannot split the two cases: an uninstalled `sys_presence` and
* an unregistered `crm_invoice` both answer 404 `OBJECT_NOT_FOUND`. The split
* is by name, against the platform's curated registry
* (`isPlatformProvidedObjectName`, @objectstack/spec): a registry name keeps
* the empty answer and the memo; any other object's 404 is rethrown and not
* memoised.
*/

import { describe, it, expect, vi } from 'vitest';
import { isPlatformProvidedObjectName } from '@objectstack/spec/system';
import { ObjectStackAdapter } from './index';

/** An error shaped the way `@objectstack/client`'s fetch wrapper throws one. */
function clientError(message: string, httpStatus: number, code?: string) {
const err = new Error(message) as Error & { httpStatus: number; code?: string };
err.httpStatus = httpStatus;
if (code) err.code = code;
return err;
}

function makeDS(find: any) {
const ds: any = new ObjectStackAdapter({
baseUrl: 'http://test.local',
fetch: vi.fn(async () =>
new Response(JSON.stringify({ success: true, data: { capabilities: {}, routes: {} } }), {
status: 200,
headers: { 'Content-Type': 'application/json' },
})),
});
ds.connected = true;
ds.connectionState = 'connected';
ds.client = { data: { find } };
return ds;
}

const MISSING = 'crm_invoice';
const NOT_FOUND_SENTENCE = "Object 'crm_invoice' is not registered";

describe('find — a 404 on a non-platform object is rethrown, not emptied', () => {
it('the names these tests lean on sit on the right side of the registry', () => {
expect(isPlatformProvidedObjectName(MISSING)).toBe(false);
expect(isPlatformProvidedObjectName('sys_presence')).toBe(true);
expect(isPlatformProvidedObjectName('sys_activity')).toBe(true);
});

it('REJECTS a plain find answered 404 OBJECT_NOT_FOUND, with the code, status and sentence', async () => {
const find = vi.fn().mockRejectedValue(clientError(NOT_FOUND_SENTENCE, 404, 'OBJECT_NOT_FOUND'));
const ds = makeDS(find);

await expect(ds.find(MISSING, { $top: 0 })).rejects.toMatchObject({
code: 'OBJECT_NOT_FOUND',
httpStatus: 404,
message: NOT_FOUND_SENTENCE,
});
});

it('does not memoise it: a second find asks the server again and rejects again', async () => {
const find = vi.fn().mockRejectedValue(clientError(NOT_FOUND_SENTENCE, 404, 'OBJECT_NOT_FOUND'));
const ds = makeDS(find);

await expect(ds.find(MISSING)).rejects.toMatchObject({ code: 'OBJECT_NOT_FOUND' });
await expect(ds.find(MISSING, { $top: 25 })).rejects.toMatchObject({ code: 'OBJECT_NOT_FOUND' });
expect(find).toHaveBeenCalledTimes(2);
});

it('a bare 404 with no code on a non-platform object is rethrown too', async () => {
const find = vi.fn().mockRejectedValue(clientError('Not found', 404));
const ds = makeDS(find);

await expect(ds.find(MISSING)).rejects.toMatchObject({ httpStatus: 404 });
await expect(ds.find(MISSING)).rejects.toMatchObject({ httpStatus: 404 });
expect(find).toHaveBeenCalledTimes(2);
});

it('over the real client: the wrapped 404 envelope reaches the caller as code + status', async () => {
const dataUrls: string[] = [];
const fetchImpl = vi.fn(async (url: RequestInfo | URL) => {
const href = String(url);
if (href.includes('/data/')) {
dataUrls.push(href);
return new Response(
JSON.stringify({ success: false, error: { code: 'OBJECT_NOT_FOUND', message: NOT_FOUND_SENTENCE } }),
{ status: 404, headers: { 'Content-Type': 'application/json' } },
);
}
return new Response(JSON.stringify({ success: true, data: { capabilities: {}, routes: {} } }), {
status: 200,
headers: { 'Content-Type': 'application/json' },
});
});
const ds: any = new ObjectStackAdapter({ baseUrl: 'http://test.local', fetch: fetchImpl });
ds.connected = true;
ds.connectionState = 'connected';

await expect(ds.find(MISSING, { $top: 0 })).rejects.toMatchObject({
code: 'OBJECT_NOT_FOUND',
httpStatus: 404,
});
await expect(ds.find(MISSING, { $top: 0 })).rejects.toMatchObject({ code: 'OBJECT_NOT_FOUND' });
expect(dataUrls).toHaveLength(2);
expect(dataUrls.every((u) => u.includes(`/data/${MISSING}`))).toBe(true);
});
});

describe('find — CONTROLS: the platform optional-collection half is unchanged', () => {
it('CONTROL: a registry name answered 404 OBJECT_NOT_FOUND degrades to empty and is memoised', async () => {
const find = vi.fn().mockRejectedValue(clientError("Object 'sys_presence' not found", 404, 'OBJECT_NOT_FOUND'));
const ds = makeDS(find);

await expect(ds.find('sys_presence')).resolves.toMatchObject({ data: [], total: 0 });
// Short-circuited: the second call never reaches the server.
await expect(ds.find('sys_presence', { $top: 5 })).resolves.toMatchObject({ data: [], total: 0 });
expect(find).toHaveBeenCalledTimes(1);
});

it('CONTROL: the memo is per resource: an emptied platform name does not empty a missing object', async () => {
const find = vi.fn(async (resource: string) => {
throw clientError(`Object '${resource}' not found`, 404, 'OBJECT_NOT_FOUND');
});
const ds = makeDS(find);

await expect(ds.find('sys_activity')).resolves.toMatchObject({ data: [], total: 0 });
await expect(ds.find(MISSING)).rejects.toMatchObject({ code: 'OBJECT_NOT_FOUND' });
});

it('CONTROL: an enable-block denial still rethrows, on a platform name and on any other', async () => {
const find = vi.fn().mockRejectedValue(clientError('Object API is disabled', 404, 'OBJECT_API_DISABLED'));
const ds = makeDS(find);

await expect(ds.find('sys_jwks')).rejects.toMatchObject({ code: 'OBJECT_API_DISABLED', httpStatus: 404 });
await expect(ds.find(MISSING)).rejects.toMatchObject({ code: 'OBJECT_API_DISABLED', httpStatus: 404 });
// Neither is memoised: the platform name is asked again.
await expect(ds.find('sys_jwks')).rejects.toMatchObject({ code: 'OBJECT_API_DISABLED' });
expect(find).toHaveBeenCalledTimes(3);
});

it('CONTROL: a 2xx with zero rows on a non-platform object is still an ordinary empty result', async () => {
const find = vi.fn().mockResolvedValue({ records: [], total: 0 });
const ds = makeDS(find);

await expect(ds.find(MISSING)).resolves.toMatchObject({ data: [], total: 0 });
});
});
35 changes: 27 additions & 8 deletions ui/packages/data-objectstack/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { isRefusedTextComparand, textComparandRefusalReason } from '@objectstack
import type { ApiError, BatchUpdateResponse, ValidateDataIssue } from '@objectstack/spec/api';
import { ValidateDataIssueSchema } from '@objectstack/spec/api';
import { stripReadDecorations } from '@objectstack/spec/kernel';
import { isPlatformProvidedObjectName } from '@objectstack/spec/system';
// #4237 — the metadata save door's advisory reader, shared with `MetadataClient`
// rather than forked. ONE reader, two call sites: the other client class calls it
// from `MetadataClient.save` (#4133/#4236), this one from the interceptor below.
Expand Down Expand Up @@ -3448,10 +3449,13 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
private inflightFieldAccess = new Map<string, Promise<RecordFieldAccess | null>>();
private unansweredFieldAccess = new Set<string>();
private fieldAccessUnsupported = false;
// Resources that have responded 404 at least once (collection not installed
// on this backend). Subsequent find() calls short-circuit to an empty result
// so optional collections like sys_presence don't hammer the server with
// failing requests on every record open / panel render.
// Platform collections that have responded 404 at least once (the package
// that registers them is not installed on this backend). Subsequent find()
// calls short-circuit to an empty result so optional collections like
// sys_presence don't hammer the server with failing requests on every record
// open / panel render. Only names in the platform registry
// (`isPlatformProvidedObjectName`, @objectstack/spec) ever enter it: a 404
// on any other object is the surface's own object missing, and is rethrown.
private missingResources = new Set<string>();
// Set once the server has told us it can't do a cross-object transactional
// batch (the client SDK's data.batchTransaction threw HTTP 404/405/501). After
Expand Down Expand Up @@ -3812,9 +3816,10 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
* Converts OData-style params to ObjectStack query options.
*/
async find(resource: string, params?: QueryParams): Promise<QueryResult<T>> {
// Short-circuit when this resource has previously responded 404 — the
// collection isn't installed on this backend. Callers (AppHeader,
// RecordDetailView, …) treat empty data as "feature unavailable".
// Short-circuit when this platform collection has previously responded
// 404 — the package that registers it isn't installed on this backend.
// Callers (AppHeader, RecordDetailView, …) treat empty data as "feature
// unavailable". Only registry names are ever memoised (see the catch).
if (this.missingResources.has(resource)) {
return { data: [], total: 0 } as QueryResult<T>;
}
Expand Down Expand Up @@ -3849,7 +3854,21 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
// (objectui#4408). Rethrow so the surface can say what happened; the
// `missingResources` memo must not absorb it either, or the very first
// denial would silently pin every later call to empty.
if (!isApiAccessDeniedError(err) && is404Error(err)) {
//
// A 404 degrades to empty ONLY for a platform-provided collection
// (the curated registry in @objectstack/spec): there, absence means
// "this deployment does not install the feature", and the optional
// probes read empty as exactly that. The error code cannot make this
// split: an uninstalled sys_presence and a list's own unregistered
// object both answer 404 `OBJECT_NOT_FOUND`. Any other object is the
// surface's own, so its 404 is rethrown, unmemoised, and the surface's
// load-failure state shows the server's code and sentence instead of
// "no data yet" (an object that failed to register is not empty).
if (
!isApiAccessDeniedError(err)
&& is404Error(err)
&& isPlatformProvidedObjectName(resource)
) {
// Mark the resource so subsequent calls don't repeat the 404.
this.missingResources.add(resource);
return { data: [], total: 0 } as QueryResult<T>;
Expand Down
Loading