Skip to content

Commit 555eb1b

Browse files
fix(objectql): the record-currency backfill cleans a driver's raw text out of a failed row, as the import door does (#30867)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} --> _Requested by **Elon** · [project thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9DX44Uu3BZcP1g6vzpMCUtzN)_ domain:server#1 · os-elon-musk · session_016WV4d1GEfMtjCJfYePyAPD Fixes #30816 Clause-②: no Metadata-protocol: none Claim: comment 6678760289 (branch `claude/project-thread-x3bsoc`). ## 维护者速读 - 改了什么:记录币种回填(`POST /api/v1/meta/_migrate-record-currency-codes` 与 `os migrate record-currency-codes`)报告里失败行的错误文字,非沙箱钩子的报错现在先经导入入口同一个清洗函数 `sanitizeRowError` 再返回。 - 为什么改:回填报告自 #30809 起经 REST 返回给持 `setup.write` 的租户管理员,数据库驱动的原始报错(SQL 语句、约束名、列名)会原样外露;导入入口早已清洗同类报错。 - 风险与回滚:只改一行取值,沙箱钩子分支不动;不含驱动文字的报错照旧原样;运维日志行仍保留完整原文。单一提交可整体回滚。 - 先看:`packages/objectql/src/record-currency-backfill.ts`(`rowFailureSentence` 的最后一行)、`packages/rest/src/meta-record-currency-backfill-route.test.ts`(REST 钉)。 - 两分钟验证:`pnpm --filter @objectstack/objectql exec vitest run src/record-currency-backfill.test.ts`,预期 21/21 通过。 - 需要你做的:无,席位落地。 ## What changes - `rowFailureSentence` (`@objectstack/objectql`): a row whose write threw anything other than a sandboxed hook's error now reads `sanitizeRowError(message)`, the import row's rule from `@objectstack/core`, instead of the raw message. The sandboxed arm (the door's withheld sentence for a crash, the hook's own sentence for a refusal) is unchanged. The operator's `logger.warn` line still carries the whole text. - No new export and no new dependency edge: `sanitizeRowError` is already public (`packages/core/src/index.ts` re-exports `./utils/import-runner.js` with `export *`; `@objectstack/rest` tests already import it from `@objectstack/core`), and `@objectstack/objectql` already depends on `@objectstack/core`. So the card's premise that core does not export it does not hold, and `Clause-②` re-derives to `no` instead of the claim's expected `yes (widening)`. ⛔ No second copy of the sanitizer's rules. - The CLI report changes in the same way, because it prints the same `failures[].error` through `formatRecordCurrencyBackfillReport`. The operator loses nothing: the CLI routes `logger.warn` to its warning line (stderr under `--json`), which keeps the raw driver text. ## Why Since #30809 the backfill report is also the REST answer of `POST /meta/_migrate-record-currency-codes`, read by a tenant admin holding `setup.write`. A driver's error kept its SQL statement, constraint name and column list verbatim in that answer, while the import door cleans the same class. Contract review 6677255440, should-fix 1. ## Tests - `packages/objectql/src/record-currency-backfill.test.ts`: three driver-shaped throws (an `update ... - REASON` statement, a SQLite UNIQUE failure, an `insert ... - FOREIGN_KEY_REASON` statement) each read `sanitizeRowError(raw)` in the report and none of the driver's names; the printed report line reads the same sentence; the warning line keeps the raw text. The existing CONTROL (a plain message with no driver text) still reads as before. - `packages/rest/src/meta-record-currency-backfill-route.test.ts`: on the real `RestServer` route over a real `ObjectQL` engine, the driver throws a driver-shaped error for one row of the caller's organization; the answer is 200, the other row is written, and the failed row carries `sanitizeRowError(raw)` with no SQL, column or placeholder text. - Found while writing the REST pin: a unique violation never reaches `rowFailureSentence` raw, because the engine already rewrites it into its own duplicate-record sentence. The REST pin therefore uses a value-too-long driver error. <details><summary>Ablation (restored, blob equal to HEAD, git diff HEAD empty)</summary> - Mutation `?? sanitizeRowError(message);` to `?? (message || 'ABLATED_30816');` via `scripts/ablation-replace.mjs`, objectql rebuilt, `ablation-dist-preflight` found the marker in 2 built files (the REST suite reads objectql's `dist/`). - objectql: 1 failed / 20 passed (the driver-shaped case). REST: 1 failed / 12 passed (the new pin). - Restore leg: blob `5fd2c441` equals HEAD, `git diff HEAD` empty, objectql rebuilt, preflight `--absent` green, both files green again (21/21, 13/13). - The mutate leg's objectql build exited 1 after emitting JS (the marker reached dist); the restore build exited 0. </details> ## Acceptance notes - `sanitizeRowError` keeps the trailing reason of a `... - REASON` driver message when the reason is not itself SQL, so a reason such as `value too long for type character varying(3)` is still shown. That is the import door's existing reading, applied here unchanged. ## Verification All on head `9e4afd8b`: - `pnpm --filter @objectstack/objectql typecheck` and `test`: 558 files, 11160 tests passed. - `pnpm --filter @objectstack/rest typecheck` and `test`: 393 files, 7456 passed, 332 skipped. - `node scripts/pm/dispatch-gates.mjs --commands`: 61 families derived; `--ran` reconciliation: 60 run with exit 0, 1 NOT MEASURED (`pnpm check:dual-build-cjs-loads`, exit 3, PREREQUISITE NOT MET: the whole tree has no `dist/`; CI runs it on a full build). - `pnpm lint`: exit 0. ## 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. - [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: 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. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. --- _Generated by [Claude Code](https://claude.ai/code/session_016WV4d1GEfMtjCJfYePyAPD)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3e04799 commit 555eb1b

4 files changed

Lines changed: 61 additions & 5 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/objectql': patch
3+
---
4+
5+
fix(objectql): the record-currency backfill cleans a driver's raw text out of a failed row, as the import door does
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
A failed row in the record-currency backfill report (`failures[].error`) now reads the import row's cleaned sentence (`sanitizeRowError` from `@objectstack/core`, the one rule both doors call) when the row's write threw anything other than a sandboxed hook's error. A driver's SQL statement, constraint name or column list no longer reaches the report, which a tenant admin reads over `POST /api/v1/meta/_migrate-record-currency-codes` and the operator reads in `os migrate record-currency-codes`. A message with no driver text reads as before. The operator's warning line keeps the whole text, so nothing is lost for diagnosis.

‎packages/objectql/src/record-currency-backfill.test.ts‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// guard and the code field's `valueDomain` judge each one.
1414

1515
import { describe, it, expect, beforeEach, vi } from 'vitest';
16+
import { sanitizeRowError } from '@objectstack/core';
1617
import { ObjectQL } from './engine.js';
1718
import {
1819
backfillRecordCurrencyCodes,
@@ -484,8 +485,28 @@ describe('a failed row reads a sandboxed hook as the write doors do', () => {
484485
}
485486
});
486487

487-
it('CONTROL: any other throw keeps its message, as before', async () => {
488+
it('CONTROL: a throw with no driver text keeps its message, as before', async () => {
488489
const { report } = await run(() => new Error('driver refused this row'));
489490
expect(report.failures).toEqual([{ recordId: 'r1', error: 'driver refused this row' }]);
490491
});
492+
493+
/**
494+
* [#30816] A driver's own throw carries its SQL, constraint and column names.
495+
* The row reads the import row's cleaned sentence (`sanitizeRowError`, the
496+
* one rule both doors call), the CLI's printed report reads the same, and
497+
* the operator's log line keeps the whole text.
498+
*/
499+
it('a driver-shaped throw: the import row\'s cleaned sentence, the raw text only in the log', async () => {
500+
for (const raw of [
501+
`update "deal" set "currency_code" = $1 where "id" = $2 - value too long for type character varying(3)`,
502+
'SQLITE_CONSTRAINT: UNIQUE constraint failed: deal.currency_code',
503+
`insert into "deal" ("id") values ($1) - insert or update on table "deal" violates foreign key constraint "deal_owner_fkey"`,
504+
]) {
505+
const { report, warned } = await run(() => new Error(raw));
506+
expect(report.failures, raw).toEqual([{ recordId: 'r1', error: sanitizeRowError(raw) }]);
507+
expect(report.failures[0]!.error, raw).not.toMatch(/"deal"|\$1|currency_code|_fkey/);
508+
expect(formatRecordCurrencyBackfillReport(report).join('\n'), raw).toContain(` • r1: ${sanitizeRowError(raw)}`);
509+
expect(warned.join('\n'), raw).toContain(raw);
510+
}
511+
});
491512
});

‎packages/objectql/src/record-currency-backfill.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@
8787
*/
8888

8989
import { isSandboxOrigin, isSandboxOwnFault, keysetWalk, mapDataError, sandboxBusinessMessage, type KeysetPageQuery } from '@objectstack/types';
90-
import { objectNotFoundError, withTransientRetry, type RetryOptions } from '@objectstack/core';
90+
import { objectNotFoundError, sanitizeRowError, withTransientRetry, type RetryOptions } from '@objectstack/core';
9191
import { isValueDomainMember } from '@objectstack/spec/shared';
9292

9393
import { isRecordCurrencyBlank, recordCurrencyFieldOf } from './validation/record-currency-code.js';
@@ -449,15 +449,18 @@ async function walkCodelessRows(
449449
* error the `/data` door answers with a 5xx (a crash, a declared 5xx, the
450450
* sandbox's own fault) takes that door's withheld sentence, and a sandboxed
451451
* refusal takes the hook's own sentence, never the `hook '<name>' threw:`
452-
* debug wrapper. Any other throw keeps its message, as before. The operator's
453-
* log line above keeps the whole text.
452+
* debug wrapper. [#30816] Any other throw is cleaned by the import row's own
453+
* sanitizer (`sanitizeRowError`, `@objectstack/core`), so a driver's raw text
454+
* (SQL, constraint names, column lists) never reaches the report; a message
455+
* with no driver text reads as before. The operator's log line above keeps
456+
* the whole text.
454457
*/
455458
function rowFailureSentence(err: unknown, object: string, message: string): string {
456459
if (isSandboxOrigin(err) || isSandboxOwnFault(err)) {
457460
const door = mapDataError(err, object);
458461
if (door.status >= 500) return String(door.body.error);
459462
}
460-
return sandboxBusinessMessage(err) ?? message;
463+
return sandboxBusinessMessage(err) ?? sanitizeRowError(message);
461464
}
462465

463466
/** Human-readable report body, shared by the CLI's dry-run and apply output. */

‎packages/rest/src/meta-record-currency-backfill-route.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
*/
1212

1313
import { describe, it, expect, beforeEach, vi } from 'vitest';
14+
import { sanitizeRowError } from '@objectstack/core';
1415
import { ObjectQL } from '@objectstack/objectql';
1516
import { RestServer } from './rest-server.js';
1617

@@ -231,6 +232,27 @@ describe('POST /meta/_migrate-record-currency-codes — the caller\'s organizati
231232
expect(again.body).toMatchObject({ codeless: 0, written: 0 });
232233
});
233234

235+
/**
236+
* [#30816] A driver's own throw on one row reaches the tenant admin as the
237+
* import row's cleaned sentence (`sanitizeRowError`), never the driver's
238+
* SQL, constraint or column names; the other rows are still written.
239+
*/
240+
it('a driver-shaped failure on one row answers the cleaned sentence, not the driver\'s text', async () => {
241+
const raw = 'update "deal" set "currency_code" = $1 where "id" = $2 - '
242+
+ 'value too long for type character varying(3)';
243+
vi.mocked(d.driver.update).mockImplementation(async (object: string, id: string, data: Record<string, unknown>) => {
244+
if (id === 'a2') throw new Error(raw);
245+
const row = { ...d.storeFor(object).get(id), ...data, id };
246+
d.storeFor(object).set(id, row);
247+
return row;
248+
});
249+
const answer = await boot(engine, ADMIN_A).call({ object: 'deal', field: 'amount', code: 'USD', apply: true });
250+
expect(answer.status).toBe(200);
251+
expect(answer.body).toMatchObject({ codeless: 2, written: 1, failures: [{ recordId: 'a2', error: sanitizeRowError(raw) }] });
252+
expect(answer.body.failures[0].error).not.toMatch(/update|"deal"|"currency_code"|\$1/);
253+
expect(deal('a1')?.currency_code).toBe('USD');
254+
});
255+
234256
it('`isSystem` with a current organization passes the capability gate and stays in that organization', async () => {
235257
const answer = await boot(engine, { userId: 'u_sys', isSystem: true, tenantId: 'org_b' })
236258
.call({ object: 'deal', field: 'amount', code: 'EUR', apply: true });

0 commit comments

Comments
 (0)