Skip to content

Commit 0b5b176

Browse files
fix(data-objectstack): a request that got no answer no longer reads as an answered 500 (#29915)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9UbbLoUpNB6DHdnmokKa35qYC8)_ Fixes #29885 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:请求根本没有得到服务器回答时(断网、主机不可达),适配器抛出的错误不再带一个编造的 500 状态码;服务器真正回答过的错误(400/401/403/404/500/503/504 等)照旧。 - **为什么改**:带着 500 的断网错误被所有读取方当成「服务器已回答」,于是界面把浏览器的 "Failed to fetch" 当作服务器原话展示,而本该提示用户检查网络连接;#29728 只在路由读取一处绕开了它,其余读取方仍然受骗。 - **风险与回滚**:风险低。只在输入没有状态码时行为变化(`statusCode` 由 500 变为未定义,错误类与 code 不变);已知读取方中无人依赖这个 500(见下方清点)。回滚即还原本 PR。 - **先看**:`ui/packages/data-objectstack/src/errors.ts`(`createErrorFromResponse` 的状态码默认值)、`ui/packages/react/src/utils/error-message.adapter-unanswered-29885.test.ts`(真实适配器 + 共享读取规则的边界钉)。 - **两分钟验证**:在 `ui/` 下运行 `pnpm exec vitest run packages/react/src/utils/error-message.adapter-unanswered-29885.test.ts packages/data-objectstack/src/errors.test.ts`,预期全部通过,其中断网用例 `answeredLoadFailure` 为 null、`classifyLoadError` 为 network。 - **需要你做的**:无,席位落地。 ## What changes `createErrorFromResponse` read `status || statusCode || 500`. A `TypeError` from `fetch` (no answer at all) has no status, so it came back as `ObjectStackError` `UNKNOWN_ERROR` with `statusCode: 500`. `ObjectStackAdapter.getObjectSchema` rewraps every non-404 failure through it, and `answeredLoadFailure` reads any numeric status as the server's answer. So every reader of that wrapper showed the browser's "Failed to fetch" as the server's sentence, where it should have said "check your connection". - **`errors.ts`**: a status-less input now comes back with `statusCode` undefined (and `details.response.status` undefined). It is still an `ObjectStackError` with code `UNKNOWN_ERROR`, a shape `classifyLoadError` already reads as `network` and `answeredLoadFailure` as no answer, so neither reader changes. An input that is an `Error` is kept as `details.originalError`, whatever its status (a plain response object is not, so a circular axios response never lands in `toJSON`). Answered statuses keep today's mapping: 400, 401, 403, 404, 503, 504 and any other status keep their class, code and `statusCode`. `statusCode` was already declared optional, so no exported type changes. - **`index.ts` (comment correction, no code)**: #29728's comment on the `getObjectSchema` rewrap said `createErrorFromResponse` "stamps `500`" on a status-less error. It now says why the cause is still needed: the wrapper replaces the server's code and sentence with its own. - `MetadataProvider`'s `details.originalError` unwrap (#29728) is kept, and is not redundant: an answered 503's wrapper still reads `CONNECTION_ERROR` "Connection error: ...", so the server's own code and sentence live only on the cause. ## Tests - `errors.test.ts`: the "response without status code" pin moved from `500` to undefined (it pinned the defect). A new describe pins a fetch `TypeError`: no status, `UNKNOWN_ERROR`, not a `ConnectionError`, message kept, the input as `details.originalError`, and `toJSON().statusCode` undefined. Its CONTROLs are an answered 500 and an answered 503 that keep their status, class and cause, plus a plain response object that carries no cause. - `error-message.adapter-unanswered-29885.test.ts` (new, in `@object-ui/react`): the REAL `ObjectStackAdapter.getObjectSchema` over a fake `fetch` that rejects with a `TypeError`. The result has no `statusCode`, `answeredLoadFailure` is `null`, and `classifyLoadError` is `network`. CONTROLs: an answered 500 and an answered 503 keep their status and read as answered. This file sits in `@object-ui/react`, outside the card's named surface, because the adapter cannot import that reader (react depends on data-objectstack). `error-message.normalisation-boundary.test.ts` is the precedent. The seat accepted the deviation. - `getObjectSchema.test.ts`: **moved pin.** #29728's network-drop case asserted `statusCode` 500, and it now asserts undefined. Class, code, message and cause are unchanged. <details><summary>Ablation (committed head ee241ae, ablation-replace.mjs, restored blob == HEAD)</summary> - Put `|| 500` back in `createErrorFromResponse`: 3 tests went red. Those were the moved "without status code" pin, the `TypeError` pin in `errors.test.ts`, and the react boundary pin. All 40 other tests stayed green, including every CONTROL. The restored blob was `304622c6` == HEAD, with `git diff HEAD` empty. </details> <details><summary>Verification (final head 8923ea2)</summary> - `pnpm exec vitest run packages/data-objectstack/ packages/react/src/utils/ packages/app-shell/src/console/__tests__/AppContent.disabledPackageDeepLink-13052.test.tsx`: 123 files and 1418 tests passed. The last file is #29728's real-adapter route harness, and its network-drop CONTROL still draws the network sentence. - Consumer sweep: all 57 test files outside data-objectstack that build a real `ObjectStackAdapter` (app-shell and console). 2 batches, 29 files / 291 tests and 28 files / 161 tests, all green. This ran at ee241ae, before the main merge, and the touched source has not changed since apart from the comment. - `turbo run type-check --filter=@object-ui/data-objectstack --filter=@object-ui/react`: 43/43 tasks. - Derived gates (`dispatch-gates --commands`, 36 at 8923ea2): all exit 0, including `pnpm --dir ui exec vitest run scripts/` (170 files passed, 2 skipped). </details> ## Acceptance notes - **Census of readers of a 500 / >=500 status.** `RecordDetailView`'s `feedReadVerdictOf` treats undefined and >=500 the same way (`failed`). `plugin-chatbot` `usePendingActions` and mobile `createOfflineDataSource` read `.status`, not `statusCode`. None changes, and none depends on the 500. Every other reader goes through `classifyLoadError` / `answeredLoadFailure`, and those now read a network drop as no answer, as intended. - **Out of scope (ruling):** the wrapper still replaces the server's ADR-0112 `code` with its own taxonomy code (`CONNECTION_ERROR`, `UNKNOWN_ERROR`). Carrying the server's code beside it is not done here, and readers get it from `details.originalError`. - **Finding, handed to the seat:** `ConnectionError`'s constructor stamps `statusCode || 503`. The adapter builds one with no status on two paths where nothing answered (`connect()` and `noteServerUnanswered`), so those errors read as an answered 503. `connectRetriesDiscovery-12318.test` pins that 503, so changing it is a caller change, and it is left as is. --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jTkV5mfMT5LzJyxGdX)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent cfd237f commit 0b5b176

6 files changed

Lines changed: 161 additions & 13 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/data-objectstack': patch
3+
---
4+
5+
Clause-②: no
6+
Metadata-protocol: none
7+
8+
A request that got no answer (a dropped connection, an unreachable host) no longer reads as a server that answered with a 500 (#29885).
9+
10+
- `createErrorFromResponse` now leaves `statusCode` undefined when its input carries no status. Before, it stamped `500`, and `ObjectStackAdapter.getObjectSchema` rewraps through it, so every reader of a schema-read failure took the browser's "Failed to fetch" for the server's own answer. The error is still an `ObjectStackError` with code `UNKNOWN_ERROR`. `classifyLoadError` reads it as `network`, and `answeredLoadFailure` returns `null`, so a failed-read state keeps its "check your connection" sentence.
11+
- An input that is an `Error` is kept as `details.originalError`, whatever its status.
12+
- Answered statuses map as before: 400, 401, 403, 404, 503, 504 and any other status keep their class, code and `statusCode`.

‎ui/packages/data-objectstack/src/errors.test.ts‎

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,57 @@ describe('Error Helpers', () => {
353353
const error = createErrorFromResponse(response);
354354

355355
expect(error).toBeInstanceOf(ObjectStackError);
356-
expect(error.statusCode).toBe(500);
356+
expect(error.code).toBe('UNKNOWN_ERROR');
357+
// objectstack#29885 — no status in, no status out: an invented 500
358+
// reads as an answer from the server.
359+
expect(error.statusCode).toBeUndefined();
360+
expect((error.details as any).response.status).toBeUndefined();
361+
});
362+
363+
// objectstack#29885 — a request that got no answer (fetch rejects with a
364+
// bare TypeError) is not an answered server error.
365+
describe('an error that got no answer (objectstack#29885)', () => {
366+
it('a fetch TypeError keeps no status, and is kept as the cause', () => {
367+
const drop = new TypeError('Failed to fetch');
368+
369+
const error = createErrorFromResponse(drop as unknown as Record<string, unknown>, 'getObjectSchema(account)');
370+
371+
expect(error).toBeInstanceOf(ObjectStackError);
372+
expect(error).not.toBeInstanceOf(ConnectionError);
373+
expect(error.code).toBe('UNKNOWN_ERROR');
374+
expect(error.statusCode).toBeUndefined();
375+
expect(error.message).toBe('Failed to fetch');
376+
expect(error.details?.originalError).toBe(drop);
377+
expect(error.toJSON().statusCode).toBeUndefined();
378+
});
379+
380+
it('CONTROL: an answered 500 keeps its status, and its cause', () => {
381+
const answered = Object.assign(new Error('Database connection refused.'), { status: 500, code: 'INTERNAL_ERROR' });
382+
383+
const error = createErrorFromResponse(answered as unknown as Record<string, unknown>);
384+
385+
expect(error.code).toBe('UNKNOWN_ERROR');
386+
expect(error.statusCode).toBe(500);
387+
expect(error.details?.originalError).toBe(answered);
388+
});
389+
390+
it('CONTROL: an answered 503 is the ConnectionError it was, with its status', () => {
391+
const answered = Object.assign(new Error('The metadata store is down.'), { status: 503 });
392+
393+
const error = createErrorFromResponse(answered as unknown as Record<string, unknown>);
394+
395+
expect(error).toBeInstanceOf(ConnectionError);
396+
expect(error.code).toBe('CONNECTION_ERROR');
397+
expect(error.statusCode).toBe(503);
398+
expect(error.details?.originalError).toBe(answered);
399+
});
400+
401+
it('CONTROL: a plain response object (not an Error) carries no cause', () => {
402+
const error = createErrorFromResponse({ status: 500, message: 'Server error' });
403+
404+
expect(error.statusCode).toBe(500);
405+
expect(error.details).not.toHaveProperty('originalError');
406+
});
357407
});
358408

359409
it('should include context in error details', () => {

‎ui/packages/data-objectstack/src/errors.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,13 +217,21 @@ export class DataApiValidationError extends ObjectStackError {
217217

218218
/**
219219
* Helper function to create an error from an HTTP response
220-
*
220+
*
221+
* An input that carries no status got no answer (a dropped connection, an
222+
* unreachable host: `fetch` rejects with a bare `TypeError`). It comes back as
223+
* an `UNKNOWN_ERROR` with `statusCode` left undefined, never with an invented
224+
* status: a reader that sees a numeric status takes it for the server's answer
225+
* (`answeredLoadFailure`), and would show the browser's "Failed to fetch" as
226+
* if the server had said it (objectstack#29885). An input that is an `Error`
227+
* is kept as `details.originalError`, whatever its status.
228+
*
221229
* @param response - Response object or error from fetch/axios
222230
* @param context - Additional context for debugging
223231
* @returns Appropriate error instance
224232
*/
225233
export function createErrorFromResponse(response: Record<string, unknown>, context?: string): ObjectStackError {
226-
const status = (response?.status as number) || (response?.statusCode as number) || 500;
234+
const status = (response?.status as number) || (response?.statusCode as number) || undefined;
227235
const message = (response?.message as string) || (response?.statusText as string) || 'Unknown error';
228236
const details = {
229237
context,
@@ -232,6 +240,9 @@ export function createErrorFromResponse(response: Record<string, unknown>, conte
232240
data: response?.data,
233241
headers: response?.headers,
234242
},
243+
// The transport's own error, when the input is one: what the server said
244+
// (its status, ADR-0112 code and sentence), or the bare fetch failure.
245+
...(response instanceof Error ? { originalError: response } : {}),
235246
};
236247

237248
switch (status) {

‎ui/packages/data-objectstack/src/getObjectSchema.test.ts‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,10 @@ describe('ObjectStackAdapter.getObjectSchema', () => {
150150
/**
151151
* objectstack#29728 — a failed by-name read that is not a 404 is rewrapped by
152152
* `createErrorFromResponse`, which replaces the server's ADR-0112 code with its
153-
* own taxonomy and stamps `500` on a request that got no answer. The rewrap
154-
* keeps the transport's own error as `details.originalError` (as the 404 arm's
155-
* `MetadataNotFoundError` does), so a reader can tell whether, and what, the
156-
* server answered. The wrapper's class, code, statusCode and message are
157-
* unchanged.
153+
* own taxonomy. The rewrap keeps the transport's own error as
154+
* `details.originalError` (as the 404 arm's `MetadataNotFoundError` does), so
155+
* a reader can tell what the server answered. Since objectstack#29885 a
156+
* request that got no answer carries no `statusCode` on the wrapper either.
158157
*/
159158
describe('ObjectStackAdapter.getObjectSchema keeps the server answer as the cause of a non-404 failure (objectstack#29728)', () => {
160159
beforeEach(() => clearSharedDiscoveryCache());
@@ -196,15 +195,15 @@ describe('ObjectStackAdapter.getObjectSchema keeps the server answer as the caus
196195
expect(err.details.originalError).toMatchObject({ status: 500, code: 'INTERNAL_ERROR', message: 'Database connection refused.' });
197196
});
198197

199-
it('no answer: the wrapper still reads 500, and its cause is the bare fetch failure, with no status', async () => {
198+
it('no answer: the wrapper carries no status (objectstack#29885), and its cause is the bare fetch failure', async () => {
200199
const drop = new TypeError('Failed to fetch');
201200
const adapter = adapterAnswering(() => {
202201
throw drop;
203202
});
204203
const err: any = await adapter.getObjectSchema('account').catch((e: unknown) => e);
205204
expect(err).toBeInstanceOf(ObjectStackError);
206205
expect(err.code).toBe('UNKNOWN_ERROR');
207-
expect(err.statusCode).toBe(500);
206+
expect(err.statusCode).toBeUndefined();
208207
expect(err.message).toBe('Failed to fetch');
209208
expect(err.details.originalError).toBe(drop);
210209
});

‎ui/packages/data-objectstack/src/index.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5717,9 +5717,9 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
57175717
// objectstack#29728 — the rewrap keeps the transport's own error as its
57185718
// cause, as the 404 arm above does. `createErrorFromResponse` replaces
57195719
// the server's ADR-0112 code with its own taxonomy (`CONNECTION_ERROR`,
5720-
// `UNKNOWN_ERROR`), and it stamps `500` on an error that carries no
5721-
// status at all (a dropped connection), so the wrapper alone cannot say
5722-
// whether the server answered or in what words. The cause can: the
5720+
// `UNKNOWN_ERROR`) and its sentence with its own ("Connection error:
5721+
// ..."), so the wrapper alone cannot say what the server answered (a
5722+
// dropped connection carries no status, objectstack#29885). The cause can: the
57235723
// by-name route read throws `status` + the server's `code` and sentence
57245724
// for an answer, and the bare fetch failure for none.
57255725
const wrapped = createErrorFromResponse(errorObj, `getObjectSchema(${objectName})`);
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
import { beforeEach, describe, expect, it, vi } from 'vitest';
10+
11+
import { ObjectStackAdapter, clearSharedDiscoveryCache } from '@object-ui/data-objectstack';
12+
import { answeredLoadFailure, classifyLoadError } from './error-message';
13+
14+
/**
15+
* objectstack#29885 — what the adapter throws for a request that got no
16+
* answer, read by the shared load-failure rule, pinned where the two meet.
17+
*
18+
* `ObjectStackAdapter.getObjectSchema` rewraps a non-404 failure through
19+
* `createErrorFromResponse`, which used to stamp `statusCode: 500` on an error
20+
* that carried no status (a dropped connection: `fetch` rejects with a bare
21+
* `TypeError`). `answeredLoadFailure` reads any numeric status as the server's
22+
* answer, so every reader of that wrapper showed the browser's "Failed to
23+
* fetch" as if the server had said it, instead of its "check your connection"
24+
* sentence. The adapter cannot import this reader (this package depends on
25+
* that one), so neither side's unit tests can pin the pair alone.
26+
*/
27+
describe('a getObjectSchema read that got no answer reads as no answer (objectstack#29885)', () => {
28+
beforeEach(() => clearSharedDiscoveryCache());
29+
30+
function adapterAnswering(byName: () => unknown) {
31+
const fetchImpl = vi.fn(async (url: unknown) => {
32+
const u = String(url);
33+
if (u.includes('/api/v1/discovery')) {
34+
return { ok: true, status: 200, statusText: 'OK', json: async () => ({ success: true, data: { version: 'v1', routes: {} } }) };
35+
}
36+
if (u.includes('/api/v1/meta/object/')) return byName();
37+
return { ok: true, status: 200, statusText: 'OK', json: async () => ({}) };
38+
});
39+
return new ObjectStackAdapter({ baseUrl: 'http://unanswered-29885.test', autoReconnect: false, fetch: fetchImpl as any });
40+
}
41+
42+
const answered = (status: number, body: unknown) => () => ({ ok: false, status, statusText: 'Error', json: async () => body });
43+
44+
it('a fetch that rejects with a TypeError: no answered failure, and a network kind', async () => {
45+
const adapter = adapterAnswering(() => {
46+
throw new TypeError('Failed to fetch');
47+
});
48+
49+
const err: any = await adapter.getObjectSchema('account').catch((e: unknown) => e);
50+
51+
expect(err).toBeInstanceOf(Error);
52+
expect(err.statusCode).toBeUndefined();
53+
expect(answeredLoadFailure(err)).toBeNull();
54+
expect(classifyLoadError(err)).toBe('network');
55+
});
56+
57+
it('CONTROL: an answered 500 keeps its status and reads as answered', async () => {
58+
const adapter = adapterAnswering(answered(500, { error: 'Database connection refused.', code: 'INTERNAL_ERROR' }));
59+
60+
const err: any = await adapter.getObjectSchema('account').catch((e: unknown) => e);
61+
62+
expect(err.statusCode).toBe(500);
63+
expect(answeredLoadFailure(err)).toMatchObject({ message: 'Database connection refused.' });
64+
});
65+
66+
it('CONTROL: an answered 503 keeps its status and reads as answered', async () => {
67+
const adapter = adapterAnswering(
68+
answered(503, { success: false, error: { code: 'SERVICE_UNAVAILABLE', message: 'The metadata store is down.' } }),
69+
);
70+
71+
const err: any = await adapter.getObjectSchema('account').catch((e: unknown) => e);
72+
73+
expect(err.statusCode).toBe(503);
74+
expect(answeredLoadFailure(err)).not.toBeNull();
75+
});
76+
});

0 commit comments

Comments
 (0)