|
| 1 | +--- |
| 2 | +"@objectstack/types": minor |
| 3 | +"@objectstack/rest": patch |
| 4 | +"@objectstack/service-storage": patch |
| 5 | +"@objectstack/service-settings": patch |
| 6 | +"@objectstack/service-datasource": patch |
| 7 | +"@objectstack/service-i18n": patch |
| 8 | +"@objectstack/plugin-sharing": patch |
| 9 | +--- |
| 10 | + |
| 11 | +refactor(types,rest,services,plugin-sharing): one shared writer for the response envelope, and `error.code` is enforced at compile time (#3973) |
| 12 | + |
| 13 | +`BaseResponseSchema` declares one envelope for every REST body the platform |
| 14 | +emits. It declared it once; the code that *wrote* it was copied per route |
| 15 | +module. After #3843 and #3983 converted the last drifting one, seven modules |
| 16 | +each carried their own two-line `sendOk` / `sendError` pair — so the envelope's |
| 17 | +shape lived in fourteen places rather than one. |
| 18 | + |
| 19 | +`pnpm check:route-envelope` proved those seven copies agreed, which is why this |
| 20 | +is a cleanup rather than a bug fix. But a guard proves agreement; it does not |
| 21 | +create it. An eighth module starts by copying the pair again — not |
| 22 | +hypothetically: `share-link-routes.ts` was found already drifting by the |
| 23 | +repo-wide scan, and its drift had broken `client.shareLinks.create()` and |
| 24 | +`.list()` through `unwrapResponse` (#3983). |
| 25 | + |
| 26 | +## What moved |
| 27 | + |
| 28 | +`sendOk` / `sendError` now live once, in `@objectstack/types` |
| 29 | +(`response-envelope.ts`), and all seven modules import them: |
| 30 | + |
| 31 | +| Module | |
| 32 | +|---| |
| 33 | +| `service-storage/storage-routes.ts` | |
| 34 | +| `service-settings/settings-routes.ts` | |
| 35 | +| `service-datasource/admin-routes.ts` | |
| 36 | +| `rest/external-datasource-routes.ts` | |
| 37 | +| `rest/package-routes.ts` | |
| 38 | +| `service-i18n/i18n-service-plugin.ts` | |
| 39 | +| `plugin-sharing/share-link-routes.ts` | |
| 40 | + |
| 41 | +Placement was the open question in #3973, not design. `packages/spec` is |
| 42 | +schemas-only (Prime Directive #2), and the callers span `rest`, four |
| 43 | +`services/*` and one `plugins/*`, which rules out anything depending on them. |
| 44 | +`@objectstack/types` depends on nothing but `@objectstack/spec`, so every caller |
| 45 | +can reach it, and it is already where the repo puts a helper the HTTP boundaries |
| 46 | +share — `looksLikeInternalErrorLeak` (#3867) sits one file over and made the |
| 47 | +same argument first. |
| 48 | + |
| 49 | +The builders take a structural `{ status(n), json(body) }`, so the package |
| 50 | +imports no HTTP contract at all: `IHttpResponse` satisfies it, and so does the |
| 51 | +`any`-typed `res` the older modules carry. |
| 52 | + |
| 53 | +## `error.code` is now checked by the compiler |
| 54 | + |
| 55 | +All seven copies typed the parameter `code: string`. ADR-0112 (#3841) closed the |
| 56 | +vocabulary — `ErrorCode` is `StandardErrorCode ∪ ERROR_CODE_LEDGER` — but an |
| 57 | +invented code was still caught only at runtime, by a conformance suite parsing a |
| 58 | +driven body, i.e. only on routes some test happened to drive. |
| 59 | + |
| 60 | +The shared `sendError` types `code` as `ErrorCode`, so an unregistered code now |
| 61 | +fails to compile, at every call site at once: |
| 62 | + |
| 63 | +```ts |
| 64 | +sendError(res, 400, 'NOT_A_REGISTERED_CODE', 'invented'); |
| 65 | +// Argument of type '"NOT_A_REGISTERED_CODE"' is not assignable to parameter of type 'ErrorCode'. |
| 66 | +``` |
| 67 | + |
| 68 | +This cost no call-site churn: every code the seven modules emit was already |
| 69 | +registered. |
| 70 | + |
| 71 | +## `extra` is closed at the same place |
| 72 | + |
| 73 | +`sendError`'s last parameter is `Pick<ApiError, 'category' | 'httpStatus' | |
| 74 | +'details' | 'requestId'>` — exactly what `ApiErrorSchema` declares beside `code` |
| 75 | +and `message`. |
| 76 | + |
| 77 | +It was `Record<string, unknown>` while `settings-routes` still hung `namespace` / |
| 78 | +`key` / `reason` / `fields` beside `code`. Those bodies passed every gate anyway: |
| 79 | +`ApiErrorSchema` is a plain `z.object`, so unknown keys were STRIPPED rather than |
| 80 | +rejected, and `envelopeViolations` inspects only the body's top level — |
| 81 | +conformant *by stripping* rather than by declaration. #4224 moved that module |
| 82 | +onto `details`, which is what lets the parameter close here. Closing it at the |
| 83 | +shared builder is the part that lasts: an undeclared sibling is now a compile |
| 84 | +error in every module at once, rather than a key that quietly evaporates in |
| 85 | +whichever module reintroduces it. |
| 86 | + |
| 87 | +## Nothing changes on the wire |
| 88 | + |
| 89 | +The seven pairs were identical modulo the optional `status` and `extra` |
| 90 | +parameters this one unions, and each module's driven conformance suite still |
| 91 | +parses its real bodies against the real spec schemas. One internal call site was |
| 92 | +rewritten: `package-routes` passed `details` positionally and now passes |
| 93 | +`{ details }`, producing the same `error.details` it always did. |
| 94 | + |
| 95 | +## The guard got stronger |
| 96 | + |
| 97 | +`scripts/check-route-envelope.mjs` counts response write sites per module. A |
| 98 | +module that routes everything through the shared pair builds **none** itself, so |
| 99 | +the seven now declare `0 / 0 / 0` where they used to declare `2 / 1 / 1`, and the |
| 100 | +shared pair is pinned separately at `2 / 1 / 1` so the invariant stays total for |
| 101 | +the surface rather than per-module. What the count asserts is no longer "your two |
| 102 | +builders are the enveloped ones" but "you have no builders" — and a new route |
| 103 | +that hand-rolls a body still moves it off zero and fails. |
0 commit comments