From 760e4e81d09dc136d737cd334702c42baddaf40f Mon Sep 17 00:00:00 2001 From: Charlie Kolb Date: Wed, 30 Sep 2026 10:31:37 +0000 Subject: [PATCH] refactor(core): Move REST error handling to backend services (no-changelog) (#39907) --- packages/@n8n/backend-services/package.json | 2 + .../__tests__/rest-error-classifier.test.ts | 149 ++++++++++++++++++ .../__tests__/rest-error-response.test.ts | 89 +++++++++++ .../src/errors/rest-error-classifier.ts} | 69 +++++--- .../src/errors/rest-error-response.ts} | 26 +-- packages/@n8n/backend-services/src/index.ts | 12 ++ .../__tests__/http-error-classifier.test.ts | 122 -------------- .../__tests__/rest-error-classifier.test.ts | 50 ++++++ ...rs.test.ts => rest-error-response.test.ts} | 105 +----------- .../__tests__/policy-decision.service.test.ts | 5 +- .../__tests__/policy-violation.error.test.ts | 12 +- .../cli/src/policy/policy-violation.error.ts | 2 +- .../v1/public-api-error-response.ts | 12 +- packages/cli/src/response-helper.ts | 9 +- pnpm-lock.yaml | 24 +++ 15 files changed, 418 insertions(+), 270 deletions(-) create mode 100644 packages/@n8n/backend-services/src/errors/__tests__/rest-error-classifier.test.ts create mode 100644 packages/@n8n/backend-services/src/errors/__tests__/rest-error-response.test.ts rename packages/{cli/src/errors/http-error-classifier.ts => @n8n/backend-services/src/errors/rest-error-classifier.ts} (52%) rename packages/{cli/src/errors/http-error-serializers.ts => @n8n/backend-services/src/errors/rest-error-response.ts} (81%) delete mode 100644 packages/cli/src/errors/__tests__/http-error-classifier.test.ts create mode 100644 packages/cli/src/errors/__tests__/rest-error-classifier.test.ts rename packages/cli/src/errors/__tests__/{http-error-serializers.test.ts => rest-error-response.test.ts} (54%) diff --git a/packages/@n8n/backend-services/package.json b/packages/@n8n/backend-services/package.json index f481d943421..7f8ee7e6a18 100644 --- a/packages/@n8n/backend-services/package.json +++ b/packages/@n8n/backend-services/package.json @@ -23,6 +23,7 @@ "dist/**/*" ], "dependencies": { + "@n8n/api-types": "workspace:*", "@n8n/backend-common": "workspace:*", "@n8n/config": "workspace:*", "@n8n/constants": "workspace:*", @@ -31,6 +32,7 @@ "@n8n/errors": "workspace:*", "@n8n/utils": "workspace:*", "cache-manager": "catalog:", + "express-openapi-validator": "5.5.3", "ioredis": "5.3.2", "n8n-workflow": "workspace:*" }, diff --git a/packages/@n8n/backend-services/src/errors/__tests__/rest-error-classifier.test.ts b/packages/@n8n/backend-services/src/errors/__tests__/rest-error-classifier.test.ts new file mode 100644 index 00000000000..6ffe8fea5f1 --- /dev/null +++ b/packages/@n8n/backend-services/src/errors/__tests__/rest-error-classifier.test.ts @@ -0,0 +1,149 @@ +import { BadRequest, Unauthorized } from 'express-openapi-validator/dist/framework/types'; + +import { + BadRequestError, + NotFoundError, + OperationalError, + UnexpectedError, + UserError, +} from '@n8n/errors'; + +import { classifyRestError, RestErrorKind, isResponseError } from '../rest-error-classifier'; + +describe('classifyRestError', () => { + it('tags ResponseError with kind responseError and http fields', () => { + const d = classifyRestError(new NotFoundError('missing')); + expect(d).toEqual({ + kind: RestErrorKind.responseError, + status: 404, + message: 'missing', + code: 404, + }); + }); + + it('tags UserError without HTTP status (serializers assign status)', () => { + const d = classifyRestError(new UserError('bad input')); + expect(d).toEqual({ + kind: RestErrorKind.userError, + message: 'bad input', + }); + }); + + it('tags n8n UnexpectedError', () => { + const d = classifyRestError(new UnexpectedError('internal bug')); + expect(d).toEqual({ + kind: RestErrorKind.unexpectedError, + message: 'internal bug', + }); + }); + + it('tags OperationalError as generic serverError', () => { + const d = classifyRestError(new OperationalError('temporarily down')); + expect(d).toEqual({ + kind: RestErrorKind.serverError, + message: 'temporarily down', + }); + }); + + it('tags express-openapi-validator HttpError', () => { + const httpError = new BadRequest({ path: '/x', message: 'schema failed' }); + const d = classifyRestError(httpError); + expect(d).toEqual({ + kind: RestErrorKind.httpError, + status: 400, + message: 'schema failed', + }); + }); + + describe('Unauthorized', () => { + it('keeps the express-openapi-validator message when no session cookie was sent', () => { + const unauthorizedError = new Unauthorized({ + path: '/x', + message: "'X-N8N-API-KEY' header required", + }); + const d = classifyRestError(unauthorizedError); + expect(d).toEqual({ + kind: RestErrorKind.httpError, + status: 401, + message: "'X-N8N-API-KEY' header required", + }); + }); + + it('replaces the api key hint with a generic message when a session cookie was sent', () => { + const unauthorizedError = new Unauthorized({ + path: '/x', + message: "'X-N8N-API-KEY' header required", + }); + const d = classifyRestError(unauthorizedError, { hasSessionCookie: true }); + expect(d).toEqual({ + kind: RestErrorKind.httpError, + status: 401, + message: 'Unauthorized', + }); + }); + }); + + it('does not classify an unrelated error with an HTTP error class name', () => { + class NotFound extends Error {} + + const d = classifyRestError(new NotFound('internal error')); + expect(d).toEqual({ + kind: RestErrorKind.serverError, + message: 'internal error', + }); + }); + + it.each([ + ['HttpError', 400], + ['NotFound', 404], + ['NotAcceptable', 406], + ['MethodNotAllowed', 405], + ['BadRequest', 400], + ['RequestEntityTooLarge', 413], + ['InternalServerError', 500], + ['UnsupportedMediaType', 415], + ['Unauthorized', 401], + ['Forbidden', 403], + ])('recognizes a validator %s error from another package instance', (className, status) => { + const CrossPackageError = { + [className]: class extends Error { + readonly status = status; + }, + }[className]; + const error = new CrossPackageError('validator error'); + + expect(classifyRestError(error)).toEqual({ + kind: RestErrorKind.httpError, + status, + message: 'validator error', + }); + }); + + it('tags plain Error as serverError', () => { + const d = classifyRestError(new Error('plain')); + expect(d).toEqual({ + kind: RestErrorKind.serverError, + message: 'plain', + }); + }); + + it('matches BadRequestError fields', () => { + const d = classifyRestError(new BadRequestError('invalid')); + expect(d).toEqual({ + kind: RestErrorKind.responseError, + status: 400, + message: 'invalid', + code: 400, + }); + }); +}); + +describe('isResponseError', () => { + it('recognizes duck-typed hook errors', () => { + const hookError = Object.assign(new Error('hook'), { + httpStatusCode: 403, + errorCode: 403, + }); + expect(isResponseError(hookError)).toBe(true); + }); +}); diff --git a/packages/@n8n/backend-services/src/errors/__tests__/rest-error-response.test.ts b/packages/@n8n/backend-services/src/errors/__tests__/rest-error-response.test.ts new file mode 100644 index 00000000000..b5095d6f11d --- /dev/null +++ b/packages/@n8n/backend-services/src/errors/__tests__/rest-error-response.test.ts @@ -0,0 +1,89 @@ +import type { SourceControlledFile } from '@n8n/api-types'; + +import { ConflictError, NotFoundError, UnexpectedError, UserError } from '@n8n/errors'; + +import { classifyRestError } from '../rest-error-classifier'; +import { serializeInternalRestError, serializePublicApiError } from '../rest-error-response'; + +describe('rest-error-response', () => { + it('serializes a ResponseError with a minimal public message', () => { + const descriptor = classifyRestError(new NotFoundError('x')); + expect(serializePublicApiError(descriptor)).toEqual({ + status: 404, + body: { message: 'x' }, + }); + }); + + it('serializes a ResponseError with its code for internal requests', () => { + const descriptor = classifyRestError(new NotFoundError('x')); + expect(serializeInternalRestError(descriptor)).toEqual({ + status: 404, + body: { + code: 404, + message: 'x', + }, + }); + }); + + it('serializes source control conflicts in both response formats', () => { + const conflicts: SourceControlledFile[] = [ + { + file: 'workflows/wf-1.json', + id: 'wf-1', + name: 'My workflow', + type: 'workflow', + status: 'modified', + location: 'local', + conflict: true, + updatedAt: '2024-01-01T00:00:00.000Z', + }, + ]; + const descriptor = classifyRestError( + new ConflictError( + 'Push blocked by conflicting files. Pass `force: true` to push anyway.', + undefined, + { conflicts }, + ), + ); + + expect(serializePublicApiError(descriptor)).toEqual({ + status: 409, + body: { + message: expect.stringContaining('conflicting files'), + conflicts, + }, + }); + expect(serializeInternalRestError(descriptor)).toEqual({ + status: 409, + body: { + code: 409, + message: expect.stringContaining('conflicting files'), + meta: { conflicts }, + }, + }); + }); + + it('maps UserError to 400', () => { + const descriptor = classifyRestError(new UserError('bad input')); + expect(serializePublicApiError(descriptor)).toEqual({ + status: 400, + body: { message: 'bad input' }, + }); + expect(serializeInternalRestError(descriptor)).toEqual({ + status: 400, + body: { code: 0, message: 'bad input' }, + }); + }); + + it('sanitizes UnexpectedError for public responses', () => { + const descriptor = classifyRestError(new UnexpectedError('secret')); + expect(serializePublicApiError(descriptor)).toEqual({ + status: 500, + body: { message: 'Internal server error' }, + }); + expect(serializeInternalRestError(descriptor)).toEqual({ + status: 500, + body: { code: 0, message: 'secret' }, + }); + }); +}); diff --git a/packages/cli/src/errors/http-error-classifier.ts b/packages/@n8n/backend-services/src/errors/rest-error-classifier.ts similarity index 52% rename from packages/cli/src/errors/http-error-classifier.ts rename to packages/@n8n/backend-services/src/errors/rest-error-classifier.ts index b5f9a94b6f9..08733fe62da 100644 --- a/packages/cli/src/errors/http-error-classifier.ts +++ b/packages/@n8n/backend-services/src/errors/rest-error-classifier.ts @@ -2,7 +2,7 @@ import { HttpError, Unauthorized } from 'express-openapi-validator/dist/framewor import { ResponseError, UnexpectedError, UserError } from '@n8n/errors'; -export const enum HttpErrorKind { +export const enum RestErrorKind { responseError = 'responseError', userError = 'userError', unexpectedError = 'unexpectedError', @@ -10,9 +10,9 @@ export const enum HttpErrorKind { serverError = 'serverError', } -export type HttpErrorDescriptor = +export type RestErrorDescriptor = | { - kind: HttpErrorKind.responseError; + kind: RestErrorKind.responseError; status: number; message: string; code: number; @@ -20,20 +20,20 @@ export type HttpErrorDescriptor = meta?: Record; } | { - kind: HttpErrorKind.userError; + kind: RestErrorKind.userError; message: string; } | { - kind: HttpErrorKind.unexpectedError; + kind: RestErrorKind.unexpectedError; message: string; } | { - kind: HttpErrorKind.httpError; + kind: RestErrorKind.httpError; status: number; message: string; } | { - kind: HttpErrorKind.serverError; + kind: RestErrorKind.serverError; message: string; }; @@ -54,18 +54,18 @@ export function isResponseError(error: Error): error is ResponseError { return false; } -export type HttpErrorClassifierContext = { +export type RestErrorClassifierContext = { /** Whether the request carried a session cookie, regardless of whether it was valid. */ hasSessionCookie?: boolean; }; -export function classifyHttpError( +export function classifyRestError( error: Error, - context?: HttpErrorClassifierContext, -): HttpErrorDescriptor { + context?: RestErrorClassifierContext, +): RestErrorDescriptor { if (isResponseError(error)) { - const descriptor: HttpErrorDescriptor & { kind: HttpErrorKind.responseError } = { - kind: HttpErrorKind.responseError, + const descriptor: RestErrorDescriptor & { kind: RestErrorKind.responseError } = { + kind: RestErrorKind.responseError, status: error.httpStatusCode, message: error.message ?? 'Unknown error', code: error.errorCode, @@ -80,26 +80,55 @@ export function classifyHttpError( } if (error instanceof UserError) { - return { kind: HttpErrorKind.userError, message: error.message }; + return { kind: RestErrorKind.userError, message: error.message }; } if (error instanceof UnexpectedError) { - return { kind: HttpErrorKind.unexpectedError, message: error.message }; + return { kind: RestErrorKind.unexpectedError, message: error.message }; } // Replacing express-openapi-validator's 'api key header required' error message // for requests that contained a session cookie. - if (error instanceof Unauthorized && context?.hasSessionCookie) { - return { kind: HttpErrorKind.httpError, status: 401, message: 'Unauthorized' }; + if (isHttpErrorInstance(error, Unauthorized, ['Unauthorized']) && context?.hasSessionCookie) { + return { kind: RestErrorKind.httpError, status: 401, message: 'Unauthorized' }; } - if (error instanceof HttpError) { + if ( + isHttpErrorInstance(error, HttpError, [ + 'HttpError', + 'NotFound', + 'NotAcceptable', + 'MethodNotAllowed', + 'BadRequest', + 'RequestEntityTooLarge', + 'InternalServerError', + 'UnsupportedMediaType', + 'Unauthorized', + 'Forbidden', + ]) + ) { return { - kind: HttpErrorKind.httpError, + kind: RestErrorKind.httpError, status: error.status || 400, message: error.message || 'Bad request', }; } - return { kind: HttpErrorKind.serverError, message: error.message ?? 'Unknown error' }; + return { kind: RestErrorKind.serverError, message: error.message ?? 'Unknown error' }; +} + +function isHttpErrorInstance( + error: Error, + errorClass: new (...args: never[]) => T, + classNames: string[], +): error is T { + if (error instanceof errorClass) { + return true; + } + + return classNames.includes(error.constructor.name) && hasHttpStatus(error); +} + +function hasHttpStatus(error: Error): error is Error & { status: number } { + return 'status' in error && typeof error.status === 'number'; } diff --git a/packages/cli/src/errors/http-error-serializers.ts b/packages/@n8n/backend-services/src/errors/rest-error-response.ts similarity index 81% rename from packages/cli/src/errors/http-error-serializers.ts rename to packages/@n8n/backend-services/src/errors/rest-error-response.ts index f8dcad37674..d21c5e74494 100644 --- a/packages/cli/src/errors/http-error-serializers.ts +++ b/packages/@n8n/backend-services/src/errors/rest-error-response.ts @@ -1,6 +1,6 @@ import { isWorkflowPublishBlockedDetails } from '@n8n/api-types'; -import { HttpErrorKind, type HttpErrorDescriptor } from '@/errors/http-error-classifier'; +import { RestErrorKind, type RestErrorDescriptor } from './rest-error-classifier'; const GENERIC_PUBLIC_MESSAGE = 'Internal server error'; @@ -12,12 +12,12 @@ export type InternalRestErrorBody = { meta?: Record; }; -export function serializePublicApiError(descriptor: HttpErrorDescriptor): { +export function serializePublicApiError(descriptor: RestErrorDescriptor): { status: number; body: { message: string }; } { switch (descriptor.kind) { - case HttpErrorKind.responseError: { + case RestErrorKind.responseError: { const body: { message: string } & Record = { message: descriptor.message, }; @@ -50,18 +50,18 @@ export function serializePublicApiError(descriptor: HttpErrorDescriptor): { }, }; } - case HttpErrorKind.userError: + case RestErrorKind.userError: return { status: 400, body: { message: descriptor.message }, }; - case HttpErrorKind.unexpectedError: - case HttpErrorKind.serverError: + case RestErrorKind.unexpectedError: + case RestErrorKind.serverError: return { status: 500, body: { message: GENERIC_PUBLIC_MESSAGE }, }; - case HttpErrorKind.httpError: + case RestErrorKind.httpError: return { status: descriptor.status, body: { message: descriptor.message }, @@ -69,12 +69,12 @@ export function serializePublicApiError(descriptor: HttpErrorDescriptor): { } } -export function serializeInternalRestError(descriptor: HttpErrorDescriptor): { +export function serializeInternalRestError(descriptor: RestErrorDescriptor): { status: number; body: InternalRestErrorBody; } { switch (descriptor.kind) { - case HttpErrorKind.responseError: { + case RestErrorKind.responseError: { const body: InternalRestErrorBody = { code: descriptor.code, message: descriptor.message, @@ -87,18 +87,18 @@ export function serializeInternalRestError(descriptor: HttpErrorDescriptor): { } return { status: descriptor.status, body }; } - case HttpErrorKind.userError: + case RestErrorKind.userError: return { status: 400, body: { code: 0, message: descriptor.message }, }; - case HttpErrorKind.unexpectedError: - case HttpErrorKind.serverError: + case RestErrorKind.unexpectedError: + case RestErrorKind.serverError: return { status: 500, body: { code: 0, message: descriptor.message }, }; - case HttpErrorKind.httpError: + case RestErrorKind.httpError: return { status: descriptor.status, body: { code: 0, message: descriptor.message }, diff --git a/packages/@n8n/backend-services/src/index.ts b/packages/@n8n/backend-services/src/index.ts index b7e636552f0..f85a6a73697 100644 --- a/packages/@n8n/backend-services/src/index.ts +++ b/packages/@n8n/backend-services/src/index.ts @@ -4,3 +4,15 @@ export { CacheService } from './services/cache/cache.service'; export { RedisClientService } from './services/redis-client.service'; export type { RedisClientType } from './services/redis.types'; export { UrlService } from './services/url.service'; +export { + classifyRestError, + isResponseError, + type RestErrorClassifierContext, + type RestErrorDescriptor, + RestErrorKind, +} from './errors/rest-error-classifier'; +export { + serializeInternalRestError, + serializePublicApiError, + type InternalRestErrorBody, +} from './errors/rest-error-response'; diff --git a/packages/cli/src/errors/__tests__/http-error-classifier.test.ts b/packages/cli/src/errors/__tests__/http-error-classifier.test.ts deleted file mode 100644 index 7899c1be2ff..00000000000 --- a/packages/cli/src/errors/__tests__/http-error-classifier.test.ts +++ /dev/null @@ -1,122 +0,0 @@ -import { BadRequest, Unauthorized } from 'express-openapi-validator/dist/framework/types'; - -import { - BadRequestError, - NotFoundError, - OperationalError, - UnexpectedError, - UserError, -} from '@n8n/errors'; -import { LicenseEulaRequiredError } from '@/errors/response-errors/license-eula-required.error'; - -import { classifyHttpError, HttpErrorKind, isResponseError } from '../http-error-classifier'; - -describe('classifyHttpError', () => { - it('tags ResponseError with kind responseError and http fields', () => { - const d = classifyHttpError(new NotFoundError('missing')); - expect(d).toEqual({ - kind: HttpErrorKind.responseError, - status: 404, - message: 'missing', - code: 404, - }); - }); - - it('tags UserError without HTTP status (serializers assign status)', () => { - const d = classifyHttpError(new UserError('bad input')); - expect(d).toEqual({ - kind: HttpErrorKind.userError, - message: 'bad input', - }); - }); - - it('tags n8n UnexpectedError', () => { - const d = classifyHttpError(new UnexpectedError('internal bug')); - expect(d).toEqual({ - kind: HttpErrorKind.unexpectedError, - message: 'internal bug', - }); - }); - - it('tags OperationalError as generic serverError', () => { - const d = classifyHttpError(new OperationalError('temporarily down')); - expect(d).toEqual({ - kind: HttpErrorKind.serverError, - message: 'temporarily down', - }); - }); - - it('tags express-openapi-validator HttpError', () => { - const err = new BadRequest({ path: '/x', message: 'schema failed' }); - const d = classifyHttpError(err); - expect(d).toEqual({ - kind: HttpErrorKind.httpError, - status: 400, - message: 'schema failed', - }); - }); - - describe('Unauthorized', () => { - it('keeps the express-openapi-validator message when no session cookie was sent', () => { - const err = new Unauthorized({ path: '/x', message: "'X-N8N-API-KEY' header required" }); - const d = classifyHttpError(err); - expect(d).toEqual({ - kind: HttpErrorKind.httpError, - status: 401, - message: "'X-N8N-API-KEY' header required", - }); - }); - - it('replaces the api key hint with a generic message when a session cookie was sent', () => { - const err = new Unauthorized({ path: '/x', message: "'X-N8N-API-KEY' header required" }); - const d = classifyHttpError(err, { hasSessionCookie: true }); - expect(d).toEqual({ - kind: HttpErrorKind.httpError, - status: 401, - message: 'Unauthorized', - }); - }); - }); - - it('tags plain Error as serverError', () => { - const d = classifyHttpError(new Error('plain')); - expect(d).toEqual({ - kind: HttpErrorKind.serverError, - message: 'plain', - }); - }); - - it('includes meta for LicenseEulaRequiredError', () => { - const eulaUrl = 'https://n8n.io/legal/eula/'; - const d = classifyHttpError( - new LicenseEulaRequiredError('License activation requires EULA acceptance', { - eulaUrl, - }), - ); - expect(d.kind).toBe(HttpErrorKind.responseError); - if (d.kind === HttpErrorKind.responseError) { - expect(d.status).toBe(400); - expect(d.meta).toEqual({ eulaUrl }); - } - }); - - it('matches BadRequestError fields', () => { - const d = classifyHttpError(new BadRequestError('invalid')); - expect(d).toEqual({ - kind: HttpErrorKind.responseError, - status: 400, - message: 'invalid', - code: 400, - }); - }); -}); - -describe('isResponseError', () => { - it('recognizes duck-typed hook errors', () => { - const err = Object.assign(new Error('hook'), { - httpStatusCode: 403, - errorCode: 403, - }); - expect(isResponseError(err)).toBe(true); - }); -}); diff --git a/packages/cli/src/errors/__tests__/rest-error-classifier.test.ts b/packages/cli/src/errors/__tests__/rest-error-classifier.test.ts new file mode 100644 index 00000000000..1f7188ff9c0 --- /dev/null +++ b/packages/cli/src/errors/__tests__/rest-error-classifier.test.ts @@ -0,0 +1,50 @@ +import { LicenseEulaRequiredError } from '@/errors/response-errors/license-eula-required.error'; +import { classifyRestError, RestErrorKind } from '@n8n/backend-services'; +import { + BadRequest, + Forbidden, + InternalServerError, + MethodNotAllowed, + NotAcceptable, + NotFound, + RequestEntityTooLarge, + Unauthorized, + UnsupportedMediaType, +} from 'express-openapi-validator/dist/framework/types'; + +describe('classifyRestError', () => { + it('includes meta for LicenseEulaRequiredError', () => { + const eulaUrl = 'https://n8n.io/legal/eula/'; + const descriptor = classifyRestError( + new LicenseEulaRequiredError('License activation requires EULA acceptance', { + eulaUrl, + }), + ); + + expect(descriptor.kind).toBe(RestErrorKind.responseError); + if (descriptor.kind === RestErrorKind.responseError) { + expect(descriptor.status).toBe(400); + expect(descriptor.meta).toEqual({ eulaUrl }); + } + }); + + it.each([ + [NotFound, 404], + [NotAcceptable, 406], + [MethodNotAllowed, 405], + [BadRequest, 400], + [RequestEntityTooLarge, 413], + [InternalServerError, 500], + [UnsupportedMediaType, 415], + [Unauthorized, 401], + [Forbidden, 403], + ])('classifies a %s from the CLI validator package', (ErrorClass, status) => { + const error = new ErrorClass({ path: '/x', message: 'validator error' }); + + expect(classifyRestError(error)).toEqual({ + kind: RestErrorKind.httpError, + status, + message: 'validator error', + }); + }); +}); diff --git a/packages/cli/src/errors/__tests__/http-error-serializers.test.ts b/packages/cli/src/errors/__tests__/rest-error-response.test.ts similarity index 54% rename from packages/cli/src/errors/__tests__/http-error-serializers.test.ts rename to packages/cli/src/errors/__tests__/rest-error-response.test.ts index bdafd2ef7e7..5b54a2f724b 100644 --- a/packages/cli/src/errors/__tests__/http-error-serializers.test.ts +++ b/packages/cli/src/errors/__tests__/rest-error-response.test.ts @@ -1,38 +1,13 @@ -import type { SourceControlledFile } from '@n8n/api-types'; - -import { classifyHttpError } from '@/errors/http-error-classifier'; -import { - serializeInternalRestError, - serializePublicApiError, -} from '@/errors/http-error-serializers'; -import { ConflictError, NotFoundError, UnexpectedError, UserError } from '@n8n/errors'; +import { classifyRestError } from '@n8n/backend-services'; +import { serializeInternalRestError, serializePublicApiError } from '@n8n/backend-services'; import { LicenseEulaRequiredError } from '@/errors/response-errors/license-eula-required.error'; import { WorkflowPublishBlockedError } from '@/errors/response-errors/workflow-publish-blocked.error'; import { toImportBlockedError } from '@/modules/n8n-packages/engine/import-blocked.error'; import { PolicyViolationError } from '@/policy/policy-violation.error'; -describe('http-error-serializers', () => { - it('serializePublicApiError: minimal message for ResponseError', () => { - const descriptor = classifyHttpError(new NotFoundError('x')); - expect(serializePublicApiError(descriptor)).toEqual({ - status: 404, - body: { message: 'x' }, - }); - }); - - it('serializeInternalRestError: includes code for ResponseError', () => { - const descriptor = classifyHttpError(new NotFoundError('x')); - expect(serializeInternalRestError(descriptor)).toEqual({ - status: 404, - body: { - code: 404, - message: 'x', - }, - }); - }); - +describe('rest-error-response', () => { it('serializePublicApiError: does not expose internal-only response error meta', () => { - const descriptor = classifyHttpError( + const descriptor = classifyRestError( new LicenseEulaRequiredError('License activation requires EULA acceptance', { eulaUrl: 'https://n8n.io/legal/eula/', }), @@ -52,7 +27,7 @@ describe('http-error-serializers', () => { }); it('returns review details publicly while keeping editor-only validation metadata internal', () => { - const descriptor = classifyHttpError( + const descriptor = classifyRestError( new WorkflowPublishBlockedError({ reason: 'changes_requested', workflowReviewRequestId: 'review-1', @@ -90,7 +65,7 @@ describe('http-error-serializers', () => { usedByWorkflows: ['wf-1'], }, ]; - const descriptor = classifyHttpError(toImportBlockedError(issues)); + const descriptor = classifyRestError(toImportBlockedError(issues)); const result = serializePublicApiError(descriptor); expect(result.status).toBe(422); @@ -112,7 +87,7 @@ describe('http-error-serializers', () => { usedByWorkflows: ['wf-1'], }, ]; - const descriptor = classifyHttpError(toImportBlockedError(issues)); + const descriptor = classifyRestError(toImportBlockedError(issues)); const result = serializePublicApiError(descriptor); expect(result.status).toBe(409); @@ -129,7 +104,7 @@ describe('http-error-serializers', () => { subjectType: 'nodeType', }, ]; - const descriptor = classifyHttpError(new PolicyViolationError([violations[0]])); + const descriptor = classifyRestError(new PolicyViolationError([violations[0]])); expect(serializePublicApiError(descriptor)).toEqual({ status: 403, @@ -147,68 +122,4 @@ describe('http-error-serializers', () => { }, }); }); - - it('both serializers expose source control push conflicts on a 409', () => { - const conflicts: SourceControlledFile[] = [ - { - file: 'workflows/wf-1.json', - id: 'wf-1', - name: 'My workflow', - type: 'workflow', - status: 'modified', - location: 'local', - conflict: true, - updatedAt: '2024-01-01T00:00:00.000Z', - }, - ]; - const descriptor = classifyHttpError( - new ConflictError( - 'Push blocked by conflicting files. Pass `force: true` to push anyway.', - undefined, - { - conflicts, - }, - ), - ); - - expect(serializePublicApiError(descriptor)).toEqual({ - status: 409, - body: { - message: expect.stringContaining('conflicting files'), - conflicts, - }, - }); - expect(serializeInternalRestError(descriptor)).toEqual({ - status: 409, - body: { - code: 409, - message: expect.stringContaining('conflicting files'), - meta: { conflicts }, - }, - }); - }); - - it('both serializers map UserError to 400', () => { - const descriptor = classifyHttpError(new UserError('bad input')); - expect(serializePublicApiError(descriptor)).toEqual({ - status: 400, - body: { message: 'bad input' }, - }); - expect(serializeInternalRestError(descriptor)).toEqual({ - status: 400, - body: { code: 0, message: 'bad input' }, - }); - }); - - it('public sanitizes UnexpectedError; internal keeps message', () => { - const descriptor = classifyHttpError(new UnexpectedError('secret')); - expect(serializePublicApiError(descriptor)).toEqual({ - status: 500, - body: { message: 'Internal server error' }, - }); - expect(serializeInternalRestError(descriptor)).toEqual({ - status: 500, - body: { code: 0, message: 'secret' }, - }); - }); }); diff --git a/packages/cli/src/modules/policy-infrastructure/__tests__/policy-decision.service.test.ts b/packages/cli/src/modules/policy-infrastructure/__tests__/policy-decision.service.test.ts index 78f8050fa3c..534c6386e41 100644 --- a/packages/cli/src/modules/policy-infrastructure/__tests__/policy-decision.service.test.ts +++ b/packages/cli/src/modules/policy-infrastructure/__tests__/policy-decision.service.test.ts @@ -16,9 +16,8 @@ import { Container, Service } from '@n8n/di'; import { OperationalError } from 'n8n-workflow'; import { mock } from 'vitest-mock-extended'; -import { classifyHttpError } from '@/errors/http-error-classifier'; import type { PolicyActor } from '@/policy/policy-enforcement-backend'; -import { serializeInternalRestError } from '@/errors/http-error-serializers'; +import { classifyRestError, serializeInternalRestError } from '@n8n/backend-services'; import { PolicyEnforcementService } from '@/policy/policy-enforcement.service'; import { PolicyViolationError } from '@/policy/policy-violation.error'; @@ -705,7 +704,7 @@ describe('PolicyDecisionService', () => { const error = await proxy .enforceWorkflowSave(saveContext, unattended) .catch((e: unknown) => e); - const { status, body } = serializeInternalRestError(classifyHttpError(error as Error)); + const { status, body } = serializeInternalRestError(classifyRestError(error as Error)); expect(status).toBe(503); expect(JSON.stringify(body)).not.toContain('policy store'); diff --git a/packages/cli/src/policy/__tests__/policy-violation.error.test.ts b/packages/cli/src/policy/__tests__/policy-violation.error.test.ts index 1d81fd0ea68..ce3099a48ae 100644 --- a/packages/cli/src/policy/__tests__/policy-violation.error.test.ts +++ b/packages/cli/src/policy/__tests__/policy-violation.error.test.ts @@ -1,8 +1,8 @@ import type { PolicyViolation } from '@n8n/decorators'; import { UserError } from 'n8n-workflow'; -import { classifyHttpError, HttpErrorKind } from '@/errors/http-error-classifier'; -import { serializeInternalRestError } from '@/errors/http-error-serializers'; +import { classifyRestError, RestErrorKind } from '@n8n/backend-services'; +import { serializeInternalRestError } from '@n8n/backend-services'; import { isPolicyRefusal, @@ -67,7 +67,7 @@ describe('PolicyViolationError', () => { }); }); - describe('classifyHttpError', () => { + describe('classifyRestError', () => { it('classifies as a responseError carrying the violations in meta', () => { const violations: NonEmptyViolations = [ violation(), @@ -75,8 +75,8 @@ describe('PolicyViolationError', () => { ]; const error = new PolicyViolationError(violations); - expect(classifyHttpError(error)).toEqual({ - kind: HttpErrorKind.responseError, + expect(classifyRestError(error)).toEqual({ + kind: RestErrorKind.responseError, status: 403, code: 403, message: error.message, @@ -85,7 +85,7 @@ describe('PolicyViolationError', () => { }); it('carries the violations into the REST response body', () => { - const descriptor = classifyHttpError(new PolicyViolationError([violation()])); + const descriptor = classifyRestError(new PolicyViolationError([violation()])); const { status, body } = serializeInternalRestError(descriptor); expect(status).toBe(403); diff --git a/packages/cli/src/policy/policy-violation.error.ts b/packages/cli/src/policy/policy-violation.error.ts index 018197b055c..23e5bac0532 100644 --- a/packages/cli/src/policy/policy-violation.error.ts +++ b/packages/cli/src/policy/policy-violation.error.ts @@ -18,7 +18,7 @@ function summarize(violations: NonEmptyViolations): string { * Thrown by every `enforce*` method when a policy blocks an action. * * `UserError` so the execution path treats a blocked run as non-retryable, plus the - * `httpStatusCode`/`errorCode` pair `classifyHttpError` looks for so `meta` — and with it the + * `httpStatusCode`/`errorCode` pair `classifyRestError` looks for so `meta` — and with it the * violations — reaches the REST body. Extending `ResponseError` would lose the first. */ export class PolicyViolationError extends UserError { diff --git a/packages/cli/src/public-api/v1/public-api-error-response.ts b/packages/cli/src/public-api/v1/public-api-error-response.ts index 77c430ccf67..ed5f6bf8b0a 100644 --- a/packages/cli/src/public-api/v1/public-api-error-response.ts +++ b/packages/cli/src/public-api/v1/public-api-error-response.ts @@ -1,8 +1,10 @@ import type { Response } from 'express'; -import type { HttpErrorClassifierContext } from '@/errors/http-error-classifier'; -import { classifyHttpError } from '@/errors/http-error-classifier'; -import { serializePublicApiError } from '@/errors/http-error-serializers'; +import { + classifyRestError, + serializePublicApiError, + type RestErrorClassifierContext, +} from '@n8n/backend-services'; /** * Maps errors from the public API stack to HTTP responses. Used by the @@ -11,9 +13,9 @@ import { serializePublicApiError } from '@/errors/http-error-serializers'; export function sendPublicApiErrorResponse( res: Response, error: Error, - context?: HttpErrorClassifierContext, + context?: RestErrorClassifierContext, ): void { - const descriptor = classifyHttpError(error, context); + const descriptor = classifyRestError(error, context); const { status, body } = serializePublicApiError(descriptor); res.status(status).json(body); } diff --git a/packages/cli/src/response-helper.ts b/packages/cli/src/response-helper.ts index a8505f80684..92726737720 100644 --- a/packages/cli/src/response-helper.ts +++ b/packages/cli/src/response-helper.ts @@ -1,4 +1,9 @@ import { inDevelopment, Logger } from '@n8n/backend-common'; +import { + classifyRestError, + isResponseError, + serializeInternalRestError, +} from '@n8n/backend-services'; import { isUniqueConstraintError, type User } from '@n8n/db'; import { Container } from '@n8n/di'; import { type ReportingOptions } from '@n8n/errors'; @@ -9,9 +14,7 @@ import { FORM_TRIGGER_PATH_IDENTIFIER, NodeApiError } from 'n8n-workflow'; import { Readable } from 'node:stream'; import picocolors from 'picocolors'; -import { classifyHttpError, isResponseError } from './errors/http-error-classifier'; import { applyFormSandboxCSP } from './webhooks/webhook-response-headers'; -import { serializeInternalRestError } from './errors/http-error-serializers'; export function sendSuccessResponse( res: Response, @@ -77,7 +80,7 @@ export function sendErrorResponse(res: Response, error: Error) { } } - const descriptor = classifyHttpError(error); + const descriptor = classifyRestError(error); const { status, body: response } = serializeInternalRestError(descriptor); if (error instanceof NodeApiError) { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 215062decb8..f3a78e89ba3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1792,6 +1792,9 @@ importers: packages/@n8n/backend-services: dependencies: + '@n8n/api-types': + specifier: workspace:* + version: link:../api-types '@n8n/backend-common': specifier: workspace:* version: link:../backend-common @@ -1816,6 +1819,9 @@ importers: cache-manager: specifier: 'catalog:' version: 5.2.3 + express-openapi-validator: + specifier: 5.5.3 + version: 5.5.3(express@5.2.1(supports-color@8.1.1)) ioredis: specifier: 5.3.2 version: 5.3.2(supports-color@8.1.1) @@ -36699,6 +36705,24 @@ snapshots: path-to-regexp: 8.4.0 qs: 6.15.2 + express-openapi-validator@5.5.3(express@5.2.1(supports-color@8.1.1)): + dependencies: + '@apidevtools/json-schema-ref-parser': 12.0.2 + '@types/multer': 1.4.12 + ajv: 8.20.0 + ajv-draft-04: 1.0.0(ajv@8.20.0) + ajv-formats: 3.0.1(ajv@8.20.0) + content-type: 1.0.5 + express: 5.2.1(supports-color@8.1.1) + json-schema-traverse: 1.0.0 + lodash.clonedeep: 4.5.0 + lodash.get: 4.4.2 + media-typer: 1.1.0 + multer: 2.3.0 + ono: 7.1.3 + path-to-regexp: 8.4.0 + qs: 6.15.2 + express-prom-bundle@8.0.0(prom-client@15.1.3): dependencies: '@types/express': 5.0.1