feat(agent-bff): map agent errors to the BFF error contract#1738
feat(agent-bff): map agent errors to the BFF error contract#1738nbouliol wants to merge 2 commits into
Conversation
Add mapAgentError() translating agent-client AgentHttpError failures into the BFF error envelope, plus the complete registry of BFF-local error factories the Slice-3 endpoints emit. Fixes PRD-670 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 new issue
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
|
|
||
| function mapJsonApiError(agentError: AgentJsonApiError, logger: Logger): BffHttpError { | ||
| const status = agentError.status ?? STATUS_BAD_REQUEST; | ||
| const message = agentError.detail ?? DEFAULT_ERROR_MESSAGE; |
There was a problem hiding this comment.
🟡 Medium http/agent-error-mapper.ts:74
mapJsonApiError reads only agentError.detail for the outgoing message, so when an agent JSON:API error provides message without detail, it returns the generic "Unexpected error" instead of the server-supplied message. Consider falling back to agentError.message before DEFAULT_ERROR_MESSAGE.
| const message = agentError.detail ?? DEFAULT_ERROR_MESSAGE; | |
| const message = agentError.detail ?? agentError.message ?? DEFAULT_ERROR_MESSAGE; |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/agent-bff/src/http/agent-error-mapper.ts around line 74:
`mapJsonApiError` reads only `agentError.detail` for the outgoing message, so when an agent JSON:API error provides `message` without `detail`, it returns the generic `"Unexpected error"` instead of the server-supplied message. Consider falling back to `agentError.message` before `DEFAULT_ERROR_MESSAGE`.
| function mapJsonApiError(agentError: AgentJsonApiError, logger: Logger): BffHttpError { | ||
| const status = agentError.status ?? STATUS_BAD_REQUEST; | ||
| const message = agentError.detail ?? DEFAULT_ERROR_MESSAGE; |
There was a problem hiding this comment.
🟠 High http/agent-error-mapper.ts:72
mapJsonApiError reads status from the JSON:API error object and falls back to 400 when it is missing. When an AgentHttpError carries the status only on the outer error (e.g. new AgentHttpError(403, { errors: [{ name: 'ForbiddenError' }] }, 'Forbidden')), the resulting BffHttpError gets status 400 instead of 403, so the BFF returns the wrong HTTP status to clients. Pass the outer AgentHttpError.status into mapJsonApiError and use it as the fallback instead of 400.
-function mapJsonApiError(agentError: AgentJsonApiError, logger: Logger): BffHttpError {
- const status = agentError.status ?? STATUS_BAD_REQUEST;
+function mapJsonApiError(
+ agentError: AgentJsonApiError,
+ fallbackStatus: number,
+ logger: Logger,
+): BffHttpError {
+ const status = agentError.status ?? fallbackStatus;
const message = agentError.detail ?? DEFAULT_ERROR_MESSAGE;🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/agent-bff/src/http/agent-error-mapper.ts around lines 72-74:
`mapJsonApiError` reads `status` from the JSON:API error object and falls back to `400` when it is missing. When an `AgentHttpError` carries the status only on the outer error (e.g. `new AgentHttpError(403, { errors: [{ name: 'ForbiddenError' }] }, 'Forbidden')`), the resulting `BffHttpError` gets status `400` instead of `403`, so the BFF returns the wrong HTTP status to clients. Pass the outer `AgentHttpError.status` into `mapJsonApiError` and use it as the fallback instead of `400`.
|
Coverage Impact ⬆️ Merging this pull request will increase total coverage on Modified Files with Diff Coverage (2)
🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
- unmapped agent error name now returns 500 mapping_error instead of a silent status fallback - extract the unmapped-name warn out of fallbackTypeByStatus (single responsibility) - use a named constant for the validation_error type - make the name-to-type test use an explicit table instead of deriving expectations from the map under test Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| if (agentError.name !== undefined) { | ||
| const type = AGENT_ERROR_TYPE_MAP[agentError.name]; | ||
|
|
||
| if (type === undefined) { |
There was a problem hiding this comment.
🟠 High http/agent-error-mapper.ts:77
When agentError.name is present but not in AGENT_ERROR_TYPE_MAP, mapJsonApiError returns mappingError(...), which always produces a BffHttpError with status 500 and type mapping_error. This overwrites the agent's original status (e.g. a 4xx) and reports a client error to BFF consumers as an internal server-side mapping failure. Per the PR contract, unknown names should log and fall back to the status-based type using fallbackTypeByStatus(status).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/agent-bff/src/http/agent-error-mapper.ts around line 77:
When `agentError.name` is present but not in `AGENT_ERROR_TYPE_MAP`, `mapJsonApiError` returns `mappingError(...)`, which always produces a `BffHttpError` with status `500` and type `mapping_error`. This overwrites the agent's original status (e.g. a 4xx) and reports a client error to BFF consumers as an internal server-side mapping failure. Per the PR contract, unknown names should log and fall back to the status-based type using `fallbackTypeByStatus(status)`.
1 participant
Context
Slice-3 BFF data/action endpoints (PRD-671..674) need one shared error layer producing the BFF envelope
{ error: { type, status, message, details? } }, so consumers branch onerror.type+error.status, never on a message substring. This ships first and is a hard dependency of every Slice-3 endpoint ticket.What this adds
src/http/agent-error-mapper.ts—mapAgentError(error, { logger }): BffHttpError:AgentHttpError) →network_error(502)agent_unavailable(503, normalized)errors[0]→ type from an explicit name registry (NotFoundError→not_found, …), status fromerrors[0].status,messagefromdetail,detailsfromdataloggerwarn (surfaces the gap without collapsing to a generic type)Errorwhose message is a JSON{ errors: [...] }stringsrc/http/bff-local-errors.ts— the complete registry of BFF-local factories the Slice-3 endpoints emit:unknown_*(404),*_not_allowed(403),invalid_request(400),relation_field_not_supported(422),mapping_error(500),unsupported_action_result(501).package.json— adds@forestadmin/agent-client(the committed BFF→agent transport;AgentHttpErroris the mapper input).Produced
BffHttpErrorinstances flow through the existingerror-middlewareunchanged. No barrel exports: in-package endpoints import relatively.Notes / deviations from the ticket text
The ticket and the global spec were stale on two points; the implementation follows the actual code:
AgentHttpError(notError(JSON.stringify(...))); the string case is kept as a defensive fallback.data, notmeta/source;detailsis sourced fromdata.NotFoundError, notNotFound), so an explicit registry replaces literal snake_case.network_error=502 /agent_unavailable=503 are chosen defaults (unspecified by the spec).Tests
test/http/agent-error-mapper.test.ts— 20 cases (registry coverage, JSON-in-message, transport→502, 5xx→503, flat body→400, unmapped-name warn).test/http/bff-local-errors.test.ts— 11 cases (every factory + details).Full suite: 389/389 pass; build + lint clean.
Fixes PRD-670
🤖 Generated with Claude Code
Note
Map agent errors to the BFF error contract in
agent-bffagent-error-mapper.tswith amapAgentErrorfunction that convertsAgentHttpError, plainError, JSON:API bodies, and flat bodies into typedBffHttpErrorinstances.AGENT_ERROR_TYPE_MAP; unmapped names produce amapping_error(500) and a log entry; server errors (status ≥ 500) normalize toagent_unavailable(503); non-AgentHttpErrorinstances becomenetwork_error(502).bff-local-errors.tswith factory functions for all local BFF error types (404unknown_collection/unknown_relation/unknown_action, 403collection_not_allowedetc., 400invalid_request, 422relation_field_not_supported, 500mapping_error, 501unsupported_action_result).mapping_error(500) rather than propagating the original status code.Macroscope summarized 886cec1.