-
Notifications
You must be signed in to change notification settings - Fork 12
feat(agent-bff): map agent errors to the BFF error contract #1738
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| import type { Logger } from '../ports/logger-port'; | ||
|
|
||
| import { AgentHttpError } from '@forestadmin/agent-client'; | ||
|
|
||
| import { BffHttpError } from './bff-http-error'; | ||
| import { mappingError } from './bff-local-errors'; | ||
|
|
||
| const STATUS_BAD_REQUEST = 400; | ||
| const STATUS_UNAUTHORIZED = 401; | ||
| const STATUS_FORBIDDEN = 403; | ||
| const STATUS_NOT_FOUND = 404; | ||
| const STATUS_UNPROCESSABLE = 422; | ||
| const STATUS_TOO_MANY_REQUESTS = 429; | ||
| const STATUS_SERVER_ERROR = 500; | ||
| const STATUS_BAD_GATEWAY = 502; | ||
| const STATUS_SERVICE_UNAVAILABLE = 503; | ||
|
|
||
| const TYPE_INVALID_REQUEST = 'invalid_request'; | ||
| const TYPE_VALIDATION_ERROR = 'validation_error'; | ||
| const TYPE_UNAUTHORIZED = 'unauthorized'; | ||
| const TYPE_FORBIDDEN = 'forbidden'; | ||
| const TYPE_NOT_FOUND = 'not_found'; | ||
| const TYPE_UNPROCESSABLE = 'unprocessable_entity'; | ||
| const TYPE_TOO_MANY_REQUESTS = 'too_many_requests'; | ||
| const TYPE_NETWORK_ERROR = 'network_error'; | ||
| const TYPE_AGENT_UNAVAILABLE = 'agent_unavailable'; | ||
|
|
||
| const DEFAULT_NETWORK_MESSAGE = 'The agent could not be reached'; | ||
| const DEFAULT_UNAVAILABLE_MESSAGE = 'The agent is unavailable'; | ||
| const DEFAULT_ERROR_MESSAGE = 'Unexpected error'; | ||
|
|
||
| export const AGENT_ERROR_TYPE_MAP: Record<string, string> = { | ||
| ValidationError: TYPE_VALIDATION_ERROR, | ||
| BadRequestError: TYPE_INVALID_REQUEST, | ||
| UnauthorizedError: TYPE_UNAUTHORIZED, | ||
| ForbiddenError: TYPE_FORBIDDEN, | ||
| NotFoundError: TYPE_NOT_FOUND, | ||
| UnprocessableError: TYPE_UNPROCESSABLE, | ||
| TooManyRequestsError: TYPE_TOO_MANY_REQUESTS, | ||
| }; | ||
|
|
||
| const FALLBACK_TYPE_BY_STATUS: Record<number, string> = { | ||
| [STATUS_BAD_REQUEST]: TYPE_INVALID_REQUEST, | ||
| [STATUS_UNAUTHORIZED]: TYPE_UNAUTHORIZED, | ||
| [STATUS_FORBIDDEN]: TYPE_FORBIDDEN, | ||
| [STATUS_NOT_FOUND]: TYPE_NOT_FOUND, | ||
| [STATUS_UNPROCESSABLE]: TYPE_UNPROCESSABLE, | ||
| [STATUS_TOO_MANY_REQUESTS]: TYPE_TOO_MANY_REQUESTS, | ||
| }; | ||
|
|
||
| interface AgentJsonApiError { | ||
| name?: string; | ||
| detail?: string; | ||
| status?: number; | ||
| data?: unknown; | ||
| } | ||
|
|
||
| function fallbackTypeByStatus(status: number): string { | ||
| return FALLBACK_TYPE_BY_STATUS[status] ?? TYPE_INVALID_REQUEST; | ||
| } | ||
|
|
||
| function firstJsonApiError(body: unknown): AgentJsonApiError | undefined { | ||
| if (typeof body !== 'object' || body === null) return undefined; | ||
|
|
||
| const { errors } = body as { errors?: unknown }; | ||
|
|
||
| return Array.isArray(errors) ? (errors[0] as AgentJsonApiError) : undefined; | ||
| } | ||
|
|
||
| function mapJsonApiError(agentError: AgentJsonApiError, logger: Logger): BffHttpError { | ||
| const status = agentError.status ?? STATUS_BAD_REQUEST; | ||
| const message = agentError.detail ?? DEFAULT_ERROR_MESSAGE; | ||
|
Comment on lines
+70
to
+72
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 High
-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: |
||
|
|
||
| if (agentError.name !== undefined) { | ||
| const type = AGENT_ERROR_TYPE_MAP[agentError.name]; | ||
|
|
||
| if (type === undefined) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 High When 🚀 Reply "fix it for me" or copy this AI Prompt for your agent: |
||
| logger('Error', 'Unmapped agent error name', { name: agentError.name, status }); | ||
|
|
||
| return mappingError(`Unmapped agent error name: ${agentError.name}`); | ||
| } | ||
|
|
||
| return new BffHttpError(status, type, message, agentError.data); | ||
| } | ||
|
|
||
| return new BffHttpError(status, fallbackTypeByStatus(status), message, agentError.data); | ||
| } | ||
|
|
||
| function parseJsonApiFromMessage(error: unknown): AgentJsonApiError | undefined { | ||
| if (!(error instanceof Error)) return undefined; | ||
|
|
||
| try { | ||
| return firstJsonApiError(JSON.parse(error.message)); | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| function mapFlatBody(status: number, body: unknown): BffHttpError { | ||
| const flat = (typeof body === 'object' && body !== null ? body : {}) as { | ||
| error?: unknown; | ||
| message?: unknown; | ||
| }; | ||
| const message = | ||
| (typeof flat.error === 'string' ? flat.error : undefined) ?? | ||
| (typeof flat.message === 'string' ? flat.message : undefined) ?? | ||
| DEFAULT_ERROR_MESSAGE; | ||
|
|
||
| return new BffHttpError(status, fallbackTypeByStatus(status), message); | ||
| } | ||
|
|
||
| export function mapAgentError(error: unknown, { logger }: { logger: Logger }): BffHttpError { | ||
| if (!(error instanceof AgentHttpError)) { | ||
| const agentError = parseJsonApiFromMessage(error); | ||
| if (agentError) return mapJsonApiError(agentError, logger); | ||
|
|
||
| const message = error instanceof Error ? error.message : DEFAULT_NETWORK_MESSAGE; | ||
|
|
||
| return new BffHttpError(STATUS_BAD_GATEWAY, TYPE_NETWORK_ERROR, message); | ||
| } | ||
|
|
||
| if (error.status >= STATUS_SERVER_ERROR) { | ||
| return new BffHttpError( | ||
| STATUS_SERVICE_UNAVAILABLE, | ||
| TYPE_AGENT_UNAVAILABLE, | ||
| DEFAULT_UNAVAILABLE_MESSAGE, | ||
| ); | ||
| } | ||
|
|
||
| const agentError = firstJsonApiError(error.body); | ||
| if (agentError) return mapJsonApiError(agentError, logger); | ||
|
|
||
| return mapFlatBody(error.status, error.body); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import { BffHttpError } from './bff-http-error'; | ||
|
|
||
| export function unknownCollection(message = 'Unknown collection'): BffHttpError { | ||
| return new BffHttpError(404, 'unknown_collection', message); | ||
| } | ||
|
|
||
| export function unknownRelation(message = 'Unknown relation'): BffHttpError { | ||
| return new BffHttpError(404, 'unknown_relation', message); | ||
| } | ||
|
|
||
| export function unknownAction(message = 'Unknown action'): BffHttpError { | ||
| return new BffHttpError(404, 'unknown_action', message); | ||
| } | ||
|
|
||
| export function collectionNotAllowed(message = 'Collection is not allowed'): BffHttpError { | ||
| return new BffHttpError(403, 'collection_not_allowed', message); | ||
| } | ||
|
|
||
| export function relationNotAllowed(message = 'Relation is not allowed'): BffHttpError { | ||
| return new BffHttpError(403, 'relation_not_allowed', message); | ||
| } | ||
|
|
||
| export function actionNotAllowed(message = 'Action is not allowed'): BffHttpError { | ||
| return new BffHttpError(403, 'action_not_allowed', message); | ||
| } | ||
|
|
||
| export function invalidRequest(message = 'Invalid request', details?: unknown): BffHttpError { | ||
| return new BffHttpError(400, 'invalid_request', message, details); | ||
| } | ||
|
|
||
| export function relationFieldNotSupported( | ||
| message = 'Relation field is not supported', | ||
| ): BffHttpError { | ||
| return new BffHttpError(422, 'relation_field_not_supported', message); | ||
| } | ||
|
|
||
| export function mappingError(message = 'Failed to map the agent response'): BffHttpError { | ||
| return new BffHttpError(500, 'mapping_error', message); | ||
| } | ||
|
|
||
| export function unsupportedActionResult(message = 'Unsupported action result'): BffHttpError { | ||
| return new BffHttpError(501, 'unsupported_action_result', message); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| import { AgentHttpError } from '@forestadmin/agent-client'; | ||
|
|
||
| import { mapAgentError } from '../../src/http/agent-error-mapper'; | ||
|
|
||
| function jsonApiBody(error: { name?: string; detail?: string; status?: number; data?: unknown }) { | ||
| return { errors: [error] }; | ||
| } | ||
|
|
||
| describe('mapAgentError', () => { | ||
| let logger: jest.Mock; | ||
|
|
||
| beforeEach(() => { | ||
| logger = jest.fn(); | ||
| }); | ||
|
|
||
| it('maps a JSON:API NotFoundError to not_found with its detail and data', () => { | ||
| const error = new AgentHttpError( | ||
| 404, | ||
| jsonApiBody({ name: 'NotFoundError', status: 404, detail: 'x', data: { id: 1 } }), | ||
| ); | ||
|
|
||
| const result = mapAgentError(error, { logger }); | ||
|
|
||
| expect(result).toMatchObject({ | ||
| type: 'not_found', | ||
| status: 404, | ||
| message: 'x', | ||
| details: { id: 1 }, | ||
| }); | ||
| }); | ||
|
|
||
| it.each([ | ||
| ['ValidationError', 'validation_error'], | ||
| ['BadRequestError', 'invalid_request'], | ||
| ['UnauthorizedError', 'unauthorized'], | ||
| ['ForbiddenError', 'forbidden'], | ||
| ['NotFoundError', 'not_found'], | ||
| ['UnprocessableError', 'unprocessable_entity'], | ||
| ['TooManyRequestsError', 'too_many_requests'], | ||
| ])('maps agent name %s to type %s', (name, type) => { | ||
| const status = 422; | ||
| const error = new AgentHttpError(status, jsonApiBody({ name, status, detail: 'd' })); | ||
|
|
||
| const result = mapAgentError(error, { logger }); | ||
|
|
||
| expect(result).toMatchObject({ type, status }); | ||
| }); | ||
|
|
||
| it('parses a JSON:API body carried in a plain Error message', () => { | ||
| const error = new Error( | ||
| JSON.stringify(jsonApiBody({ name: 'ForbiddenError', status: 403, detail: 'nope' })), | ||
| ); | ||
|
|
||
| const result = mapAgentError(error, { logger }); | ||
|
|
||
| expect(result).toMatchObject({ type: 'forbidden', status: 403, message: 'nope' }); | ||
| }); | ||
|
|
||
| it('maps a transport failure to network_error (502)', () => { | ||
| const result = mapAgentError(new Error('ECONNREFUSED'), { logger }); | ||
|
|
||
| expect(result).toMatchObject({ type: 'network_error', status: 502 }); | ||
| }); | ||
|
|
||
| it('normalizes an agent 500 to agent_unavailable (503)', () => { | ||
| const result = mapAgentError(new AgentHttpError(500, {}), { logger }); | ||
|
|
||
| expect(result).toMatchObject({ type: 'agent_unavailable', status: 503 }); | ||
| }); | ||
|
|
||
| it('normalizes an agent 503 to agent_unavailable (503)', () => { | ||
| const result = mapAgentError(new AgentHttpError(503, {}), { logger }); | ||
|
|
||
| expect(result).toMatchObject({ type: 'agent_unavailable', status: 503 }); | ||
| }); | ||
|
|
||
| it('maps a flat native-action body to invalid_request (400) using body.error', () => { | ||
| const result = mapAgentError(new AgentHttpError(400, { error: 'boom' }), { logger }); | ||
|
|
||
| expect(result).toMatchObject({ type: 'invalid_request', status: 400, message: 'boom' }); | ||
| }); | ||
|
|
||
| it('maps an unmapped agent error name to mapping_error (500) and logs it', () => { | ||
| const error = new AgentHttpError( | ||
| 400, | ||
| jsonApiBody({ name: 'MissingCollectionError', status: 400, detail: 'gone' }), | ||
| ); | ||
|
|
||
| const result = mapAgentError(error, { logger }); | ||
|
|
||
| expect(result).toMatchObject({ type: 'mapping_error', status: 500 }); | ||
| expect(logger).toHaveBeenCalledWith( | ||
| 'Error', | ||
| expect.any(String), | ||
| expect.objectContaining({ name: 'MissingCollectionError' }), | ||
| ); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| import { | ||
| actionNotAllowed, | ||
| collectionNotAllowed, | ||
| invalidRequest, | ||
| mappingError, | ||
| relationFieldNotSupported, | ||
| relationNotAllowed, | ||
| unknownAction, | ||
| unknownCollection, | ||
| unknownRelation, | ||
| unsupportedActionResult, | ||
| } from '../../src/http/bff-local-errors'; | ||
|
|
||
| describe('bff local errors', () => { | ||
| it.each([ | ||
| [unknownCollection, 'unknown_collection', 404], | ||
| [unknownRelation, 'unknown_relation', 404], | ||
| [unknownAction, 'unknown_action', 404], | ||
| [collectionNotAllowed, 'collection_not_allowed', 403], | ||
| [relationNotAllowed, 'relation_not_allowed', 403], | ||
| [actionNotAllowed, 'action_not_allowed', 403], | ||
| [invalidRequest, 'invalid_request', 400], | ||
| [relationFieldNotSupported, 'relation_field_not_supported', 422], | ||
| [mappingError, 'mapping_error', 500], | ||
| [unsupportedActionResult, 'unsupported_action_result', 501], | ||
| ])('%p builds a %s error with status %d', (factory, type, status) => { | ||
| expect(factory()).toMatchObject({ type, status }); | ||
| }); | ||
|
|
||
| it('carries details on invalidRequest', () => { | ||
| expect(invalidRequest('bad', { field: 'x' })).toMatchObject({ | ||
| type: 'invalid_request', | ||
| status: 400, | ||
| details: { field: 'x' }, | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Medium
http/agent-error-mapper.ts:74mapJsonApiErrorreads onlyagentError.detailfor the outgoing message, so when an agent JSON:API error providesmessagewithoutdetail, it returns the generic"Unexpected error"instead of the server-supplied message. Consider falling back toagentError.messagebeforeDEFAULT_ERROR_MESSAGE.🚀 Reply "fix it for me" or copy this AI Prompt for your agent: