Skip to content

Commit 5a7d3eb

Browse files
feat(api): document declared error shapes across public oRPC routes (#1109)
* feat(api): document declared error shapes across public oRPC routes Attach commonApiErrors (401/403/422/429/500 + workspace-access denials) once to the shared public API stacks in orpc.ts, so every public procedure inherits a consistent OpenAPI error contract without a per-router .errors() call. Remap oRPC's own BAD_REQUEST validation throw to 422 so schema-level and business-level validation failures share one status, and align validationException to 422 to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013VWhmQGALFoH4NVXMsXypA * chore: fix revie --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 9e16dc5 commit 5a7d3eb

41 files changed

Lines changed: 414 additions & 30 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.agents/skills/orpc-api/SKILL.md

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ generated `operationId` (`myFeature.get`), which the MCP server turns into
168168
the tool name (`my_feature_get`).
169169

170170
```typescript
171+
import { possibleErrorsOnFindingResource } from "@/lib/orpc/orpc-error-helper"
171172
import { workspaceTokenAuthAPIForScope } from "@/orpc"
172173

173174
// Pick the resource-area scope this feature belongs to
@@ -184,6 +185,8 @@ export const myFeaturePublicRouter = {
184185
})
185186
.input(z.object({ id: zodBigintAsString() }))
186187
.output(publicMyFeatureResponse)
188+
// Declares only what varies by operation shape — see "Declared errors"
189+
.errors(possibleErrorsOnFindingResource)
187190
.handler(async ({ context, input }) => {
188191
// context.workspace is available from token auth
189192
return await findMyFeature({
@@ -385,6 +388,41 @@ throw notFoundException("Item not found")
385388
throw new ChatbotXException("Custom error", "BAD_REQUEST", 400)
386389
```
387390

391+
### Declared errors (`.errors()`) — public routes only
392+
393+
Every public procedure must declare the errors it can throw, because that
394+
declaration is what renders the non-2xx responses in the OpenAPI spec (and
395+
what the MCP server and CLI show a caller). The declaration is split in two:
396+
397+
| Layer | Where | Contains |
398+
|-------|-------|----------|
399+
| Shared | `commonApiErrors`, attached **once** to the public stacks in `@/orpc` | 401 (`UNAUTHORIZED`, `INVALID_CHATBOT_TOKEN`), 403 (`FORBIDDEN`, `trialExpired`, `macLimitReached`), 422 (`invalidRequestData`, `validation`), 429 (`tooManyRequests`), 500 (`INTERNAL_SERVER_ERROR`) |
400+
| Per-route | one `possibleErrorsOn*Resource` set from `@/lib/orpc/orpc-error-helper` | only what varies by operation shape — `notFound` (404) and `businessError` (400) |
401+
402+
Pick the per-route set by what the handler can actually fail with, not by the
403+
HTTP verb:
404+
405+
- `possibleErrorsOnListingResource` — a collection read that cannot 404.
406+
- `possibleErrorsOnFindingResource` — a read that resolves one resource.
407+
- `possibleErrorsOnCreatingResource` — a create with no parent lookup.
408+
- `possibleErrorsOnMutatingResource` — an update, **or a create that resolves a
409+
parent from a path param** (e.g. `POST /v1/contacts/{identifier}/notes` calls
410+
`contactService.resolveIdByIdentifier`, which throws 404).
411+
- `possibleErrorsOnDeletingResource` — a delete.
412+
413+
**Never re-declare a `commonApiErrors` code in a per-route set.** Doing so
414+
duplicates the entry in the generated spec. The guard in
415+
`apps/builder/__tests__/public-spec-operations.test.ts` fails on both mistakes:
416+
a route missing a universal code, and a duplicated one.
417+
418+
**The `code` string is the contract, not the status.** oRPC matches a thrown
419+
`ORPCError` to its declaration by `code` *and* exact `status`
420+
(`validateORPCError` in `@orpc/contract`). On a miss it does not error — it
421+
returns the error with `defined: false`, so an undeclared code still reaches
422+
the client but never appears in the spec. That silent degrade is why a new
423+
`ChatbotXException` code thrown from a public route needs a matching entry in
424+
one of these sets.
425+
388426
## Logging
389427

390428
Import the logger from the nearest `lib/log` or `lib/logger` module. Never use `console` in handlers.

apps/builder/__tests__/orpc-error-mapping.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,18 @@ vi.mock("@chatbotx.io/sdk", () => ({
4949
},
5050
}))
5151

52+
// `@/orpc` now attaches `commonApiErrors` (from orpc-error-helper.ts) to the
53+
// public stacks, which reuses `DENIAL_MESSAGES` from this module — stub it so
54+
// importing `@/orpc` doesn't pull in the real `@chatbotx.io/business` barrel
55+
// (and transitively the google-calendar integration's `@chatbotx.io/sdk`
56+
// `Integration` export, which the mock above doesn't provide).
57+
vi.mock("@/lib/workspace/authorize-workspace-access", () => ({
58+
DENIAL_MESSAGES: {
59+
trialExpired: "Trial expired",
60+
macLimitReached: "Monthly active contact limit reached",
61+
},
62+
}))
63+
5264
const { ChatbotXException } = await import("@chatbotx.io/business/errors")
5365
const { ModelNotfoundException } = await import("@chatbotx.io/database/errors")
5466
const { SdkException } = await import("@chatbotx.io/sdk")
@@ -106,6 +118,28 @@ describe("mapKnownOrpcErrors", () => {
106118
expect(mockLoggerWarn).toHaveBeenCalledTimes(1)
107119
})
108120

121+
test("maps a validationException-shaped ChatbotXException to a 422 validation error", () => {
122+
const error = new ChatbotXException("Name already taken", "validation", 422)
123+
124+
expect(() => mapKnownOrpcErrors(error)).toThrow(
125+
expect.objectContaining({ code: "validation", status: 422 }),
126+
)
127+
expect(mockLoggerWarn).toHaveBeenCalledTimes(1)
128+
})
129+
130+
test("maps a tooManyRequests ChatbotXException to a 429 error", () => {
131+
const error = new ChatbotXException(
132+
"Too many requests. Retry after 5s.",
133+
"tooManyRequests",
134+
429,
135+
)
136+
137+
expect(() => mapKnownOrpcErrors(error)).toThrow(
138+
expect.objectContaining({ code: "tooManyRequests", status: 429 }),
139+
)
140+
expect(mockLoggerWarn).toHaveBeenCalledTimes(1)
141+
})
142+
109143
test("maps a 5xx ChatbotXException without warn-logging — the route callback logs it once", () => {
110144
const error = new ChatbotXException("boom", "upstream", 502)
111145

apps/builder/__tests__/public-spec-operations.test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ type SpecOperation = {
3030
tags: string[]
3131
summary?: string
3232
security?: Record<string, string[]>[]
33+
responseStatuses: string[]
3334
}
3435

3536
const LEGACY_WORKSPACE_TOKEN_PATTERN = /workspace[_.]?token/i
@@ -133,6 +134,7 @@ beforeAll(async () => {
133134
tags: op.tags ?? [],
134135
summary: op.summary,
135136
security: op.security,
137+
responseStatuses: Object.keys(op.responses ?? {}),
136138
})
137139

138140
const successResponse = Object.entries(op.responses ?? {}).find(
@@ -221,3 +223,68 @@ describe("public API spec — operation naming guard", () => {
221223
}
222224
})
223225
})
226+
227+
const PATH_PARAM_PATTERN = /\{[^}]+\}/
228+
229+
describe("public API spec — error response coverage", () => {
230+
const COMMON_ERROR_STATUSES = ["400", "401", "403", "429", "500"]
231+
232+
// `channels.me` has no `.input()` and no possible business-logic failure —
233+
// it echoes the authenticated token's identity — so it legitimately has no
234+
// 400 (business error) or 422 (validation error) case.
235+
const NO_400_OPERATION_IDS = new Set(["channels.me"])
236+
237+
test("every operation documents the shared 400/401/403/429/500 errors", () => {
238+
const missing = operations
239+
.filter((op) => !NO_400_OPERATION_IDS.has(op.operationId))
240+
.filter((op) =>
241+
COMMON_ERROR_STATUSES.some(
242+
(status) => !op.responseStatuses.includes(status),
243+
),
244+
)
245+
.map((op) => op.operationId)
246+
247+
expect(missing).toEqual([])
248+
})
249+
250+
test("every DELETE, PUT/PATCH, and GET-by-id operation documents 404", () => {
251+
const shouldDocument404 = operations.filter(
252+
(op) =>
253+
op.method === "DELETE" ||
254+
op.method === "PUT" ||
255+
op.method === "PATCH" ||
256+
(op.method === "GET" && PATH_PARAM_PATTERN.test(op.path)),
257+
)
258+
259+
expect(shouldDocument404.length).toBeGreaterThan(0)
260+
261+
const missing404 = shouldDocument404
262+
.filter((op) => !op.responseStatuses.includes("404"))
263+
.map((op) => op.operationId)
264+
265+
expect(missing404).toEqual([])
266+
})
267+
268+
test("every POST/PUT/PATCH operation documents 422", () => {
269+
const bodyMethods = operations.filter(
270+
(op) =>
271+
op.method === "POST" || op.method === "PUT" || op.method === "PATCH",
272+
)
273+
274+
expect(bodyMethods.length).toBeGreaterThan(0)
275+
276+
const missing422 = bodyMethods
277+
.filter((op) => !op.responseStatuses.includes("422"))
278+
.map((op) => op.operationId)
279+
280+
expect(missing422).toEqual([])
281+
})
282+
283+
test("no operation documents 413 — the payload-too-large status is out of scope", () => {
284+
const with413 = operations
285+
.filter((op) => op.responseStatuses.includes("413"))
286+
.map((op) => op.operationId)
287+
288+
expect(with413).toEqual([])
289+
})
290+
})

apps/builder/__tests__/workspace-token-auth-middleware.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ describe("workspaceTokenAuthMidddleware", () => {
118118

119119
await expect(callMiddleware(headers, "GET")).rejects.toMatchObject({
120120
code: "INVALID_CHATBOT_TOKEN",
121+
status: 401,
121122
})
122123
})
123124

@@ -128,6 +129,7 @@ describe("workspaceTokenAuthMidddleware", () => {
128129

129130
await expect(callMiddleware(headers, "GET")).rejects.toMatchObject({
130131
code: "INVALID_CHATBOT_TOKEN",
132+
status: 401,
131133
})
132134
expect(findWorkspaceByTokenHash).toHaveBeenCalledTimes(1)
133135
// Invalid guesses never resolve a workspace, so only the pre-auth

apps/builder/__tests__/workspace-token-scope-enforcement.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,19 @@ describe("workspace API token resource-scope enforcement", () => {
182182

183183
await expect(invoke(procedure)).rejects.toMatchObject({
184184
code: "INVALID_CHATBOT_TOKEN",
185+
status: 401,
186+
})
187+
})
188+
189+
test("an invalid token error is upgraded to defined:true once commonApiErrors is attached", async () => {
190+
findWorkspaceByTokenHash.mockResolvedValue(undefined)
191+
192+
const procedure = buildProcedure("contacts", "GET")
193+
194+
await expect(invoke(procedure)).rejects.toMatchObject({
195+
code: "INVALID_CHATBOT_TOKEN",
196+
status: 401,
197+
defined: true,
185198
})
186199
})
187200
})

apps/builder/src/enterprise/features/inbox-teams/api/public.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { possibleErrorsOnListingResource } from "@/lib/orpc/orpc-error-helper"
12
import { paginateInMemory, publicListRequest } from "@/lib/public-api/list"
23
import { workspaceTokenAuthAPIForScope } from "@/orpc"
34
import { listInboxTeams } from "../queries"
@@ -15,6 +16,7 @@ export const inboxTeamsPublicRouter = {
1516
})
1617
.input(publicListRequest)
1718
.output(publicListInboxTeamsResponse)
19+
.errors(possibleErrorsOnListingResource)
1820
.handler(async ({ context, input }) => {
1921
const { data } = await listInboxTeams({
2022
workspaceId: context.workspace.id,

apps/builder/src/features/ai-agents/api/public.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { aiAgentService } from "@chatbotx.io/business"
2+
import { possibleErrorsOnListingResource } from "@/lib/orpc/orpc-error-helper"
23
import { publicListRequest } from "@/lib/public-api/list"
34
import { workspaceTokenAuthAPIForScope } from "@/orpc"
45
import { listAIAgentsResponse } from "../schema/query"
@@ -15,6 +16,7 @@ export const aiAgentsPublicRouter = {
1516
})
1617
.input(publicListRequest)
1718
.output(listAIAgentsResponse)
19+
.errors(possibleErrorsOnListingResource)
1820
.handler(
1921
async ({ context, input }) =>
2022
await aiAgentService.listAIAgents({

apps/builder/src/features/automated-response/api/public.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { automatedResponseService } from "@chatbotx.io/business"
2+
import { possibleErrorsOnListingResource } from "@/lib/orpc/orpc-error-helper"
23
import { publicListRequest, publicListResponse } from "@/lib/public-api/list"
34
import { workspaceTokenAuthAPIForScope } from "@/orpc"
45
import { publicKeywordResource } from "../schema/resource"
@@ -15,6 +16,7 @@ export const keywordsPublicRouter = {
1516
})
1617
.input(publicListRequest)
1718
.output(publicListResponse(publicKeywordResource))
19+
.errors(possibleErrorsOnListingResource)
1820
.handler(async ({ context, input }) => {
1921
const result = await automatedResponseService.list({
2022
workspaceId: context.workspace.id,

apps/builder/src/features/bot-fields/api/public.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
possibleErrorsOnCreatingResource,
55
possibleErrorsOnDeletingResource,
66
possibleErrorsOnFindingResource,
7+
possibleErrorsOnMutatingResource,
78
} from "@/lib/orpc/orpc-error-helper"
89
import { publicListRequest } from "@/lib/public-api/list"
910
import { workspaceTokenAuthAPIForScope } from "@/orpc"
@@ -84,7 +85,7 @@ export const botFieldsPublicRouter = {
8485
z.object({ idOrName: z.string().max(255), value: z.string().max(255) }),
8586
)
8687
.output(publicBotFieldResource)
87-
.errors(possibleErrorsOnCreatingResource)
88+
.errors(possibleErrorsOnMutatingResource)
8889
.handler(async ({ context, input }) => {
8990
const { idOrName, ...rest } = input
9091
return await botFieldService.updateByKey({
@@ -109,7 +110,7 @@ export const botFieldsPublicRouter = {
109110
),
110111
}),
111112
)
112-
.errors(possibleErrorsOnCreatingResource)
113+
.errors(possibleErrorsOnMutatingResource)
113114
.handler(async ({ context, input }) => {
114115
await Promise.all(
115116
input.fields.map(({ key, value }) =>
@@ -146,7 +147,7 @@ export const botFieldsPublicRouter = {
146147
),
147148
}),
148149
)
149-
.errors(possibleErrorsOnCreatingResource)
150+
.errors(possibleErrorsOnMutatingResource)
150151
.handler(async ({ context, input }) => {
151152
await botFieldService.bulkUpdateByKeys({
152153
workspaceId: context.workspace.id,

apps/builder/src/features/broadcasts/api/public.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
import z from "zod"
2+
import {
3+
possibleErrorsOnFindingResource,
4+
possibleErrorsOnListingResource,
5+
} from "@/lib/orpc/orpc-error-helper"
26
import { publicListRequest } from "@/lib/public-api/list"
37
import { workspaceTokenAuthAPIForScope } from "@/orpc"
48
import {
@@ -24,6 +28,7 @@ export const broadcastsPublicRouter = {
2428
})
2529
.input(publicListRequest)
2630
.output(publicListBroadcastsResponse)
31+
.errors(possibleErrorsOnListingResource)
2732
.handler(
2833
async ({ context, input }) =>
2934
await listBroadcasts({
@@ -43,6 +48,7 @@ export const broadcastsPublicRouter = {
4348
})
4449
.input(z.object({ idOrName: z.string() }))
4550
.output(publicBroadcastResource)
51+
.errors(possibleErrorsOnFindingResource)
4652
.handler(
4753
async ({ context, input }) =>
4854
await publicGetBroadcast(context.workspace.id, input.idOrName),
@@ -63,6 +69,7 @@ export const broadcastsPublicRouter = {
6369
}),
6470
)
6571
.output(listBroadcastAudienceResponse)
72+
.errors(possibleErrorsOnFindingResource)
6673
.handler(async ({ context, input }) => {
6774
const broadcast = await publicGetBroadcast(
6875
context.workspace.id,

0 commit comments

Comments
 (0)