Skip to content

Commit aad5ee0

Browse files
fix(app-shell): a route whose object read the server answered shows the server reason, not "check your connection" (#29883)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9Ubb1XxsxCUUTENZK7nrfYe3Zo)_ Fixes #29728 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:应用路由读取对象定义失败时,如果服务器其实已经回答了(5xx,或带有本版本不认识的错误码的 404),页面现在显示服务器给出的原因和错误码,而不再让人「检查网络连接」。 - **为什么改**:服务器明明说了原因,界面却让用户去排查网络,误导用户也误导排障;列表页、记录页、应用页早已按同一规则显示服务器原因,路由级这一处是最后一个。 - **风险与回滚**:风险低。适配器只是在抛出的包装错误上多带一个 `details.originalError`,类、code、statusCode、message 都不变;网络中断和 403 仍显示原来的文案,重试行为不变。回滚即还原本 PR。 - **先看**:`ui/packages/data-objectstack/src/index.ts`(getObjectSchema 的非 404 分支)、`ui/packages/app-shell/src/providers/MetadataProvider.tsx`(失败读取保存服务器回答)、`ui/packages/app-shell/src/console/AppContent.tsx`(RouteObjectReadFailed)。 - **两分钟验证**:在 `ui/` 下运行 `pnpm exec vitest run packages/app-shell/src/console/__tests__/AppContent.disabledPackageDeepLink-13052.test.tsx`,预期 18 个用例全部通过,其中 503 用例显示 SERVICE_UNAVAILABLE 和服务器原文。 - **需要你做的**:无,席位落地。 ## What changes On main, an app route whose by-name object read failed drew "Check your connection and try again" whatever happened: an answered 503, an answered 500, a 404 with a code this build does not know, a network drop and a 403 all drew the same sentence. Measured on `99c06965` through the real `ObjectStackAdapter` under the real `MetadataProvider` and `AppContent`. The provider never saw the server's answer. `requestObjectSchema` throws `Error{status, code, message}` with the server's own code and sentence, but `getObjectSchema`'s non-404 catch rewrapped it with `createErrorFromResponse` and kept no reference to it. A 503 became `ConnectionError` `CONNECTION_ERROR` "Connection error: ...", a 500 became `UNKNOWN_ERROR`, and a network drop got an invented status 500. - **`@object-ui/data-objectstack`**: `getObjectSchema`'s non-404 rewrap now carries the transport error as `details.originalError`, as its 404 arm (`MetadataNotFoundError`) and three other wrap sites in the package already do. The wrapper's class, `code`, `statusCode` and `message` are unchanged, and `createErrorFromResponse` keeps its signature. - **`MetadataProvider`**: `readObjectThroughAdapter` unwraps `details.originalError` for any `ObjectStackError` (before, only `MetadataNotFoundError`). A failed by-name read stores `answeredLoadFailure(err)` beside the failed name, in a new `answers` record on the scope entry. Nothing is stored for a refused read (`forbidden` / `unauthorized`), which keeps its state as `PageView` does. The record is dropped when the name is answered or invalidated, and returned beside `failed` / `refused`. - **`appObjectScope`**: `failedAnswer` is the answer of the first failed name that has one. It is carried on the object, page and dashboard route paths. - **`AppContent`**: `RouteObjectReadFailed` draws `answer.message` in place of the network sentence, plus a code chip (`route-object-read-failed-code`) in `PageView`'s shape. No i18n rows were added, and Retry is unchanged. ## Tests - `AppContent.disabledPackageDeepLink-13052.test.tsx`: a new top-level describe over the REAL adapter driven by a fake `fetch`. It pins: - an answered 503 (wrapped envelope), a 500 (flat envelope) and an unknown-code 404 each show the server code and sentence, and not the network sentence; - a page route that binds the object shows the 503 answer too; - CONTROLs: a network drop keeps the network sentence with no chip; a 403 keeps today's state; `RESOURCE_NOT_FOUND` and `OBJECT_PACKAGE_DISABLED` keep theirs; - Retry still asks once more and clears the answer, and a retry that gets no answer draws the network sentence again. - `getObjectSchema.test.ts`: a new describe pins the wrapper's class, code, statusCode and message, plus `details.originalError`, for a 503, a 500 and a network drop, and a 404 CONTROL. <details><summary>Ablations (committed head, ablation-replace.mjs, restored blob == HEAD)</summary> - Drop the provider unwrap (back to `MetadataNotFoundError` only): 5 of 9 new route pins went red (503, 500, page 503, network CONTROL, retry-to-network). The unknown-404 pin stayed green, because a 404 is still unwrapped. - Drop the `answers` record (`if (answer && false)`): 6 of 9 went red (503, 500, unknown-404, page 503, both Retry pins). </details> <details><summary>Verification</summary> - Related sweep: 301 app-shell test files naming `appObjectScope` / `AppContent` / `MetadataProvider` / `route-object-read-failed` (6 batches, all green), plus the whole data-objectstack suite (114 files, green). - `turbo run type-check --filter=@object-ui/app-shell --filter=@object-ui/data-objectstack`: 66/66 tasks. - eslint on the six touched files: 0 errors. - Derived gates (`dispatch-gates --commands`): 36 derived, 36 run, all exit 0, reconciled with `--ran` (0 NOT-MEASURED), at `ff352f41`. The head `effe42c8` adds only a merge of one unrelated main commit (cloud-connection). - Sweep, type-check and ablations ran at `2a886b17` / `ff352f41`; the touched files are unchanged since. </details> ## Acceptance notes - **Verdict edge:** with the unwrap, a non-404 status that carries `RESOURCE_NOT_FOUND` or `OBJECT_PACKAGE_DISABLED` is now read by its code: an absence or a refusal, not a failure. No server path is known to answer that way. - **Finding, filed by the seat:** `createErrorFromResponse` stamps `500` on any error with no status. As a result, every `getObjectSchema` caller that reads the wrapper takes a network drop for an answered 500 (43 source files, 58 call sites under `ui/`). This PR reads the cause instead and leaves the wrapper as it is. --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jTGjiGTnPZnatdavVb)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 00f72f9 commit aad5ee0

7 files changed

Lines changed: 391 additions & 26 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
'@object-ui/data-objectstack': patch
4+
---
5+
6+
Clause-②: no
7+
Metadata-protocol: none
8+
9+
An app route whose object definition the server failed to serve now shows the server's own reason, instead of asking the person to check their connection (#29728).
10+
11+
- When the by-name object read is answered with a 5xx, or with a 404 whose code this build does not know, the route's failed-read state draws the server's sentence and its code, the same way the list, record and app-page states already do (`answeredLoadFailure`). "Check your connection and try again" now appears only when no server answered. A refused read (403) keeps the state it drew before. Retry is unchanged.
12+
- `@object-ui/data-objectstack`: `ObjectStackAdapter.getObjectSchema` keeps the transport's own error as `details.originalError` when it rewraps a non-404 failure, as its 404 arm already did. The thrown error's class, `code`, `statusCode` and `message` are unchanged. Read the cause, not the wrapper, to learn whether and what the server answered: the wrapper replaces the server's code with its own (`CONNECTION_ERROR`, `UNKNOWN_ERROR`) and reports status 500 for a request that got no answer.

‎ui/packages/app-shell/src/console/AppContent.tsx‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { useAssistant } from '../assistant/assistantBus.js';
1414
import { ModalForm } from '@object-ui/plugin-form';
1515
import { Empty, EmptyTitle, EmptyDescription, Button, DataErrorState } from '@object-ui/components';
1616
import { toast } from 'sonner';
17-
import { useActionRunner, useGlobalUndo, useMutationInvalidationBridge, notifyDataChanged, FilterScopeProvider, resolveApprovalLock, claimAnswerWarnings, toastRecordSave, type AnswerWarningClaim, type RecordSaveFeedback } from '@object-ui/react';
17+
import { useActionRunner, useGlobalUndo, useMutationInvalidationBridge, notifyDataChanged, FilterScopeProvider, resolveApprovalLock, claimAnswerWarnings, toastRecordSave, type AnswerWarningClaim, type AnsweredLoadFailure, type RecordSaveFeedback } from '@object-ui/react';
1818
import { useRecordApprovals, openApprovalOf } from '../hooks/useRecordApprovals.js';
1919
import { useObjectTranslation, useObjectLabel, recordSaveMessage as buildRecordSaveMessage } from '@object-ui/i18n';
2020
import type { AppAccessVerdict, ConnectionState } from '@object-ui/data-objectstack';
@@ -475,6 +475,7 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps =
475475
scope: objectScope,
476476
ready: objectScopeReady,
477477
failedNames: routeObjectReadsFailed,
478+
failedAnswer: routeObjectReadAnswer,
478479
routeObjectRefusal,
479480
} = readAppObjectScope(
480481
readObjectScope,
@@ -1553,7 +1554,9 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps =
15531554
// objectui#13052 — the route's object read failed (a 5xx, a
15541555
// network drop, a code this build does not know): say so in
15551556
// the shell, and let Retry ask those names once more.
1557+
// objectstack#29728 — in the server's words when it answered.
15561558
<RouteObjectReadFailed
1559+
answer={routeObjectReadAnswer}
15571560
onRetry={() => {
15581561
for (const name of routeObjectReadsFailed) metadata.invalidate('object', name);
15591562
}}
@@ -2137,8 +2140,15 @@ function RouteNotFound() {
21372140
* Retry that asks again once (`invalidate` drops the failed read; the next
21382141
* render asks it). The list's own load-failure copy: this is the same
21392142
* question, a list that could not be loaded.
2143+
*
2144+
* objectstack#29728 — when a server answered the read (a 5xx, a 404 with a
2145+
* code this build does not know), `answer` carries its code and sentence
2146+
* (`answeredLoadFailure`, the rule the list, record and app page share): the
2147+
* sentence replaces the network one, and the code is drawn beneath it, as the
2148+
* app page's failed-read state draws it. The network sentence is only for a
2149+
* read no server answered.
21402150
*/
2141-
function RouteObjectReadFailed({ onRetry }: { onRetry: () => void }) {
2151+
function RouteObjectReadFailed({ answer, onRetry }: { answer: AnsweredLoadFailure | null; onRetry: () => void }) {
21422152
const { t } = useObjectTranslation();
21432153
// #25915 — drawn by the shared `DataErrorState`, so the failure is
21442154
// `role="alert"` by construction. Retry is passed as `children` (not
@@ -2148,10 +2158,18 @@ function RouteObjectReadFailed({ onRetry }: { onRetry: () => void }) {
21482158
<DataErrorState
21492159
data-testid="route-object-read-failed"
21502160
title={t('list.loadErrorTitle', { defaultValue: 'Couldn\u2019t load records' })}
2151-
message={t('list.loadErrorMessage', {
2161+
message={answer?.message ?? t('list.loadErrorMessage', {
21522162
defaultValue: 'Something went wrong while loading this data. Check your connection and try again.',
21532163
})}
21542164
>
2165+
{answer?.code ? (
2166+
<code
2167+
data-testid="route-object-read-failed-code"
2168+
className="mb-3 rounded bg-muted px-1.5 py-0.5 font-mono text-xs text-muted-foreground"
2169+
>
2170+
{answer.code}
2171+
</code>
2172+
) : null}
21552173
<Button onClick={onRetry} data-testid="route-object-read-failed-retry">
21562174
{t('common.retry', { defaultValue: 'Retry' })}
21572175
</Button>

‎ui/packages/app-shell/src/console/__tests__/AppContent.disabledPackageDeepLink-13052.test.tsx‎

Lines changed: 173 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@
1818
* (d) a 404 carrying a code this build does not know draws the failed state;
1919
* CONTROL: a served object renders, and every open asks its object once.
2020
*
21+
* objectstack#29728 (its own describe below, over a REAL `ObjectStackAdapter`
22+
* driven by a fake `fetch`): a failed read the server ANSWERED (a 5xx, a 404
23+
* with a code this build does not know) draws the server's sentence and code
24+
* in that failed state; the network sentence is only for a read that got no
25+
* answer, and a refused read (403) keeps it, as before.
26+
*
2127
* The harness is `AppContent.objectScope-12811.test.tsx`'s: the REAL
2228
* `MetadataProvider` under the real `AppContent`, with a client double that
2329
* records each call by URL. The double throws what the real adapter throws for
@@ -35,7 +41,7 @@ import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-libra
3541
import { MemoryRouter, Routes, Route, useParams } from 'react-router-dom';
3642
import { AuthProvider, ActiveOrganizationStorage } from '@object-ui/auth';
3743
import type { AuthClient, AuthOrganization } from '@object-ui/auth';
38-
import { MetadataNotFoundError } from '@object-ui/data-objectstack';
44+
import { MetadataNotFoundError, ObjectStackAdapter } from '@object-ui/data-objectstack';
3945

4046
vi.mock('@object-ui/plugin-designer', async (importOriginal) => ({
4147
...(await importOriginal<typeof import('@object-ui/plugin-designer')>()),
@@ -307,3 +313,169 @@ describe('a deep link to an object read by name always leaves the splash (object
307313
expect(screen.queryByRole('alert')).not.toBeInTheDocument();
308314
});
309315
});
316+
317+
// ── objectstack#29728 — the failed state says what the server said ─────────
318+
319+
type Answered = 'served' | 'wrapped-503' | 'flat-500' | 'unknown-404' | 'network' | 'forbidden-403' | 'absent' | 'disabled';
320+
321+
const wire = (status: number, body: unknown) =>
322+
new Response(JSON.stringify(body), { status, headers: { 'content-type': 'application/json' } });
323+
324+
function answerWire(answer: Answered): Response {
325+
switch (answer) {
326+
case 'served':
327+
return wire(200, { success: true, data: { type: 'object', name: PROBE, item: structuredClone(PROBE_DEF) } });
328+
case 'wrapped-503':
329+
return wire(503, { success: false, error: { code: 'SERVICE_UNAVAILABLE', message: 'The metadata store is down for maintenance.' } });
330+
case 'flat-500':
331+
return wire(500, { error: 'Database connection refused.', code: 'INTERNAL_ERROR' });
332+
case 'unknown-404':
333+
return wire(404, { success: false, error: { code: 'SOME_FUTURE_REFUSAL', message: 'This object is quarantined.' } });
334+
case 'forbidden-403':
335+
return wire(403, { success: false, error: { code: 'PERMISSION_DENIED', message: `You may not read ${PROBE}.` } });
336+
case 'absent':
337+
return wire(404, { success: false, error: { code: 'RESOURCE_NOT_FOUND', message: 'not found' } });
338+
case 'disabled':
339+
return wire(404, { success: false, error: { code: 'OBJECT_PACKAGE_DISABLED', message: 'disabled package' } });
340+
case 'network':
341+
throw new TypeError('Failed to fetch');
342+
}
343+
}
344+
345+
const PROBE_PAGE = { name: 'probe_page', label: 'Probe page', type: 'app', object: PROBE, _packageId: PKG, regions: [] };
346+
const wireServer = { calls: [] as string[], next: 'served' as Answered };
347+
let wireSeq = 0;
348+
349+
/** The REAL adapter (its error rewrap included), over a fake `fetch`. */
350+
function makeRealAdapter() {
351+
return new ObjectStackAdapter({
352+
baseUrl: `http://route-answer-29728-${(wireSeq += 1)}.test`,
353+
autoReconnect: false,
354+
fetch: (async (input: RequestInfo | URL) => {
355+
const url = new URL(String(input));
356+
if (url.pathname.endsWith('/discovery')) return wire(200, { success: true, data: {} });
357+
wireServer.calls.push(url.pathname);
358+
if (url.pathname === `/api/v1/meta/object/${PROBE}`) return answerWire(wireServer.next);
359+
if (url.pathname === `/api/v1/meta/page/${PROBE_PAGE.name}`) {
360+
return wire(200, { success: true, data: { type: 'page', name: PROBE_PAGE.name, item: structuredClone(PROBE_PAGE) } });
361+
}
362+
const list = /^\/api\/v1\/meta\/([a-z_]+)$/.exec(url.pathname);
363+
if (list) {
364+
const pkg = url.searchParams.get('packageId');
365+
const all: any[] = list[1] === 'object' ? OBJECTS : list[1] === 'app' ? APPS : list[1] === 'page' ? [PROBE_PAGE] : [];
366+
const items = pkg ? all.filter((o) => o._packageId === pkg) : all;
367+
return wire(200, { success: true, data: { type: list[1], items: structuredClone(items) } });
368+
}
369+
return wire(404, { success: false, error: { code: 'RESOURCE_NOT_FOUND', message: `no route ${url.pathname}` } });
370+
}) as typeof fetch,
371+
});
372+
}
373+
374+
function bootReal(url: string) {
375+
render(
376+
<AuthProvider authUrl="/api/auth" client={authClient()}>
377+
<MetadataProvider adapter={makeRealAdapter()}>
378+
<MemoryRouter initialEntries={[url]}>
379+
<Routes>
380+
<Route path="/apps/:appName/*" element={<AppContent />} />
381+
</Routes>
382+
</MemoryRouter>
383+
</MetadataProvider>
384+
</AuthProvider>,
385+
);
386+
}
387+
388+
const NETWORK_SENTENCE = 'Check your connection and try again.';
389+
const wireProbeReads = () => wireServer.calls.filter((c) => c === `/api/v1/meta/object/${PROBE}`).length;
390+
391+
describe('a route whose object read the server answered says what the server said (objectstack#29728)', () => {
392+
beforeEach(() => {
393+
wireServer.calls = [];
394+
wireServer.next = 'served';
395+
});
396+
397+
const answeredCases = [
398+
{ answer: 'wrapped-503', code: 'SERVICE_UNAVAILABLE', sentence: 'The metadata store is down for maintenance.' },
399+
{ answer: 'flat-500', code: 'INTERNAL_ERROR', sentence: 'Database connection refused.' },
400+
{ answer: 'unknown-404', code: 'SOME_FUTURE_REFUSAL', sentence: 'This object is quarantined.' },
401+
] as const;
402+
403+
for (const { answer, code, sentence } of answeredCases) {
404+
it(`an answered ${answer} draws the server's code and sentence, not the network sentence`, async () => {
405+
wireServer.next = answer;
406+
bootReal(DEEP_LINK);
407+
await waitFor(() => expect(screen.getByTestId('route-object-read-failed')).toBeInTheDocument());
408+
const failed = screen.getByTestId('route-object-read-failed');
409+
expect(failed).toHaveTextContent(sentence);
410+
expect(failed).not.toHaveTextContent(NETWORK_SENTENCE);
411+
expect(screen.getByTestId('route-object-read-failed-code')).toHaveTextContent(code);
412+
expect(failed).toContainElement(screen.getByTestId('route-object-read-failed-code'));
413+
expect(splashUp()).toBe(false);
414+
});
415+
}
416+
417+
it('an answered 503 on a PAGE route (the objects its page binds) draws the server code and sentence', async () => {
418+
wireServer.next = 'wrapped-503';
419+
bootReal(`/apps/showcase_app/page/${PROBE_PAGE.name}`);
420+
await waitFor(() => expect(screen.getByTestId('route-object-read-failed')).toBeInTheDocument());
421+
expect(screen.getByTestId('route-object-read-failed')).toHaveTextContent('The metadata store is down for maintenance.');
422+
expect(screen.getByTestId('route-object-read-failed-code')).toHaveTextContent('SERVICE_UNAVAILABLE');
423+
});
424+
425+
it('CONTROL: a request that got no answer keeps the network sentence, with no code', async () => {
426+
wireServer.next = 'network';
427+
bootReal(DEEP_LINK);
428+
await waitFor(() => expect(screen.getByTestId('route-object-read-failed')).toBeInTheDocument());
429+
expect(screen.getByTestId('route-object-read-failed')).toHaveTextContent(NETWORK_SENTENCE);
430+
expect(screen.getByTestId('route-object-read-failed')).not.toHaveTextContent('Failed to fetch');
431+
expect(screen.queryByTestId('route-object-read-failed-code')).not.toBeInTheDocument();
432+
});
433+
434+
it('CONTROL: a refused read (403) keeps the failed state it drew before, with no server text', async () => {
435+
wireServer.next = 'forbidden-403';
436+
bootReal(DEEP_LINK);
437+
await waitFor(() => expect(screen.getByTestId('route-object-read-failed')).toBeInTheDocument());
438+
expect(screen.getByTestId('route-object-read-failed')).toHaveTextContent(NETWORK_SENTENCE);
439+
expect(screen.getByTestId('route-object-read-failed')).not.toHaveTextContent(`You may not read ${PROBE}.`);
440+
expect(screen.queryByTestId('route-object-read-failed-code')).not.toBeInTheDocument();
441+
});
442+
443+
it('CONTROL: RESOURCE_NOT_FOUND and OBJECT_PACKAGE_DISABLED keep their own states', async () => {
444+
wireServer.next = 'absent';
445+
bootReal(DEEP_LINK);
446+
await waitFor(() => expect(screen.getByTestId('object-not-found')).toBeInTheDocument());
447+
expect(screen.queryByTestId('route-object-read-failed')).not.toBeInTheDocument();
448+
cleanup();
449+
450+
wireServer.next = 'disabled';
451+
bootReal(DEEP_LINK);
452+
await waitFor(() => expect(screen.getByTestId('object-package-disabled')).toBeInTheDocument());
453+
expect(screen.queryByTestId('route-object-read-failed')).not.toBeInTheDocument();
454+
});
455+
456+
it('Retry is unchanged: one more read, and a served answer renders the object with no failed state left', async () => {
457+
wireServer.next = 'wrapped-503';
458+
bootReal(DEEP_LINK);
459+
await waitFor(() => expect(screen.getByTestId('route-object-read-failed-code')).toBeInTheDocument());
460+
await settle();
461+
expect(wireProbeReads()).toBe(1);
462+
463+
wireServer.next = 'served';
464+
fireEvent.click(screen.getByTestId('route-object-read-failed-retry'));
465+
await waitFor(() => expect(screen.getByTestId('object-view')).toHaveTextContent(PROBE));
466+
await settle();
467+
expect(wireProbeReads()).toBe(2);
468+
expect(screen.queryByTestId('route-object-read-failed-code')).not.toBeInTheDocument();
469+
});
470+
471+
it('Retry that the server answers differently draws the new answer', async () => {
472+
wireServer.next = 'wrapped-503';
473+
bootReal(DEEP_LINK);
474+
await waitFor(() => expect(screen.getByTestId('route-object-read-failed-code')).toHaveTextContent('SERVICE_UNAVAILABLE'));
475+
wireServer.next = 'network';
476+
fireEvent.click(screen.getByTestId('route-object-read-failed-retry'));
477+
await waitFor(() => expect(wireProbeReads()).toBe(2));
478+
await waitFor(() => expect(screen.queryByTestId('route-object-read-failed-code')).not.toBeInTheDocument());
479+
expect(screen.getByTestId('route-object-read-failed')).toHaveTextContent(NETWORK_SENTENCE);
480+
});
481+
});

0 commit comments

Comments
 (0)