Skip to content

Commit 74fd4bf

Browse files
fix(cloud-connection): unmanaged install-local writes the log line its fallback names, at any status (#29857)
<!-- claude-projects-attribution: {"github_login":"os-warren"} --> _Requested by **Warren** · [project thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ8Bjniy1KUKzykb69Rgkafke)_ Fixes #29822 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:`POST /api/v1/marketplace/install-local` 的 unmanaged 分支,导入被拒时,只要回答用的是兜底句「the reason is in the server log」,就写那条 `warn` 日志,不再只在 5xx 时写;能原样带回原因的拒绝不写。状态码、错误码、响应形状都不变。 - 为什么改:用 `statusCode` 或校验形状(无数字 `status`)声明 4xx 的拒绝,消息不会回给客户端,回答却让运维去看日志,而日志里什么都没有(#29342 的 dev 经 HTTP 门实测:400、无日志)。 - 风险与回滚:只多写一条 `warn`,不改任何对外字段;回滚即还原一个 commit。 - 先看:`packages/cloud-connection/src/marketplace-install-local-plugin.ts` 的 unmanaged catch(`message === fallback` 那一行),其次 `marketplace-install-local-install-modes.test.ts` 新增的三个用例。 - 两分钟验证:`pnpm --filter @objectstack/cloud-connection exec vitest run --maxWorkers=2 src/marketplace-install-local-install-modes.test.ts`,预期 26/26 通过。 - 需要你做的:无,席位落地。 ## What changes The unmanaged branch of `POST /api/v1/marketplace/install-local` catches the import's throw and answers `clientFacingFailureText(err, fallback)`, where the fallback says "the reason is in the server log". It used to write that log line only when the throw resolved to 5xx. `clientFacingFailureText` quotes a message only when the throw declared a numeric 4xx `status`, so a 4xx this door resolves through `statusCode` or the validation shape got the fallback and no log line. Now there is one rule at every status: the door computes the answer first, and writes the `warn` line exactly when the answer is the fallback sentence. The fallback sentence is spelled once. The log line also names the resolved status and code. A refusal whose reason reaches the body writes no line, as before. ## Why The operator who moves a package between environments with `os package install --unmanaged` or the Console was told where the reason is, and the reason was not there. ## Tests `packages/cloud-connection/src/marketplace-install-local-install-modes.test.ts`, beside the existing 5xx case: - Two cases, a 4xx declared by `statusCode: 409` and one declared by the validation shape: the body carries the reason, or the `warn` log carries the import's message. - One control: a refusal with a numeric 409 `status` is quoted in the body and writes no log line. Ablation (fix committed first at 4ac6248, mutation through `scripts/ablation-replace.mjs`, `if (message === fallback) {` back to `if (resolved.status >= 500) {`, anchor 1 → 0, blob `ec8cba1e` → `1a44e3bf`): both 4xx cases go red (`expected '[MarketplaceInstallLocal] [protocol] …' to contain 'Record is locked by approval apv_7'`, and the same for the validation sentence), 2 failed / 24 passed. Restored: `git diff HEAD` empty, blob back to `ec8cba1e` = the HEAD blob. The test imports the plugin from `src` by a relative path, so no `dist/` leg is involved. ## Acceptance notes How the other doors that use a "reason is in the server log" fallback pair it with a log line, read on `origin/main` 42ff86f: - `protocol.ts` `migrateStoredMetadata` and the publish side-effects collector, `protocol-publish-backfill.ts`, and `runtime/src/domains/packages.ts` `applyPublishedSeeds` log unconditionally beside the fallback, at any status. - Batch rows (`toRowApiError` → `clientFacingRowFailureText`) and the external-driver read floor (`external-driver-floor.ts` `readRefusal`) log at the point they withhold, which is the rule this PR applies. - `seed-loader.ts` logs the cause beside each withheld row through its `seedCauseLabel` lines. - `packagePersistFailureError` always answers 5xx, so the boundary logs it. No exported helper couples "fallback text ⇔ log line" today. The batch-row one is private to `metadata-protocol`, so this door applies the rule inline. No other door showed the same 4xx gap. ## Verification All local readings are on head `4ac62487` (the only commit). <details><summary>Package test, typecheck, lint</summary> - `pnpm --filter @objectstack/cloud-connection test` :: exit 0 — Test Files 68 passed (68), Tests 823 passed (823) - `pnpm --filter @objectstack/cloud-connection typecheck` (`tsc --noEmit` and `tsconfig.test.json`) :: exit 0 - `pnpm lint` (`eslint . --no-inline-config`, whole repo) :: exit 0 - The pin file alone: 26 passed (26); ablated: 2 failed, 24 passed. </details> <details><summary>Derived gates (dispatch-gates --commands, reconciled with --ran)</summary> `dispatch-gates --ran`: 59 derived families accounted for, 57 run with exit 0, 2 NOT MEASURED (exit 3, prerequisite not met), 0 unrun. - `node scripts/check-plugin-teardown-shape.mjs --self-test`: NOT MEASURED, its positive control is pinned to a commit outside this shallow clone. CI runs it. - `pnpm check:dual-build-cjs-loads`: NOT MEASURED, it reads every package's `dist/` and only the cloud-connection closure was built here. CI runs it. </details> ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. - [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: 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. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. --- _Generated by [Claude Code](https://claude.ai/code/session_01JQwxbZcVyAXy4Gm9UakRew)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 592845b commit 74fd4bf

3 files changed

Lines changed: 58 additions & 7 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/cloud-connection': patch
3+
---
4+
5+
fix(cloud-connection): an unmanaged install-local refusal that points at the server log now writes the log line
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
`POST /api/v1/marketplace/install-local` with `installMode: 'unmanaged'` answers a refused import with the import's own code and status. When the refusal's message may not be quoted back (it declared its 4xx through `statusCode` or the validation shape rather than a numeric `status`), the answer says "the reason is in the server log". Until now the door wrote that log line only for a 5xx, so a 4xx refusal sent the operator to a log that held nothing about it.
11+
12+
The door now follows one rule at every status: whenever the answer uses that fallback sentence, it writes the `warn` line naming the package, the status, the code and the import's own message. A refusal whose reason reaches the response body writes no line. The status, the code and the response shape are unchanged.

‎packages/cloud-connection/src/marketplace-install-local-install-modes.test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,37 @@ describe('install-local — the `unmanaged` branch is the import, and nothing of
389389
expect(h.logger.warn.mock.calls.map((call: any[]) => String(call[0])).join('\n')).toContain('SQLITE_ERROR');
390390
});
391391

392+
// A 4xx refusal either carries its reason in the body, or writes the log
393+
// line the body points at. Each throw below resolves to a 4xx through a
394+
// declaration other than a numeric `status`, so its message is withheld.
395+
it.each([
396+
['`statusCode` 409', Object.assign(new Error('Record is locked by approval apv_7'), { statusCode: 409, code: 'RECORD_LOCKED' })],
397+
['the validation shape (no status)', Object.assign(new Error('Validation failed: crm_lead.plan is required'), {
398+
code: 'VALIDATION_FAILED',
399+
fields: [{ field: 'plan', code: 'required', message: 'plan is required' }],
400+
})],
401+
])('a 4xx refusal declared by %s: the body carries its reason, or the log line it points at is written', async (_label, thrown) => {
402+
const h = await mount(dir, { importer: async () => { throw thrown; } });
403+
const res = await h.install({ manifest: crm({ installModes: BOTH }), installMode: 'unmanaged' });
404+
expect(res.status, JSON.stringify(res.payload)).toBeGreaterThanOrEqual(400);
405+
expect(res.status).toBeLessThan(500);
406+
const warned = h.logger.warn.mock.calls.map((call: any[]) => String(call[0])).join('\n');
407+
if (res.payload.error.message.includes('the reason is in the server log')) {
408+
expect(warned).toContain(thrown.message);
409+
} else {
410+
expect(res.payload.error.message).toContain(thrown.message);
411+
}
412+
});
413+
414+
it('a 4xx refusal whose reason reaches the body writes no log line', async () => {
415+
const refusal = Object.assign(new Error("Package 'com.example.crm' already has a writable copy."), { status: 409, code: 'RESOURCE_CONFLICT' });
416+
const h = await mount(dir, { importer: async () => { throw refusal; } });
417+
const res = await h.install({ manifest: crm({ installModes: BOTH }), installMode: 'unmanaged' });
418+
expect(res.status).toBe(409);
419+
expect(res.payload.error.message).toBe(refusal.message);
420+
expect(h.logger.warn.mock.calls.map((call: any[]) => String(call[0])).join('\n')).not.toContain(refusal.message);
421+
});
422+
392423
it('a runtime whose protocol has no copy-once import answers 503 SERVICE_UNAVAILABLE — never a managed install instead', async () => {
393424
const h = await mount(dir, { importer: null });
394425
const res = await h.install({ manifest: crm({ installModes: BOTH }), installMode: 'unmanaged' });

‎packages/cloud-connection/src/marketplace-install-local-plugin.ts‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3598,7 +3598,8 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
35983598
* (uncopyable content, collisions, an owned namespace, an existing
35993599
* target) and this door answers each with the verb's own code and
36003600
* status, the message when the verb declared it a refusal, and
3601-
* `details`; a 5xx's message is withheld and logged;
3601+
* `details`; any other message is withheld and logged, at any
3602+
* status, so the log line the answer points at exists;
36023603
* - translation bundles are COPIED by the verb as `translation` items, so
36033604
* they reload at every boot with the environment's translations — this
36043605
* install writes no ledger entry for a rehydrate to replay;
@@ -3651,20 +3652,27 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
36513652
} as Parameters<NonNullable<UnmanagedImporter['importUnmanagedPackage']>>[0]);
36523653
} catch (err: any) {
36533654
const resolved = resolveThrownHttpError(err, 500);
3654-
if (resolved.status >= 500) {
3655+
// [#29822] One rule, whatever the status: the fallback sentence
3656+
// promises a log line, so whenever it IS the answer, the line is
3657+
// written. `clientFacingFailureText` quotes only a refusal that
3658+
// declared a 4xx `status`; a 4xx this door resolves from another
3659+
// declaration (`statusCode`, the validation shape) gets the
3660+
// fallback too, and used to get no log line. A quoted reason
3661+
// needs no log line: the caller already has it.
3662+
const fallback = `The unmanaged install of '${manifestId}' failed; the reason is in the server log.`;
3663+
const message = clientFacingFailureText(err, fallback);
3664+
if (message === fallback) {
36553665
ctx.logger?.warn?.(
3656-
`[MarketplaceInstallLocal] the unmanaged install of ${manifestId} failed: ${err?.message ?? err}`,
3666+
`[MarketplaceInstallLocal] the unmanaged install of ${manifestId} failed (${resolved.status} `
3667+
+ `${resolved.code}): ${err?.message ?? err}`,
36573668
);
36583669
}
36593670
const details = isPlainObject(err?.details) ? err.details : undefined;
36603671
return c.json({
36613672
success: false,
36623673
error: {
36633674
code: resolved.code,
3664-
message: clientFacingFailureText(
3665-
err,
3666-
`The unmanaged install of '${manifestId}' failed; the reason is in the server log.`,
3667-
),
3675+
message,
36683676
...(details ? { details } : {}),
36693677
},
36703678
}, resolved.status as any);

0 commit comments

Comments
 (0)