Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions .changeset/publish-failure-reads-error-message.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
---
"@objectstack/cli": patch
---

`os package publish` now prints the reason a publish was refused instead of the
literal `[object Object]` (#10763).

Both request helpers in `package/publish.ts` built their failure text the same
way:

```ts
const errMsg = parsed?.error ?? response.statusText ?? `HTTP ${response.status}`;
return { ok: false, status: response.status, body: parsed, error: String(errMsg) };
```

In the declared envelope `error` is an **object** — `{ code, message }` — so
`String(errMsg)` stringified the object. The `??` chain never reached
`statusText`, because an object is not nullish; there was no useful fallback to
reach. Every failed publish printed the same seven characters no matter what the
control plane had refused, at all three call sites: package registration,
version publish, and the icon upload.

Both sites now read through a new `readErrorMessage` in
`packages/cli/src/utils/response-envelope.ts`, which returns the declared
envelope's `error.message`, degrades to `error.code` when a refusal carries no
message, and falls back to a non-blank `statusText` and then the status line. A
blank `statusText` counts as absent — HTTP/2 carries no reason phrase, and the
old `??` chain kept the empty string and printed nothing after the status code.

The reader also accepts the flat `error: '<sentence>'` shape, deliberately and
temporarily. That is a **measured** property of these routes rather than an
assumption: `/api/v1/cloud/**` is served by the sibling `cloud` repo, and the
closest first-hand reader of that same `service-cloud` family — objectui's
`readApiError` — records that it answers failures in both shapes while cloud#944
converts it. A strict envelope-only read (the `readEnvelope` landed by #10675
for the in-repo `/api/v1/datasources/**` routes) would have replaced today's
live flat dialect with a different unreadable failure, so it is not reused here;
the reasoning, and the condition under which the flat branch is deleted, are
recorded on the function.

No request the CLI sends changes, and the server sends exactly what it sent
before — this is only how a failure is read and shown.
7 changes: 3 additions & 4 deletions packages/cli/src/commands/package/publish.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,6 +23,7 @@ import { resolve as resolvePath, basename, dirname, isAbsolute } from 'node:path
import { Args, Command, Flags } from '@oclif/core';
import { printHeader, printKV, printSuccess, printError, printStep } from '../../utils/format.js';
import { DEFAULT_CLOUD_URL, tryReadCloudConfig } from '../../utils/cloud-config.js';
import { readErrorMessage } from '../../utils/response-envelope.js';

const MANIFEST_ID_RE = /^[a-z0-9][a-z0-9._-]{0,254}$/i;

Expand DownExpand Up@@ -643,8 +644,7 @@ export default class PackagePublish extends Command {
let parsed: any = null;
try { parsed = await response.json(); } catch { /* empty/non-json */ }
if (!response.ok) {
const errMsg = parsed?.error ?? response.statusText ?? `HTTP ${response.status}`;
return { ok: false, status: response.status, body: parsed, error: String(errMsg) };
return { ok: false, status: response.status, body: parsed, error: readErrorMessage(parsed, response) };
}
return { ok: true, status: response.status, body: parsed };
} catch (err: any) {
Expand DownExpand Up@@ -688,8 +688,7 @@ export default class PackagePublish extends Command {
let parsed: any = null;
try { parsed = await response.json(); } catch { /* empty/non-json */ }
if (!response.ok) {
const errMsg = parsed?.error ?? response.statusText ?? `HTTP ${response.status}`;
return { ok: false, status: response.status, body: parsed, error: String(errMsg) };
return { ok: false, status: response.status, body: parsed, error: readErrorMessage(parsed, response) };
}
return { ok: true, status: response.status, body: parsed };
} catch (err: any) {
Expand Down
74 changes: 73 additions & 1 deletion packages/cli/src/utils/response-envelope.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,7 +14,7 @@
import { describe, expect, it } from 'vitest';
import { sendError, sendOk } from '@objectstack/types';
import { serverBody } from './__tests__/server-body.js';
import { readEnvelope, readEnvelopeFrom } from './response-envelope.js';
import { readEnvelope, readEnvelopeFrom, readErrorMessage } from './response-envelope.js';

describe('readEnvelope', () => {
it('returns the payload nested under `data` for a `sendOk` body', () => {
Expand DownExpand Up@@ -87,3 +87,75 @@ describe('readEnvelopeFrom', () => {
expect((read as { message: string }).message).toContain('HTTP 404');
});
});

/**
* `readErrorMessage` — the PRINTABLE-message reader (#10763).
*
* Two dialects are covered because the control plane really does emit two: the
* declared envelope, and the flat `error: '<sentence>'` its `fail()` helper
* still writes while cloud#944 converts it. The declared arm is built with
* `sendError`, the one writer, for the reason the file header gives. The flat
* arm has to be a literal — its writer lives in the closed `cloud` repo and
* cannot be imported — so it is written here exactly as objectui's `readApiError`
* records it, and that transcription is the thing to re-check if it ever drifts.
*/
describe('readErrorMessage', () => {
const res = (status: number, statusText = 'Bad Request') => ({ status, statusText });

it('reads `error.message` — the FIELD — out of the declared envelope', () => {
const body = serverBody((r) =>
sendError(r, 422, 'PACKAGE_PUBLISH_FAILED', 'Version 1.2.0 already exists for com.acme.crm.'),
);

expect(readErrorMessage(body, res(422))).toBe('Version 1.2.0 already exists for com.acme.crm.');
});

it('reads the control plane’s flat `fail()` dialect, which is still live (cloud#944)', () => {
const body = { success: false, error: 'Publisher is not verified.' };

expect(readErrorMessage(body, res(403))).toBe('Publisher is not verified.');
});

/**
* The defect this card is named for. `String(parsed?.error)` over the
* declared envelope produced this literal, and `??` never reached the
* `statusText` fallback because an object is not nullish.
*/
it('NEVER renders an error object as text, in any shape', () => {
const bodies: unknown[] = [
serverBody((r) => sendError(r, 400, 'VALIDATION_ERROR', 'Manifest is invalid.')),
{ success: false, error: { code: 'FORBIDDEN' } },
{ success: false, error: {} },
{ success: false, error: [] },
{ success: false, error: { message: 42 } },
{ success: false, error: { message: ' ' } },
];

for (const body of bodies) {
const read = readErrorMessage(body, res(400));
expect(typeof read, JSON.stringify(body)).toBe('string');
expect(read, JSON.stringify(body)).not.toContain('[object');
}
});

it('falls back to the code when the envelope refuses without a message', () => {
expect(readErrorMessage({ success: false, error: { code: 'PACKAGE_PUBLISH_FAILED' } }, res(422)))
.toBe('PACKAGE_PUBLISH_FAILED');
});

it('falls back to `statusText`, then to the status line, when the body carries no text', () => {
expect(readErrorMessage(null, res(502, 'Bad Gateway'))).toBe('Bad Gateway');
expect(readErrorMessage({ success: false }, res(502, 'Bad Gateway'))).toBe('Bad Gateway');
expect(readErrorMessage('not json at all', res(502, 'Bad Gateway'))).toBe('Bad Gateway');
});

/**
* HTTP/2 carries no reason phrase, so `fetch` reports `statusText` as ''. The
* original chain used `??`, which keeps an empty string and printed nothing
* after the status code — the second half of "there is no useful fallback".
*/
it('treats a blank `statusText` as absent rather than printing nothing', () => {
expect(readErrorMessage(null, { status: 500, statusText: '' })).toBe('HTTP 500');
expect(readErrorMessage(null, { status: 500 })).toBe('HTTP 500');
});
});
92 changes: 92 additions & 0 deletions packages/cli/src/utils/response-envelope.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -113,3 +113,95 @@ export async function readEnvelopeFrom<T>(res: EnvelopeSource): Promise<Envelope
}
return readEnvelope<T>(body, res.status);
}

/**
* Read a PRINTABLE failure message out of a response body the CLI has already
* decided is a failure.
*
* ## Why this is tolerant when {@link readEnvelope} above is strict
*
* They do different jobs, and the strictness follows the job rather than the
* file. `readEnvelope` decides **whether** a request succeeded and hands back
* its payload; tolerating an off-spec body there would let a payload be read as
* data, which is how a second de-facto contract grows (Prime Directive #12) and
* why that reader refuses the legacy flat shape outright.
*
* This function decides **nothing**. The caller has already seen
* `!response.ok`; the only remaining question is which bytes to show a human.
* The failure is reported either way — the choice is between the server's own
* sentence and a placeholder.
*
* ## What the control plane actually sends (measured, not assumed)
*
* The `/api/v1/cloud/**` publish routes are served by the sibling `cloud` repo,
* which this repo's dispatcher explicitly refuses — so no in-repo ledger can
* vouch for them (`docs/audits/2026-07-dispatcher-client-route-coverage.md`
* §10). The measurement comes from the closest first-hand reader of that same
* `service-cloud` family, `readApiError` in objectui's
* `packages/app-shell/src/console/marketplace/marketplaceApi.ts`, which records
* that those routes answer failures in TWO shapes and are mid-conversion from
* one to the other (cloud#944):
*
* { success: false, error: 'a sentence' } today, via the cloud's `fail()`
* { success: false, error: { code, message } } the declared envelope
*
* So BOTH arms are live. That is what rules out reusing `readEnvelope` here:
* against the dialect the control plane still emits today it would discard a
* real explanation and print "not the declared envelope" instead — trading one
* unreadable failure for another.
*
* ## This accommodation is bounded, and it is the consumer's only option
*
* The flat dialect is a PRODUCER defect, tracked and being converted at the
* producer (cloud#944) — it is not a shape this repo can fix, and not one it is
* hiding. When that conversion lands, the `fail()` branch below is deletable on
* its own, and nothing else here changes.
*
* Deliberately NOT accepted: a top-level `body.message`. objectui's reader
* tolerates one because a few OTHER routes in that family put text there; no
* publish route was measured doing it, and inventing a third dialect to read is
* the accretion #12 forbids.
*
* ## Why it can never return `[object Object]`
*
* Every branch either yields a checked non-empty string or falls through. The
* defect under repair was `String(parsed?.error)` over an `error` that is an
* OBJECT: `??` never fell through to `statusText`, because an object is not
* nullish, so the fallback chain was unreachable and the operator got
* `[object Object]` instead of the reason the publish was refused.
*
* `statusText` is treated as absent when blank for the same reason the original
* chain failed: HTTP/2 carries no reason phrase, so `??` would have kept an
* empty string and printed nothing at all.
*/
export function readErrorMessage(
body: unknown,
res: { status: number; statusText?: string },
): string {
return errorTextFrom(body) ?? blankToUndefined(res.statusText) ?? `HTTP ${res.status}`;
}

/** A string is usable as a message only when it is actually a non-blank string. */
function blankToUndefined(value: unknown): string | undefined {
return typeof value === 'string' && value.trim().length > 0 ? value : undefined;
}

/**
* The server's own text, in whichever of the two measured dialects it arrived —
* or `undefined` when the body carries none, so the caller can fall back.
*/
function errorTextFrom(body: unknown): string | undefined {
if (typeof body !== 'object' || body === null) return undefined;
const error = (body as { error?: unknown }).error;

if (typeof error === 'object' && error !== null) {
// The declared envelope: `message` is a FIELD of `error`, never the object
// itself. `code` is the last resort that keeps a refusal naming SOMETHING
// machine-readable rather than degrading to a bare status line.
const declared = error as { code?: unknown; message?: unknown };
return blankToUndefined(declared.message) ?? blankToUndefined(declared.code);
}

// The control plane's `fail()` dialect (cloud#944): `error` IS the sentence.
return blankToUndefined(error);
}
Loading
Loading