Skip to content

@objectstack/client reads category/retryable from the body top level — the contract puts them inside error (ADR-0112 D9b, unowned after the batch split) #4006

Description

@os-zhuang

ADR-0112 ruling D9b names this defect and scheduled it to be "fixed when the probe area is touched in batch 3". It fell between the batches: batch 3 (#3971) touched that exact area but landed 10 seconds after the ADR merged, so its author never saw the ruling; batch 1 (#3988) deliberately scoped to spec,core,runtime,docs — no client. Verified still present on main after both. This issue gives it an owner.

The defect

packages/client/src/index.ts:4485-4488 attaches error metadata for programmatic access:

error.code = errorCode;                    // three-location probe, both shapes ✅
error.category = errorBody?.category;      // ❌ top level only
error.httpStatus = res.status;             // transport truth ✅
error.retryable = errorBody?.retryable;    // ❌ top level only

The contract (ApiErrorSchema, and the flat envelope every conformant route emits) carries category and retryable inside error: { success: false, error: { code, message, category?, retryable? } }. Nothing conformant puts them at the body top level. So for every server that follows its own contract, err.category and err.retryable are undefined — the SDK documents them as branch points and they never fire.

The inconsistency is local and visible: three lines down, error.details already does the two-shape read this issue asks for —

error.details = errorBody?.details ?? errorBody?.error?.details ?? errorBody;

— and error.code goes through the full multi-location probe. category/retryable are the only two fields on this block that read one (wrong) location.

Fix shape

Same pattern as the neighbours, nested-first with top-level fallback for legacy/non-conformant servers:

error.category = errorBody?.error?.category ?? errorBody?.category;
error.retryable = errorBody?.error?.retryable ?? errorBody?.retryable;

Plus a test asserting both fields arrive from the enveloped shape (the existing client error-path suites drive the flat envelope already; today no test can assert these because they never populate).

Rider (same sweep, one line)

content/docs/releases/v17.mdx:563 — the #3971 release entry's derived-map example still reads `403` → `permission_denied`; #3988 renamed the derived codes, so the current spelling is PERMISSION_DENIED. One-line doc correction, cheapest carried by whichever PR fixes this issue.

Non-scope

The retirement of the three-location probe itself is a separate, open decision (ADR-0112 D5 says delete after batch 3; the code comment now argues legacy-server fallback) — tracked in its own issue, not here. This issue is only the two mis-nested reads, which are wrong under either outcome of that decision.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions