From feed158ba2274d360b84cfdb93e85e619a8e795d Mon Sep 17 00:00:00 2001 From: Sam Wooler Date: Fri, 9 Oct 2026 17:46:14 +0000 Subject: [PATCH] feat(API): Support binary response bodies on Public API controller routes (no-changelog) (#40704) Co-authored-by: Claude Opus 5.5 --- .agents/skills/public-api/SKILL.md | 2 +- .agents/skills/public-api/reference.md | 29 +++ .../controller/__tests__/api-response.test.ts | 60 ++++- .../decorators/src/controller/api-response.ts | 70 +++++- .../@n8n/decorators/src/controller/index.ts | 4 + .../@n8n/decorators/src/controller/types.ts | 21 ++ .../public-api-controller.registry.test.ts | 220 ++++++++++++++++++ .../__tests__/binary-response.test.ts | 200 ++++++++++++++++ .../public-api/media-types/binary-response.ts | 119 ++++++++++ .../public-api-controller.registry.ts | 13 ++ .../public-api/public-api-route-resolver.ts | 4 + .../__tests__/decorator-routes.test.ts | 64 +++++ .../v1/openapi-gen/__tests__/generate.test.ts | 29 +++ .../v1/openapi-gen/decorator-routes.ts | 61 +++-- 14 files changed, 872 insertions(+), 24 deletions(-) create mode 100644 packages/cli/src/public-api/media-types/__tests__/binary-response.test.ts create mode 100644 packages/cli/src/public-api/media-types/binary-response.ts diff --git a/.agents/skills/public-api/SKILL.md b/.agents/skills/public-api/SKILL.md index 4e745fb89f5..66c6e02ff3a 100644 --- a/.agents/skills/public-api/SKILL.md +++ b/.agents/skills/public-api/SKILL.md @@ -109,7 +109,7 @@ model; reuse only what applies. Decorators, all from `@n8n/decorators`: | `@Get/@Post/@Put/@Patch/@Delete('/path')` | Route method. | | `@ApiKeyScope('res:action')` | API-key grant check. | | `@ProjectScope/@GlobalScope('res:action')` | User RBAC check. | -| `@ApiResponse(status)` / `@ApiResponse(status, Dto)` | Success status + (optional) output DTO; registry `.parse()`s + strips the return value. Exactly one per route — a second `@ApiResponse` throws. `204` can't carry a DTO — throws. | +| `@ApiResponse(status)` / `@ApiResponse(status, Dto)` | Success status + (optional) output DTO; registry `.parse()`s + strips the return value. Exactly one per route — a second `@ApiResponse` throws. `204` can't carry a DTO or a binary body — throws. For a binary response body (returned as `{ body, headers? }`), use `@ApiResponse(status, { mediaType, description?, headers? })` — see [Binary response bodies](reference.md#binary-response-bodies). | | `@ApiErrorResponse(status)` | Declares an additional documented non-2xx status (e.g. `404`, `409`). Stack multiple for more than one. `400`/`401`/`403` are added automatically (body/query present, always, and `@ApiKeyScope` present, respectively) — don't declare those yourself. | | `@ApiSummary(text)` / `@ApiDescription(text)` / `@ApiTags([...])` | OpenAPI summary/description/tags. `@ApiTags` sorts alphabetically regardless of the order you pass. All optional but expected on every real route. | | `@Query` / `@Body` / `@Param('name')` | Bind + validate via a `Z.class` DTO / path param. `@Body` is JSON by default; `@Body({ mediaType: 'multipart/form-data', uploadLimits })` takes a `multipart/form-data` body instead — see [Request body media types](reference.md#request-body-media-types). | diff --git a/.agents/skills/public-api/reference.md b/.agents/skills/public-api/reference.md index e67b24a13ae..52a3760546e 100644 --- a/.agents/skills/public-api/reference.md +++ b/.agents/skills/public-api/reference.md @@ -146,6 +146,35 @@ handler per media type (`REQUEST_BODY_HANDLERS` in `REQUEST_BODY_HANDLERS` — the registry, resolver, generator and `/discover` need no change, since they all read the handler, not the media type. +## Binary response bodies + +`@ApiResponse(status, { mediaType, description?, headers? })` declares a +success body that the controller method returns as `{ body, headers? }`, for +example an `application/gzip` stream. The framework treats options whose +`mediaType` is in `BINARY_RESPONSE_MEDIA_TYPES` as a binary response. +`mediaType` accepts only those types. + +- `body` is a `Buffer` or a `Readable`. The method returns the result and never + writes to `res`. +- `headers` holds the response headers. Every header declared in `headers` must + be in the result. A missing header fails the request with a `500`. Matching is + case-insensitive. +- The registry checks the headers, then sets the declared status, the headers, + and `Content-Type: `. A header in the result cannot replace the + `Content-Type`. +- A stream's first chunk is read before any header is set. So a stream that + fails before its first chunk gives the normal JSON error response, with no + binary `Content-Type`. A method that throws before it returns does the same. +- After the first chunk is sent, an error ends the response. A client that + disconnects is not an error, and the framework destroys the stream. +- A method that returns no binary body fails with a `500`. +- The generator documents the body as `{ type: string, format: binary }` + under the `mediaType` content key, with the description and headers. + The description defaults to `Operation successful.`. + +The runtime part is `sendBinaryResponse` in +`packages/cli/src/public-api/media-types/binary-response.ts`. + ## Migrating legacy EOV endpoints Legacy `express-openapi-validator` endpoints live under diff --git a/packages/@n8n/decorators/src/controller/__tests__/api-response.test.ts b/packages/@n8n/decorators/src/controller/__tests__/api-response.test.ts index 771e9957a5d..e97e0514291 100644 --- a/packages/@n8n/decorators/src/controller/__tests__/api-response.test.ts +++ b/packages/@n8n/decorators/src/controller/__tests__/api-response.test.ts @@ -5,7 +5,7 @@ import { z } from 'zod'; import { ApiResponse } from '../api-response'; import { ControllerRegistryMetadata } from '../controller-registry-metadata'; import { Get } from '../route'; -import type { Controller } from '../types'; +import type { BinaryResponse, Controller } from '../types'; const ExampleDto = Z.class({ id: z.string(), @@ -64,4 +64,62 @@ describe('@ApiResponse Decorator', () => { expect(route.successStatus).toBe(204); expect(route.responseDto).toBeUndefined(); }); + + it('should store binary response metadata and leave responseDto undefined', () => { + const binaryOptions: BinaryResponse = { + mediaType: 'application/gzip', + description: 'Compressed archive', + headers: { + 'X-Archive-Size': { description: 'Size of the archive' }, + }, + }; + + class TestController { + @Get('/') + @ApiResponse(200, binaryOptions) + async handler() {} + } + + const route = controllerRegistryMetadata.getRouteMetadata( + TestController as Controller, + 'handler', + ); + expect(route.binaryResponse).toEqual(binaryOptions); + expect(route.successStatus).toBe(200); + expect(route.responseDto).toBeUndefined(); + }); + + it('should reject a binary response with an unsupported media type', () => { + expect(() => { + class TestController { + @Get('/') + @ApiResponse(200, { mediaType: 'text/csv' } as never) + async handler() {} + } + void TestController; + }).toThrow('unsupported binary media type "text/csv"'); + }); + + it('should reject a body that is neither a response DTO nor binary options', () => { + expect(() => { + class TestController { + @Get('/') + @ApiResponse(200, {} as never) + async handler() {} + } + void TestController; + }).toThrow('neither a response DTO nor binary options'); + }); + + it('should reject 204 with binary response', () => { + expect(() => { + const binaryOptions = { mediaType: 'application/gzip' } as never; + class TestController { + @Get('/') + @ApiResponse(204, binaryOptions) + async handler() {} + } + void TestController; + }).toThrow('declares a 204 @ApiResponse with a binary body'); + }); }); diff --git a/packages/@n8n/decorators/src/controller/api-response.ts b/packages/@n8n/decorators/src/controller/api-response.ts index 2b465165387..122cbb9ae32 100644 --- a/packages/@n8n/decorators/src/controller/api-response.ts +++ b/packages/@n8n/decorators/src/controller/api-response.ts @@ -1,15 +1,47 @@ import { Container } from '@n8n/di'; import { ControllerRegistryMetadata } from './controller-registry-metadata'; -import type { Controller, ResponseDtoClass, SuccessStatus } from './types'; +import { + BINARY_RESPONSE_MEDIA_TYPES, + type BinaryResponse, + type Controller, + type ResponseDtoClass, + type SuccessStatus, +} from './types'; + +function hasMediaType(body: unknown): body is { mediaType: unknown } { + return typeof body === 'object' && body !== null && 'mediaType' in body; +} + +function isBinaryResponse(body: unknown): body is BinaryResponse { + return ( + hasMediaType(body) && + BINARY_RESPONSE_MEDIA_TYPES.some((mediaType) => mediaType === body.mediaType) + ); +} + +function isResponseDto(body: unknown): body is ResponseDtoClass { + return ( + (typeof body === 'function' || (typeof body === 'object' && body !== null)) && 'parse' in body + ); +} /** - * Declares what a route returns on success: its HTTP status, and its public output DTO if it sends a - * body. Only one @ApiResponse decorator should be present per endpoint otherwise an error will be thrown. + * Declares what a route returns on success: its HTTP status, and either its public output DTO or a + * binary body that the controller method returns. + * + * Only one @ApiResponse decorator should be present per endpoint otherwise an error will be thrown. */ -export const ApiResponse = - (status: SuccessStatus, dto?: ResponseDtoClass): MethodDecorator => - (target, handlerName) => { +export function ApiResponse(status: SuccessStatus, dto?: ResponseDtoClass): MethodDecorator; +export function ApiResponse( + status: Exclude, + binary: BinaryResponse, +): MethodDecorator; +export function ApiResponse( + status: SuccessStatus, + body?: ResponseDtoClass | BinaryResponse, +): MethodDecorator { + return (target, handlerName) => { const routeMetadata = Container.get(ControllerRegistryMetadata).getRouteMetadata( target.constructor as Controller, String(handlerName), @@ -23,12 +55,32 @@ export const ApiResponse = } // HTTP 204 No Content responses must not have a body - if (status === 204 && dto !== undefined) { + if (status === 204 && body !== undefined) { + const bodyKind = isBinaryResponse(body) ? 'a binary body' : 'a response DTO'; throw new Error( - `${String(handlerName)} declares a 204 @ApiResponse with a response DTO - a 204 response must not have a body`, + `${String(handlerName)} declares a 204 @ApiResponse with ${bodyKind} - a 204 response must not have a body`, ); } routeMetadata.successStatus = status; - routeMetadata.responseDto = dto; + + if (isBinaryResponse(body)) { + routeMetadata.binaryResponse = body; + return; + } + + if (hasMediaType(body)) { + throw new Error( + `${String(handlerName)} declares an unsupported binary media type "${String(body.mediaType)}" - supported: ${BINARY_RESPONSE_MEDIA_TYPES.join(', ')}`, + ); + } + + if (body !== undefined && !isResponseDto(body)) { + throw new Error( + `${String(handlerName)} declares an @ApiResponse body that is neither a response DTO nor binary options`, + ); + } + + routeMetadata.responseDto = body; }; +} diff --git a/packages/@n8n/decorators/src/controller/index.ts b/packages/@n8n/decorators/src/controller/index.ts index 75a5f78ad08..e29cff3c8a6 100644 --- a/packages/@n8n/decorators/src/controller/index.ts +++ b/packages/@n8n/decorators/src/controller/index.ts @@ -19,6 +19,9 @@ export type { AccessScope, ApiKeyScopeRequirement, Arg, + BinaryResponse, + BinaryResponseMediaType, + BinaryResult, Controller, CorsOptions, DeprecationInfo, @@ -30,6 +33,7 @@ export type { RequestBodyMediaOptions, RequestBodyMediaType, ResponseDtoClass, + ResponseHeader, RouteMetadata, StaticRouterMetadata, SuccessStatus, diff --git a/packages/@n8n/decorators/src/controller/types.ts b/packages/@n8n/decorators/src/controller/types.ts index 62d514bda35..a09be58c356 100644 --- a/packages/@n8n/decorators/src/controller/types.ts +++ b/packages/@n8n/decorators/src/controller/types.ts @@ -2,6 +2,7 @@ import type { ZodClass } from '@n8n/api-types'; import type { BooleanLicenseFeature } from '@n8n/constants'; import type { Constructable } from '@n8n/di'; import type { ApiKeyScope, Scope } from '@n8n/permissions'; +import type { Readable } from 'node:stream'; import type { RequestHandler, Router } from 'express'; import type { ZodTypeAny } from 'zod'; @@ -14,6 +15,24 @@ export type ApiKeyScopeRequirement = export type ResponseDtoClass = Pick; +export const BINARY_RESPONSE_MEDIA_TYPES = ['application/gzip'] as const; +export type BinaryResponseMediaType = (typeof BINARY_RESPONSE_MEDIA_TYPES)[number]; + +export interface ResponseHeader { + description: string; +} + +export interface BinaryResponse { + mediaType: BinaryResponseMediaType; + description?: string; + headers?: Record; +} + +export interface BinaryResult { + body: Buffer | Readable; + headers?: Record; +} + export type SuccessStatus = 200 | 201 | 202 | 204; export interface ErrorResponse { @@ -99,6 +118,8 @@ export interface RouteMetadata { accessScope?: AccessScope; apiKeyScope?: ApiKeyScopeRequirement; responseDto?: ResponseDtoClass; + /** Mutually exclusive with `responseDto`. */ + binaryResponse?: BinaryResponse; /** OpenAPI HTTP status sent on success, and documented as such. */ successStatus?: SuccessStatus; /** OpenAPI operation summary. */ diff --git a/packages/cli/src/public-api/__tests__/public-api-controller.registry.test.ts b/packages/cli/src/public-api/__tests__/public-api-controller.registry.test.ts index f8a4f7601a7..482fdb674dd 100644 --- a/packages/cli/src/public-api/__tests__/public-api-controller.registry.test.ts +++ b/packages/cli/src/public-api/__tests__/public-api-controller.registry.test.ts @@ -20,6 +20,7 @@ import { import type { Controller, MultipartUploadLimits } from '@n8n/decorators'; import { Container, Service } from '@n8n/di'; import express from 'express'; +import { Readable } from 'node:stream'; import request from 'supertest'; import { mock } from 'vitest-mock-extended'; import { z } from 'zod'; @@ -724,4 +725,223 @@ describe('PublicApiControllerRegistry', () => { expect(response.body).toEqual({ message: 'Forbidden' }); }); }); + + describe('binary response bodies', () => { + // supertest buffers only text and JSON bodies by default + const readBinaryBody = ( + res: request.Response, + callback: (error: Error | null, body: Buffer) => void, + ) => { + const chunks: Buffer[] = []; + res.on('data', (chunk: Buffer) => chunks.push(chunk)); + res.on('end', () => callback(null, Buffer.concat(chunks))); + }; + + it('sends the body the method returns, with the declared status, media type and headers', async () => { + @Service() + class WidgetsPublicController { + @Get('/') + @ApiResponse(202, { + mediaType: 'application/gzip', + headers: { 'X-Example': { description: 'Example header.' } }, + }) + async method() { + return { body: Buffer.from([1, 2, 3]), headers: { 'X-Example': '1' } }; + } + } + markPublicApiController(WidgetsPublicController as Controller, '/widgets'); + + const response = await request(activate()) + .get('/api/v1/widgets') + .buffer(true) + .parse(readBinaryBody) + .expect(202); + + expect(response.headers['content-type']).toBe('application/gzip'); + expect(response.headers['x-example']).toBe('1'); + expect(response.body).toEqual(Buffer.from([1, 2, 3])); + }); + + it('streams a returned stream', async () => { + @Service() + class WidgetsPublicController { + @Get('/') + @ApiResponse(200, { mediaType: 'application/gzip' }) + async method() { + return { body: Readable.from([Buffer.from('ab'), Buffer.from('cd')]) }; + } + } + markPublicApiController(WidgetsPublicController as Controller, '/widgets'); + + const response = await request(activate()) + .get('/api/v1/widgets') + .buffer(true) + .parse(readBinaryBody) + .expect(200); + + expect(response.body.toString()).toBe('abcd'); + }); + + it('matches a declared header name case-insensitively', async () => { + @Service() + class WidgetsPublicController { + @Get('/') + @ApiResponse(200, { + mediaType: 'application/gzip', + headers: { 'X-Example': { description: 'Example header.' } }, + }) + async method() { + return { body: Buffer.from('x'), headers: { 'x-example': '1' } }; + } + } + markPublicApiController(WidgetsPublicController as Controller, '/widgets'); + + const response = await request(activate()) + .get('/api/v1/widgets') + .buffer(true) + .parse(readBinaryBody) + .expect(200); + + expect(response.headers['x-example']).toBe('1'); + }); + + it('keeps the declared media type when a result header tries to replace it', async () => { + @Service() + class WidgetsPublicController { + @Get('/') + @ApiResponse(200, { mediaType: 'application/gzip' }) + async method() { + return { body: Buffer.from('x'), headers: { 'Content-Type': 'text/plain' } }; + } + } + markPublicApiController(WidgetsPublicController as Controller, '/widgets'); + + const response = await request(activate()) + .get('/api/v1/widgets') + .buffer(true) + .parse(readBinaryBody) + .expect(200); + + expect(response.headers['content-type']).toBe('application/gzip'); + }); + + it('fails with 500 when the method returns no binary body', async () => { + @Service() + class WidgetsPublicController { + @Get('/') + @ApiResponse(200, { mediaType: 'application/gzip' }) + async method() {} + } + markPublicApiController(WidgetsPublicController as Controller, '/widgets'); + + const response = await request(activate()).get('/api/v1/widgets').expect(500); + + expect(response.headers['content-type']).toMatch(/application\/json/); + expect(response.body.message).toBe('Internal server error'); + }); + + it('fails with 500 when a declared header is missing from the result', async () => { + @Service() + class WidgetsBinaryHeaderPublicController { + @Get('/') + @ApiResponse(200, { + mediaType: 'application/gzip', + headers: { 'X-Required': { description: 'Must be set.' } }, + }) + async method() { + return { body: Buffer.from('x') }; + } + } + markPublicApiController(WidgetsBinaryHeaderPublicController as Controller, '/widgets'); + + const response = await request(activate()).get('/api/v1/widgets').expect(500); + + expect(response.headers['content-type']).toMatch(/application\/json/); + expect(response.body.message).toBe('Internal server error'); + }); + + it('sends a clean JSON error when the method throws before the body starts', async () => { + @Service() + class WidgetsPublicController { + @Get('/') + @ApiResponse(200, { mediaType: 'application/gzip' }) + async method() { + throw new NotFoundError('missing'); + } + } + markPublicApiController(WidgetsPublicController as Controller, '/widgets'); + + const response = await request(activate()).get('/api/v1/widgets').expect(404); + + expect(response.headers['content-type']).toMatch(/application\/json/); + expect(response.body.message).toBe('missing'); + }); + + it('sends a clean JSON error when a stream fails before its first chunk', async () => { + @Service() + class WidgetsPublicController { + @Get('/') + @ApiResponse(200, { + mediaType: 'application/gzip', + headers: { 'X-Example': { description: 'Example header.' } }, + }) + async method() { + const failing = new Readable({ + read() { + this.destroy(new NotFoundError('missing')); + }, + }); + return { body: failing, headers: { 'X-Example': '1' } }; + } + } + markPublicApiController(WidgetsPublicController as Controller, '/widgets'); + + const response = await request(activate()).get('/api/v1/widgets').expect(404); + + expect(response.headers['content-type']).toMatch(/application\/json/); + expect(response.headers['x-example']).toBeUndefined(); + expect(response.body.message).toBe('missing'); + }); + + it('keeps headers set before the method on an error response', async () => { + const since = new Date('2026-07-23T00:00:00Z'); + + @Service() + class WidgetsPublicController { + @Get('/') + @ApiResponse(200, { mediaType: 'application/gzip' }) + @Deprecated({ since }) + async method() { + throw new NotFoundError('missing'); + } + } + markPublicApiController(WidgetsPublicController as Controller, '/widgets'); + + const response = await request(activate()).get('/api/v1/widgets').expect(404); + + expect(response.headers.deprecation).toBe(`@${Math.floor(since.getTime() / 1000)}`); + }); + + it('keeps a Content-Type set by earlier middleware on an error response', async () => { + @Service() + class WidgetsBinaryMiddlewarePublicController { + @Middleware() + label(_req: express.Request, res: express.Response, next: express.NextFunction) { + res.setHeader('Content-Type', 'text/plain'); + next(); + } + + @Get('/') + @ApiResponse(200, { mediaType: 'application/gzip' }) + async method() { + throw new NotFoundError('missing'); + } + } + markPublicApiController(WidgetsBinaryMiddlewarePublicController as Controller, '/widgets'); + + const response = await request(activate()).get('/api/v1/widgets').expect(404); + + expect(response.headers['content-type']).toMatch(/text\/plain/); + }); + }); }); diff --git a/packages/cli/src/public-api/media-types/__tests__/binary-response.test.ts b/packages/cli/src/public-api/media-types/__tests__/binary-response.test.ts new file mode 100644 index 00000000000..623966e424b --- /dev/null +++ b/packages/cli/src/public-api/media-types/__tests__/binary-response.test.ts @@ -0,0 +1,200 @@ +import type { BinaryResponse } from '@n8n/decorators'; +import type { Response } from 'express'; +import { Readable, Writable } from 'node:stream'; +import { mock } from 'vitest-mock-extended'; + +import { sendBinaryResponse } from '../binary-response'; + +/** A live response mock whose `status` returns the response, as Express does. */ +function mockResponse() { + const res = mock(); + res.status.mockReturnValue(res); + res.destroyed = false; + return res; +} + +describe('sendBinaryResponse', () => { + const binaryResponse: BinaryResponse = { + mediaType: 'application/gzip', + headers: { 'X-Required': { description: 'Must be set.' } }, + }; + + it('rejects a result that is missing a declared header, before the response is touched', async () => { + const res = mockResponse(); + + await expect( + sendBinaryResponse(res, binaryResponse, 200, 'Widgets.method', { body: Buffer.from('x') }), + ).rejects.toThrow('Widgets.method did not set the declared response header(s): X-Required'); + + expect(res.status).not.toHaveBeenCalled(); + expect(res.setHeader).not.toHaveBeenCalled(); + }); + + it('matches a declared header name case-insensitively', async () => { + const res = mockResponse(); + + await sendBinaryResponse(res, binaryResponse, 200, 'Widgets.method', { + body: Buffer.from('x'), + headers: { 'x-required': '1' }, + }); + + expect(res.end).toHaveBeenCalledWith(Buffer.from('x')); + }); + + it('rejects a result without a binary body', async () => { + const res = mockResponse(); + + await expect( + sendBinaryResponse(res, binaryResponse, 200, 'Widgets.method', { + body: 'not binary', + headers: { 'X-Required': '1' }, + }), + ).rejects.toThrow('Widgets.method declares a binary @ApiResponse but returned no body'); + + expect(res.status).not.toHaveBeenCalled(); + }); + + it('ends an empty stream with a 200 and no body', async () => { + const sent: Buffer[] = []; + const client = new Writable({ + write(chunk: Buffer, _encoding, callback) { + sent.push(chunk); + callback(); + }, + }); + const res = Object.assign(client, { + status: vi.fn(() => client), + setHeader: vi.fn(() => client), + }); + + await sendBinaryResponse(res as unknown as Response, binaryResponse, 200, 'Widgets.method', { + body: Readable.from([]), + headers: { 'X-Required': '1' }, + }); + + expect(res.status).toHaveBeenCalledWith(200); + expect(client.writableFinished).toBe(true); + expect(sent).toEqual([]); + }); + + it('resolves when the client disconnects after the first chunk', async () => { + const sent: Buffer[] = []; + const client = new Writable({ + write(chunk: Buffer, _encoding, callback) { + sent.push(chunk); + // Drop the connection on the second chunk, before its write completes. + if (sent.length === 2) { + client.destroy(); + return; + } + callback(); + }, + }); + const res = Object.assign(client, { status: () => client, setHeader: () => client }); + + await expect( + sendBinaryResponse(res as unknown as Response, binaryResponse, 200, 'Widgets.method', { + body: Readable.from([Buffer.from('ab'), Buffer.from('cd'), Buffer.from('ef')]), + headers: { 'X-Required': '1' }, + }), + ).resolves.toBeUndefined(); + + expect(sent).toHaveLength(2); + }); + + it('destroys the stream when the client has already left', async () => { + const client = new Writable({ + write(_chunk, _encoding, callback) { + callback(); + }, + }); + client.destroy(); + const res = Object.assign(client, { status: () => client, setHeader: () => client }); + const body = Readable.from([Buffer.from('ab')]); + + await expect( + sendBinaryResponse(res as unknown as Response, binaryResponse, 200, 'Widgets.method', { + body, + headers: { 'X-Required': '1' }, + }), + ).resolves.toBeUndefined(); + + expect(body.destroyed).toBe(true); + }); + + it('destroys a returned stream when a declared header is missing', async () => { + const body = Readable.from([Buffer.from('x')]); + + await expect( + sendBinaryResponse(mockResponse(), binaryResponse, 200, 'Widgets.method', { body }), + ).rejects.toThrow('X-Required'); + + expect(body.destroyed).toBe(true); + }); + + it('destroys the stream when the client leaves while the first chunk is pending', async () => { + const client = new Writable({ + write(_chunk, _encoding, callback) { + callback(); + }, + }); + const res = Object.assign(client, { status: () => client, setHeader: () => client }); + // A source that never produces a chunk. + const body = new Readable({ read() {} }); + + const sending = sendBinaryResponse( + res as unknown as Response, + binaryResponse, + 200, + 'Widgets.method', + { + body, + headers: { 'X-Required': '1' }, + }, + ); + client.destroy(); + + await expect(sending).resolves.toBeUndefined(); + expect(body.destroyed).toBe(true); + }); + + it('destroys a stalled stream when the client left before the response started', async () => { + const client = new Writable({ + write(_chunk, _encoding, callback) { + callback(); + }, + }); + client.destroy(); + // Let the close event fire before the response starts, so a listener added later never sees it. + await new Promise((resolve) => setImmediate(resolve)); + const res = Object.assign(client, { status: () => client, setHeader: () => client }); + // A source that never produces a chunk. + const body = new Readable({ read() {} }); + + await expect( + sendBinaryResponse(res as unknown as Response, binaryResponse, 200, 'Widgets.method', { + body, + headers: { 'X-Required': '1' }, + }), + ).resolves.toBeUndefined(); + + expect(body.destroyed).toBe(true); + }); + + it('destroys the stream when setting a header throws', async () => { + const res = mockResponse(); + res.setHeader.mockImplementation(() => { + throw new Error('Invalid character in header content'); + }); + const body = Readable.from([Buffer.from('x')]); + + await expect( + sendBinaryResponse(res, binaryResponse, 200, 'Widgets.method', { + body, + headers: { 'X-Required': '1' }, + }), + ).rejects.toThrow('Invalid character in header content'); + + expect(body.destroyed).toBe(true); + }); +}); diff --git a/packages/cli/src/public-api/media-types/binary-response.ts b/packages/cli/src/public-api/media-types/binary-response.ts new file mode 100644 index 00000000000..44b4443f868 --- /dev/null +++ b/packages/cli/src/public-api/media-types/binary-response.ts @@ -0,0 +1,119 @@ +import type { BinaryResponse, BinaryResult, SuccessStatus } from '@n8n/decorators'; +import { UnexpectedError } from '@n8n/errors'; +import type { Response } from 'express'; +import { Readable } from 'node:stream'; +import { pipeline } from 'node:stream/promises'; + +function isBinaryResult(result: unknown): result is BinaryResult { + return ( + typeof result === 'object' && + result !== null && + 'body' in result && + (Buffer.isBuffer(result.body) || result.body instanceof Readable) + ); +} + +function isClientGone(error: unknown): boolean { + return ( + error instanceof Error && + 'code' in error && + (error.code === 'ERR_STREAM_PREMATURE_CLOSE' || error.code === 'ERR_STREAM_UNABLE_TO_PIPE') + ); +} + +function setResponseHeaders( + res: Response, + successStatus: SuccessStatus, + mediaType: BinaryResponse['mediaType'], + headers: NonNullable, +) { + for (const [name, value] of Object.entries(headers)) { + res.setHeader(name, value); + } + // Set last, so a result header cannot replace the declared media type. + res.status(successStatus).setHeader('Content-Type', mediaType); +} + +async function sendStream(res: Response, body: Readable, setHeaders: () => void) { + const release = () => body.destroy(); + res.once('close', release); + + // The client may have left before the listener was attached. + if (res.destroyed) { + release(); + } + + try { + // Peek at the first chunk before any header is set. If the stream fails here, the error + // still gets a JSON response. + const iterator = body[Symbol.asyncIterator](); + const first = await iterator.next(); + + setHeaders(); + + const rest = { [Symbol.asyncIterator]: () => iterator }; + async function* withFirstChunk() { + if (!first.done) { + yield first.value; + } + yield* rest; + } + + await pipeline(Readable.from(withFirstChunk()), res); + } catch (error) { + // The client disconnected. There is no one left to send to. + if (!isClientGone(error)) { + throw error; + } + } finally { + res.off('close', release); + } +} + +/** + * Sends the result of a route whose success body is binary. + */ +export async function sendBinaryResponse( + res: Response, + binaryResponse: BinaryResponse, + successStatus: SuccessStatus, + routeName: string, + result: unknown, +): Promise { + if (!isBinaryResult(result)) { + throw new UnexpectedError(`${routeName} declares a binary @ApiResponse but returned no body`); + } + + const { body } = result; + const declaredHeaders = binaryResponse.headers ?? {}; + const resultHeaders = result.headers ?? {}; + + try { + const setHeaderNames = new Set(Object.keys(resultHeaders).map((name) => name.toLowerCase())); + const missing = Object.keys(declaredHeaders).filter( + (name) => !setHeaderNames.has(name.toLowerCase()), + ); + if (missing.length) { + throw new UnexpectedError( + `${routeName} did not set the declared response header(s): ${missing.join(', ')}`, + ); + } + + const setHeaders = () => + setResponseHeaders(res, successStatus, binaryResponse.mediaType, resultHeaders); + + // A buffer is already in memory, so send it in one write. + if (Buffer.isBuffer(body)) { + setHeaders(); + res.end(body); + return; + } + + await sendStream(res, body, setHeaders); + } finally { + // However this function exits, the stream must not stay open. + if (body instanceof Readable) { + body.destroy(); + } + } +} diff --git a/packages/cli/src/public-api/public-api-controller.registry.ts b/packages/cli/src/public-api/public-api-controller.registry.ts index 6c31c3a8114..5006c18c633 100644 --- a/packages/cli/src/public-api/public-api-controller.registry.ts +++ b/packages/cli/src/public-api/public-api-controller.registry.ts @@ -20,6 +20,7 @@ import { FeatureNotLicensedError } from '@/errors/feature-not-licensed.error'; import { BadRequestError, UnsupportedMediaTypeError } from '@n8n/errors'; import { License } from '@/license'; import { assertContentType } from '@/public-api/media-types/content-type'; +import { sendBinaryResponse } from '@/public-api/media-types/binary-response'; import type { RequestBodyHandler } from '@/public-api/media-types/request-body'; import { userHasScopes } from '@/permissions.ee/check-access'; import { USER_QUOTA_FORBIDDEN_MESSAGE } from '@/public-api/constants'; @@ -120,6 +121,18 @@ export class PublicApiControllerRegistry { } } + if (route.binaryResponse) { + const result = await controller[handlerName](...args); + await sendBinaryResponse( + res, + route.binaryResponse, + successStatus, + `${controllerClass.name}.${handlerName}`, + result, + ); + return; + } + const result = await controller[handlerName](...args); if (res.headersSent) return; diff --git a/packages/cli/src/public-api/public-api-route-resolver.ts b/packages/cli/src/public-api/public-api-route-resolver.ts index 0053e5b4ed2..3de183b4ab4 100644 --- a/packages/cli/src/public-api/public-api-route-resolver.ts +++ b/packages/cli/src/public-api/public-api-route-resolver.ts @@ -2,6 +2,7 @@ import type { ZodClass } from '@n8n/api-types'; import type { ApiKeyScopeRequirement, Arg, + BinaryResponse, Controller, DeprecationInfo, ErrorResponse, @@ -83,6 +84,8 @@ export interface ResolvedPublicApiRoute { requestBodyHandler?: RequestBodyHandler; requestQueryDto?: ZodClass; responseDto?: ResponseDtoClass; + /** Mutually exclusive with `responseDto`. */ + binaryResponse?: BinaryResponse; /** Success status declared via `@ApiResponse` - always present, see `resolveSuccessStatus`. */ successStatus: SuccessStatus; apiKeyScope?: ApiKeyScopeRequirement; @@ -296,6 +299,7 @@ export function resolvePublicApiRoutes(): ResolvedPublicApiRoute[] { requestBodyHandler, requestQueryDto, responseDto: route.responseDto, + binaryResponse: route.binaryResponse, successStatus: resolveSuccessStatus(controllerClass.name, handlerName, route.successStatus), apiKeyScope: route.apiKeyScope, summary: route.summary, diff --git a/packages/cli/src/public-api/v1/openapi-gen/__tests__/decorator-routes.test.ts b/packages/cli/src/public-api/v1/openapi-gen/__tests__/decorator-routes.test.ts index bbd8467932c..268fd806d49 100644 --- a/packages/cli/src/public-api/v1/openapi-gen/__tests__/decorator-routes.test.ts +++ b/packages/cli/src/public-api/v1/openapi-gen/__tests__/decorator-routes.test.ts @@ -373,4 +373,68 @@ describe('getDecoratorGeneratedOperations', () => { expect(getSharedResponseSchemas().has(WidgetResponseDto)).toBe(false); }); + + it('documents a binary success response', () => { + class WidgetsPublicController { + @Get('/') + @ApiResponse(200, { + mediaType: 'application/gzip', + description: 'An archive.', + headers: { + 'X-Example': { description: 'A header.' }, + }, + }) + method() {} + } + markPublicApiController(WidgetsPublicController as Controller, '/widgets'); + + const [operation] = getDecoratorGeneratedOperations(); + + expect(operation.config.responses[200]).toEqual({ + description: 'An archive.', + headers: { + 'X-Example': { + description: 'A header.', + required: true, + schema: { type: 'string' }, + }, + }, + content: { + 'application/gzip': { + schema: { type: 'string', format: 'binary' }, + }, + }, + }); + }); + + it('defaults the binary response description to "Operation successful." and omits headers when none are declared', () => { + class WidgetsPublicController { + @Get('/') + @ApiResponse(200, { mediaType: 'application/gzip' }) + method() {} + } + markPublicApiController(WidgetsPublicController as Controller, '/widgets'); + + const [operation] = getDecoratorGeneratedOperations(); + + expect(operation.config.responses[200]).toEqual({ + description: 'Operation successful.', + content: { + 'application/gzip': { + schema: { type: 'string', format: 'binary' }, + }, + }, + }); + }); + + it('does not include a binary route in getSharedResponseSchemas', () => { + class WidgetsPublicController { + @Get('/') + @ApiResponse(200, { mediaType: 'application/gzip' }) + method() {} + } + markPublicApiController(WidgetsPublicController as Controller, '/widgets'); + + expect(getSharedResponseSchemas().size).toBe(0); + }); }); diff --git a/packages/cli/src/public-api/v1/openapi-gen/__tests__/generate.test.ts b/packages/cli/src/public-api/v1/openapi-gen/__tests__/generate.test.ts index 5db932501fa..49d43b358ee 100644 --- a/packages/cli/src/public-api/v1/openapi-gen/__tests__/generate.test.ts +++ b/packages/cli/src/public-api/v1/openapi-gen/__tests__/generate.test.ts @@ -9,6 +9,7 @@ import { registerSharedSchemas, type OpenApiDocument, } from '../generate'; +import { buildBinarySuccessResponse } from '../decorator-routes'; function makeNamedResponseDto(className: string, shape: Parameters[0]) { return { [className]: class extends Z.class(shape) {} }[className]; @@ -268,4 +269,32 @@ describe('shared schema registry', () => { expect(registered.get(dtoA)).toBeDefined(); expect(registered.get(dtoB)).toBeDefined(); }); + + it('emits a binary response body with format: binary and its header descriptions', () => { + const registry = new OpenAPIRegistry(); + registry.registerPath({ + method: 'get', + path: '/exports', + responses: { + 200: buildBinarySuccessResponse({ + mediaType: 'application/gzip', + description: 'An archive.', + headers: { 'X-Archive-Size': { description: 'Size of the archive' } }, + }), + }, + }); + + const [artifact] = buildArtifactsFromRegistry(registry, [ + { + outputPath: 'handlers/exports/spec/paths/export.generated.yml', + pathKey: '/exports', + method: 'get', + }, + ]); + + expect(artifact.content).toContain('application/gzip:'); + expect(artifact.content).toContain('format: binary'); + expect(artifact.content).toContain('X-Archive-Size:'); + expect(artifact.content).toContain('Size of the archive'); + }); }); diff --git a/packages/cli/src/public-api/v1/openapi-gen/decorator-routes.ts b/packages/cli/src/public-api/v1/openapi-gen/decorator-routes.ts index 0fcaf11ecfd..3aebd9147cf 100644 --- a/packages/cli/src/public-api/v1/openapi-gen/decorator-routes.ts +++ b/packages/cli/src/public-api/v1/openapi-gen/decorator-routes.ts @@ -7,7 +7,7 @@ import '../controllers'; import { OpenAPIRegistry, OpenApiGeneratorV3 } from '@asteasolutions/zod-to-openapi'; import type { RouteConfig } from '@asteasolutions/zod-to-openapi'; -import type { ResponseDtoClass } from '@n8n/decorators'; +import type { BinaryResponse, ResponseDtoClass } from '@n8n/decorators'; import { isRecord } from '@n8n/utils/is-record'; import { UnexpectedError } from 'n8n-workflow'; import { z } from 'zod'; @@ -199,30 +199,65 @@ export function buildRequestBodyJsonSchema( return isRecord(schema) ? schema : undefined; } +/** Documents a success body the controller method writes itself, with its declared headers. */ +export function buildBinarySuccessResponse({ mediaType, description, headers }: BinaryResponse) { + const responseHeaders: Record< + string, + { description: string; required: true; schema: { type: 'string' } } + > = {}; + + for (const [name, header] of Object.entries(headers ?? {})) { + responseHeaders[name] = { + description: header.description, + required: true, + schema: { type: 'string' }, + }; + } + + return { + description: description ?? 'Operation successful.', + headers: Object.keys(responseHeaders).length ? responseHeaders : undefined, + content: { + [mediaType]: { schema: { type: 'string' as const, format: 'binary' } }, + }, + }; +} + +/** Documents a JSON success body from the route's response DTO, or a bare success when it has none. */ +export function buildJsonSuccessResponse( + responseDto: ResponseDtoClass | undefined, + resolveSchema: SchemaResolver, +) { + const hasResponseContent = responseDto && hasNamedSchema(responseDto); + + return { + description: 'Operation successful.', + content: hasResponseContent + ? { + 'application/json': { + schema: resolveSchema(responseDto, responseDto.schema), + }, + } + : undefined, + }; +} + /** * Response set is derived from what `PublicApiControllerRegistry` actually does at runtime, not * invented: the success status is the one `@ApiResponse` declares (and the same one the registry * sends), auth always 401s, `@ApiKeyScope` always 403s on mismatch, and a body/query DTO always * 400s on failed `.safeParse()`. Anything else - like a 404 from a business-rule lookup that isn't * visible in decorator metadata - has to be declared explicitly via `@ApiErrorResponse`. + * A binary `@ApiResponse` documents its media type, description and headers instead of a JSON DTO. */ function buildResponses( route: ResolvedPublicApiRoute, resolveSchema: SchemaResolver, ): RouteConfig['responses'] { const responses: RouteConfig['responses'] = { - [route.successStatus]: { - description: 'Operation successful.', - ...(route.responseDto && hasNamedSchema(route.responseDto) - ? { - content: { - 'application/json': { - schema: resolveSchema(route.responseDto, route.responseDto.schema), - }, - }, - } - : {}), - }, + [route.successStatus]: route.binaryResponse + ? buildBinarySuccessResponse(route.binaryResponse) + : buildJsonSuccessResponse(route.responseDto, resolveSchema), }; // If the route has a request body or query, we add an HTTP 400 as a possible response