Skip to content

Commit 83fc096

Browse files
fix(data-objectstack): a ConnectionError nothing answered no longer carries an invented 503 (#29947)
<!-- claude-projects-attribution: {"github_login":"os-support-ai"} --> _Requested by **Yibo Wang** · [project thread](https://claude.ai/code/project/chan_01BVUyyPie8gHwoPM18iABwB?thread=cmsg_01BVUyyPie8gHwoPM18iABwB4xjiquevJqfBXiUoqfQwa1)_ Fixes #29916 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:后端连不上时(主机不可达、请求被浏览器直接拒绝),适配器的 `ConnectionError` 不再带一个编造的 503;服务器真正回答过的 503/504 照旧带状态码,发现接口被服务器以错误状态拒绝时带的是服务器的真实状态。 - **为什么改**:带着 503 的断网错误被列表页、记录页等读取方当成「服务器已回答」,于是把浏览器的 "Failed to fetch" 当成服务器原话展示,而不是提示用户检查网络;#29915 修了 `createErrorFromResponse` 的同类问题,这是剩下的那个来源。 - **风险与回滚**:风险低。只在没传状态码时行为变化(`statusCode` 由 503 变为未定义,错误类与 code 不变);仓内没有任何读取方按 503 分支;回滚即还原本 PR。 - **先看**:`ui/packages/data-objectstack/src/errors.ts`(构造器去掉 `|| 503`)、`ui/packages/data-objectstack/src/index.ts`(`connect()` 的 catch 与发现接口 `!res.ok` 分支)、`ui/packages/react/src/utils/error-message.adapter-connection-unanswered-29916.test.ts`(真实适配器 + 共享读取规则的边界钉)。 - **两分钟验证**:在仓根运行 `pnpm --dir ui exec vitest run packages/react/src/utils/error-message.adapter-connection-unanswered-29916.test.ts packages/data-objectstack/src/connectRetriesDiscovery-12318.test.ts`,预期 9 个测试全部通过,其中断网用例读作 `network`,已回答的 503 用例读作已回答。 - **需要你做的**:无,席位落地。 ## What changes The `ConnectionError` constructor no longer invents a status. It stamped `statusCode || 503`; it now passes `statusCode` through, so an error built without one keeps `statusCode` undefined. Same shape as #29915 for `createErrorFromResponse`. I moved the default in the constructor, not at the two callers. The constructor's only status input is an optional number, so a caller could not say "no answer" to it. Every caller that omits a status is a no-answer path. Every `new ConnectionError(` caller, and what it carries now: | Caller | Path | Before | After | |---|---|---|---| | `index.ts` `connect()` catch, discovery `fetch` rejected | nothing answered | 503 (invented) | none | | `index.ts` `connect()` catch, discovery answered `!res.ok` | server answered | 503 (invented, whatever the server said) | the server's status (`res.status`) | | `index.ts` `noteServerUnanswered` (post-connect `error` state) | nothing answered | 503 (invented) | none | | `errors.ts` `createErrorFromResponse` case 503 | server answered | 503 | 503 (unchanged) | | `errors.ts` `createErrorFromResponse` case 504 | server answered | 504 | 504 (unchanged) | Why the second row matters: the `connect()` catch covers both a rejected `fetch` and a discovery the server refused. Dropping the default alone would have flipped an answered discovery failure from "answered (wrong status)" to "unanswered". So the `!res.ok` throw now puts `status: res.status` on its error, and the catch hands a numeric `status` to the constructor. A discovery 401 now reads as `unauthorized`. Every data method opens with `await this.connect()`. So a list or record read against an unreachable backend rejects with the `connect()` error, and `answeredLoadFailure` / `classifyLoadError` read it. After this PR that read returns `null` and classifies as `network`. **Exported contract (the PM's mechanism assumption, measured):** `ConnectionError` and `ObjectStackError.statusCode?: number` are exported. The TypeScript type is unchanged, since `statusCode` was already optional. The runtime value changes for `new ConnectionError(msg)` with no status: `503` becomes `undefined`. Docs that describe it: - `ui/packages/data-objectstack/README.md` L582 calls it "Network/connection errors (503/504)". That still describes the answered 503/504 cases, but it does not say a no-answer error carries no status. - `content/docs/frontend/data-objectstack.mdx` L497 shows "`code 'CONNECTION_ERROR', statusCode 503`" for a `getObjectSchema` failure. That path only builds a `ConnectionError` from an answered 503/504 (no-answer goes through `createErrorFromResponse` to `UNKNOWN_ERROR` since #29915), so it stays true. I left both docs as they are, because neither is in the claimed file surface. A wording follow-up for the README line is in the report. ## Tests - New `ui/packages/react/src/utils/error-message.adapter-connection-unanswered-29916.test.ts`. It sits next to #29915's boundary pin, because the adapter cannot import `answeredLoadFailure` (react depends on data-objectstack). It drives the real `ObjectStackAdapter` over a fake `fetch`: - `connect()` over a rejecting `fetch`: `ConnectionError`, `statusCode` undefined, `answeredLoadFailure` null, `classifyLoadError` `network`. - `find()` before connect with discovery unreachable: same. - after a good connect, an unanswered `find()` trips `noteServerUnanswered`: the `error` state event's `ConnectionError` has no status, null, `network`. - CONTROL: `createErrorFromResponse({ status: 503 })` keeps 503 and reads as answered. - CONTROL: a discovery the server answered with 503 keeps 503 and reads as answered. - a discovery answered 401 carries 401 and classifies `unauthorized`. - **Moved pin:** `connectRetriesDiscovery-12318.test.ts`, "refused past the budget". It asserted `statusCode: 503` on a discovery that was refused and never answered. It now asserts `statusCode` undefined. - `errors.test.ts`: the bare-constructor pin moves from 503 to undefined. A new CONTROL checks that a passed 503 / 504 is kept. ## Acceptance notes - A test file outside the claimed surface: the new pin lives in `ui/packages/react/src/utils/`, not in data-objectstack. This is #29915's precedent, for the reason above. No other file outside the claim is touched. - No consumer in `ui/` branches on a `ConnectionError`'s 503. I grepped `statusCode === 503`, `status === 503` and `CONNECTION_ERROR` outside tests. The two 503 hits (`metadata-admin/external/api.ts`, `preview/commitHistory.ts`) read raw `fetch` responses, not this class. ## Verification All of it ran on head `7aaf1de9`. - `pnpm --dir ui exec vitest run packages/data-objectstack/ packages/react/src/utils/` (under the verify lock): VERDICT command-exit 0, 123 files and 1407 tests passed. - Consumer sweep: all 60 test files outside data-objectstack that build a real `ObjectStackAdapter` (app-shell, console and others, found with `grep -rl "new ObjectStackAdapter|createObjectStackAdapter"`). VERDICT 0, 60 files and 487 tests passed. - `cd ui && pnpm exec turbo run build type-check --filter=@object-ui/data-objectstack --filter=@object-ui/react`: VERDICT 0, 44/44 tasks. `@object-ui/data-objectstack:type-check` and `@object-ui/react:type-check` are both in the output. - `node scripts/check-sdui-manifest.mjs`: exit 0, intact, 106 components. - Ablation, on committed head `7aaf1de9` with `scripts/ablation-replace.mjs`. Vitest aliases `@object-ui/data-objectstack` to `src`, so no dist rebuild was needed. - I put `statusCode || 503` back in the constructor. 5 tests went red: the moved 12318 pin, the `errors.test.ts` bare-constructor pin, and the three no-answer pins in the new file. The other 1402 stayed green, including every CONTROL. On restore, the blob was `35dfadc2` == HEAD and `git diff HEAD` was empty. - I dropped the discovery status hand-off (passed `undefined`). 2 tests went red: the answered-503 discovery CONTROL and the 401 pin. The other 4 stayed green. On restore, the blob was `2098bca3` == HEAD. <details><summary>Derived gates: dispatch-gates --commands, 36 at 7aaf1de, all exit 0; --ran: 36 derived, 36 run, 0 NOT-MEASURED</summary> ```text node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0 node scripts/check-adr-0087-registration.mjs --self-test :: exit 0 node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0 node scripts/check-closing-keyword-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0 node scripts/check-comment-mask-corpus.mjs :: exit 0 node scripts/check-empty-changeset.mjs --base origin/main :: exit 0 node scripts/check-empty-changeset.mjs --self-test :: exit 0 node scripts/check-sdui-manifest.mjs :: exit 0 node scripts/check-sdui-manifest.mjs --self-test :: exit 0 node scripts/pm/release-rehearsal-clone.mjs --self-test :: exit 0 node scripts/release-pending-publish.mjs --self-test :: exit 0 pnpm --dir ui exec vitest run scripts/ :: exit 0 pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0 pnpm check:changeset-gate-self-tests :: exit 0 pnpm check:closed-ceremony-text :: exit 0 pnpm check:cross-package-test-inputs :: exit 0 pnpm check:dispatcher-error-vocabulary :: exit 0 pnpm check:driver-memory-census :: exit 0 pnpm check:engine-double-contract :: exit 0 pnpm check:error-status-conformance :: exit 0 pnpm check:gitlink-declared :: exit 0 pnpm check:nul-bytes :: exit 0 pnpm check:published-files :: exit 0 pnpm check:query-options-erasure :: exit 0 pnpm check:refd-timer-probe :: exit 0 pnpm check:registry-log-declared :: exit 0 pnpm check:rest-log-spy-declared :: exit 0 pnpm check:skill-identifier-liveness :: exit 0 pnpm check:test-source-alias :: exit 0 pnpm check:tier-file-adoption :: exit 0 pnpm check:type-check-coverage :: exit 0 pnpm check:type-check-debt :: exit 0 pnpm check:undeclared-dep-imports :: exit 0 pnpm check:watch-hint-literal :: exit 0 pnpm check:where-matcher :: exit 0 ``` `pnpm --dir ui exec vitest run scripts/`: 170 files passed, 2 skipped (4979 tests). </details> ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (not touched) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. (CI runs it) - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: its generated artifacts are regenerated and committed. (not touched) - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. (except the 维护者速读 block the dispatch contract requires) - [x] The *Before Submitting* list in CONTRIBUTING.md is done. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01WGufzfeRhCDTUFg4Reps3f Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1f6bc5a commit 83fc096

6 files changed

Lines changed: 173 additions & 5 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 `ConnectionError` for a request nothing answered (an unreachable host, a rejected `fetch`) no longer reads as a server that answered with a 503 (#29916).
9+
10+
- The `ConnectionError` constructor leaves `statusCode` undefined when the caller passes none. Before, it stamped `503`. `connect()` builds it that way when discovery's `fetch` rejects, every data method opens with `await this.connect()`, and the post-connect `error` connection state builds it the same way, so a read against a down backend was taken for the server's own answer. `answeredLoadFailure` now returns `null` for it and `classifyLoadError` reads `network`, so a failed-read state keeps its "check your connection" sentence.
11+
- A discovery read the server answered with an error status (`!res.ok`) now carries that status (a 401 reads as `unauthorized`), instead of the invented 503.
12+
- An answered 503 or 504 through `createErrorFromResponse` keeps its status, as before.

‎ui/packages/data-objectstack/src/connectRetriesDiscovery-12318.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,10 @@ describe('a failed first connect() retries discovery as a new attempt (objectui#
156156
expect(fetch).toHaveBeenCalledTimes(3);
157157
expect(outcome.settled).toBe('rejected');
158158
expect(outcome.error).toBeInstanceOf(ConnectionError);
159-
expect(outcome.error).toMatchObject({ code: 'CONNECTION_ERROR', statusCode: 503 });
159+
expect(outcome.error).toMatchObject({ code: 'CONNECTION_ERROR' });
160+
// Discovery was refused, never answered: the error carries no status
161+
// (objectstack#29916; it used to carry an invented 503).
162+
expect((outcome.error as ConnectionError).statusCode).toBeUndefined();
160163
expect(adapter.getConnectionState()).toBe('error');
161164
expect(adapter.isConnected()).toBe(false);
162165
expect(states).toEqual([

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,9 @@ describe('Error Classes', () => {
149149

150150
expect(error.message).toBe('Connection error: Network timeout');
151151
expect(error.code).toBe('CONNECTION_ERROR');
152-
expect(error.statusCode).toBe(503);
152+
// Built without a status, nothing answered: no invented 503
153+
// (objectstack#29916).
154+
expect(error.statusCode).toBeUndefined();
153155
expect(error.name).toBe('ConnectionError');
154156
expect(error.url).toBe('https://api.example.com');
155157
expect(error).toBeInstanceOf(ObjectStackError);
@@ -162,6 +164,11 @@ describe('Error Classes', () => {
162164
expect(error.url).toBeUndefined();
163165
expect(error.message).toBe('Connection error: Network timeout');
164166
});
167+
168+
it('CONTROL: a status the caller passes is kept', () => {
169+
expect(new ConnectionError('down', undefined, undefined, 503).statusCode).toBe(503);
170+
expect(new ConnectionError('timeout', undefined, undefined, 504).statusCode).toBe(504);
171+
});
165172
});
166173

167174
describe('AuthenticationError', () => {

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,16 @@ export class BulkOperationError extends ObjectStackError {
121121

122122
/**
123123
* Error thrown when connection to ObjectStack server fails
124+
*
125+
* `statusCode` is the server's answer and nothing else: a 503 / 504 the server
126+
* sent (`createErrorFromResponse`), or the status a refused discovery read
127+
* came back with (`connect()`). Built without one, the error stands for a
128+
* request nothing answered (the host unreachable, `fetch` rejected), and
129+
* `statusCode` stays undefined rather than an invented 503: a reader that sees
130+
* a numeric status takes it for the server's answer (`answeredLoadFailure`)
131+
* and would show "Failed to fetch" as the server's own words
132+
* (objectstack#29916, the same rule objectstack#29885 set for
133+
* `createErrorFromResponse`).
124134
*/
125135
export class ConnectionError extends ObjectStackError {
126136
constructor(
@@ -132,7 +142,7 @@ export class ConnectionError extends ObjectStackError {
132142
super(
133143
`Connection error: ${message}`,
134144
'CONNECTION_ERROR',
135-
statusCode || 503,
145+
statusCode,
136146
{ url, ...details }
137147
);
138148
this.name = 'ConnectionError';

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3613,7 +3613,12 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
36133613
: undefined,
36143614
});
36153615
if (!res.ok) {
3616-
throw new Error(`discovery ${res.status} ${res.statusText}`);
3616+
// The server answered: its status rides on the error, so the
3617+
// ConnectionError below carries the server's status rather than
3618+
// none (objectstack#29916).
3619+
throw Object.assign(new Error(`discovery ${res.status} ${res.statusText}`), {
3620+
status: res.status,
3621+
});
36173622
}
36183623
const body = await res.json();
36193624
return body && typeof body.success === 'boolean' && 'data' in body
@@ -3635,10 +3640,15 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
36353640
this.setConnectionState('connected');
36363641
} catch (error: unknown) {
36373642
const errorMessage = error instanceof Error ? error.message : 'Failed to connect to ObjectStack server';
3643+
// A status only when discovery was answered (the `!res.ok` throw above);
3644+
// a rejected `fetch` got no answer and the error carries no status
3645+
// (objectstack#29916).
3646+
const answeredStatus = (error as { status?: unknown } | null)?.status;
36383647
const connectionError = new ConnectionError(
36393648
errorMessage,
36403649
undefined,
3641-
{ originalError: error }
3650+
{ originalError: error },
3651+
typeof answeredStatus === 'number' ? answeredStatus : undefined,
36423652
);
36433653

36443654
this.setConnectionState('error', connectionError);
@@ -3788,6 +3798,7 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
37883798
if (this.connectionState !== 'reconnecting') this.setConnectionState('reconnecting');
37893799
return;
37903800
}
3801+
// No status: nothing answered this request (objectstack#29916).
37913802
this.setConnectionState(
37923803
'error',
37933804
new ConnectionError(
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
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 {
12+
ConnectionError,
13+
ObjectStackAdapter,
14+
clearSharedDiscoveryCache,
15+
createErrorFromResponse,
16+
type ConnectionStateEvent,
17+
} from '@object-ui/data-objectstack';
18+
import { answeredLoadFailure, classifyLoadError } from './error-message';
19+
20+
/**
21+
* objectstack#29916 — a `ConnectionError` for a request nothing answered,
22+
* read by the shared load-failure rule, pinned where the two meet.
23+
*
24+
* The `ConnectionError` constructor used to stamp `statusCode: 503` on every
25+
* error built without a status. `connect()`'s catch (a discovery `fetch` that
26+
* rejected) and the post-connect "the server did not answer" state both build
27+
* it that way, and every data method opens with `await this.connect()`, so a
28+
* list read against a down backend rejected with an error that looked
29+
* answered: `answeredLoadFailure` takes any numeric status for the server's
30+
* answer and showed "Connection error: Failed to fetch" as the server's own
31+
* words. Same defect as objectstack#29885 for `createErrorFromResponse`.
32+
*/
33+
describe('a ConnectionError that nothing answered reads as no answer (objectstack#29916)', () => {
34+
beforeEach(() => clearSharedDiscoveryCache());
35+
36+
const discoveryOk = () => ({
37+
ok: true,
38+
status: 200,
39+
statusText: 'OK',
40+
json: async () => ({ success: true, data: { version: 'v1', routes: {} } }),
41+
});
42+
43+
function adapterWith(discovery: () => unknown, data: () => unknown = () => { throw new TypeError('Failed to fetch'); }) {
44+
const fetchImpl = vi.fn(async (url: unknown) => {
45+
if (String(url).includes('/api/v1/discovery')) return discovery();
46+
return data();
47+
});
48+
return new ObjectStackAdapter({
49+
baseUrl: 'http://connection-unanswered-29916.test',
50+
autoReconnect: false,
51+
fetch: fetchImpl as any,
52+
});
53+
}
54+
55+
it('connect() over a fetch that rejects: a ConnectionError with no status, no answered failure, a network kind', async () => {
56+
const adapter = adapterWith(() => {
57+
throw new TypeError('Failed to fetch');
58+
});
59+
60+
const err: any = await adapter.connect().catch((e: unknown) => e);
61+
62+
expect(err).toBeInstanceOf(ConnectionError);
63+
expect(err.code).toBe('CONNECTION_ERROR');
64+
expect(err.statusCode).toBeUndefined();
65+
expect(answeredLoadFailure(err)).toBeNull();
66+
expect(classifyLoadError(err)).toBe('network');
67+
});
68+
69+
it('a data read before connect, discovery unreachable: the read rejects as no answer', async () => {
70+
const adapter = adapterWith(() => {
71+
throw new TypeError('Failed to fetch');
72+
});
73+
74+
const err: any = await adapter.find('account').catch((e: unknown) => e);
75+
76+
expect(err).toBeInstanceOf(ConnectionError);
77+
expect(err.statusCode).toBeUndefined();
78+
expect(answeredLoadFailure(err)).toBeNull();
79+
expect(classifyLoadError(err)).toBe('network');
80+
});
81+
82+
it('an unanswered request after connect: the error-state ConnectionError carries no status', async () => {
83+
const adapter = adapterWith(discoveryOk);
84+
await adapter.connect();
85+
const events: ConnectionStateEvent[] = [];
86+
adapter.onConnectionStateChange((event) => events.push(event));
87+
88+
await expect(adapter.find('account')).rejects.toBeInstanceOf(TypeError);
89+
90+
expect(events.map((e) => e.state)).toEqual(['error']);
91+
const err: any = events[0].error;
92+
expect(err).toBeInstanceOf(ConnectionError);
93+
expect(err.statusCode).toBeUndefined();
94+
expect(answeredLoadFailure(err)).toBeNull();
95+
expect(classifyLoadError(err)).toBe('network');
96+
});
97+
98+
it('CONTROL: an answered 503 through createErrorFromResponse keeps its status and reads as answered', () => {
99+
const err: any = createErrorFromResponse({ status: 503, message: 'The metadata store is down.' }, 'find(account)');
100+
101+
expect(err).toBeInstanceOf(ConnectionError);
102+
expect(err.statusCode).toBe(503);
103+
expect(answeredLoadFailure(err)).not.toBeNull();
104+
});
105+
106+
it('CONTROL: a discovery the server answered with 503 keeps that status and reads as answered', async () => {
107+
const adapter = adapterWith(() => ({ ok: false, status: 503, statusText: 'Service Unavailable', json: async () => ({}) }));
108+
109+
const err: any = await adapter.connect().catch((e: unknown) => e);
110+
111+
expect(err).toBeInstanceOf(ConnectionError);
112+
expect(err.statusCode).toBe(503);
113+
expect(answeredLoadFailure(err)).not.toBeNull();
114+
});
115+
116+
it('a discovery the server answered with 401 carries 401, not an invented 503', async () => {
117+
const adapter = adapterWith(() => ({ ok: false, status: 401, statusText: 'Unauthorized', json: async () => ({}) }));
118+
119+
const err: any = await adapter.connect().catch((e: unknown) => e);
120+
121+
expect(err).toBeInstanceOf(ConnectionError);
122+
expect(err.statusCode).toBe(401);
123+
expect(classifyLoadError(err)).toBe('unauthorized');
124+
});
125+
});

0 commit comments

Comments
 (0)