diff --git a/.serena/memories/media-processor/core.md b/.serena/memories/media-processor/core.md index d9a8ad0fad..022744b5ae 100644 --- a/.serena/memories/media-processor/core.md +++ b/.serena/memories/media-processor/core.md @@ -62,7 +62,7 @@ media-processor/ - SVG: max input size enforced before parsing (default 2MB, `PENPOT_MEDIA_PROCESSOR_SVG_MAX_SIZE`); an oversized input is rejected with 413, never truncated - SVG workers: `PENPOT_MEDIA_PROCESSOR_SVG_WORKERS` (default 2; 0 runs inline), `PENPOT_MEDIA_PROCESSOR_SVG_WORKER_MAX_OLD_MB` (default 512, per-worker V8 old-generation cap) and `PENPOT_MEDIA_PROCESSOR_SVG_TIMEOUT` (default 30000 ms) - Font: prlimit wraps FontForge processes with memory (AS) and CPU time limits -- Concurrency: p-queue limits concurrent requests (default 10) +- Concurrency: p-queue limits concurrent requests (default 10). Image and font share one queue; SVG has **its own**, so an SVG request waiting for a free worker does not hold a queue slot that image or font requests need. The app wiring lives in `app.ts` (`createApp`), kept apart from `index.ts` so tests can build the real app. - Upload: hybrid storage — memory for files < 10MB, disk for larger; configurable via `PENPOT_MEDIA_PROCESSOR_MEMORY_THRESHOLD` - Max file size: configurable (default 350MB) diff --git a/media-processor/src/app.ts b/media-processor/src/app.ts new file mode 100644 index 0000000000..cdc98d0306 --- /dev/null +++ b/media-processor/src/app.ts @@ -0,0 +1,37 @@ +import express, { type Express } from "express"; +import { healthRoutes } from "./routes/health.js"; +import { createImageRoutes } from "./routes/image.js"; +import { createFontRoutes } from "./routes/font.js"; +import { createSvgRoutes } from "./routes/svg.js"; +import { errorHandler } from "./middleware/error-handler.js"; +import { timeoutMiddleware } from "./middleware/timeout.js"; +import { sharedKeyAuth } from "./middleware/auth.js"; +import { createQueueMiddleware } from "./middleware/queue.js"; +import { loggingMiddleware } from "./middleware/logging.js"; +import type { AppConfig } from "./types.js"; + +/** + * Build the HTTP app. Kept apart from `index.ts` (which loads the config, wires + * the services and listens) so tests can build the real app without starting a + * server. + */ +export function createApp(config: Pick): Express { + const app = express(); + + // Image and font share one queue. SVG gets its own: an SVG request waiting for + // a free worker holds its queue slot for its whole lifetime, and on a shared + // queue that wait would block image and font requests behind it. + const queueMiddleware = createQueueMiddleware(config.maxConcurrentRequests); + const svgQueueMiddleware = createQueueMiddleware(config.maxConcurrentRequests); + + app.use(timeoutMiddleware(config.requestTimeout)); + app.use(loggingMiddleware); + + app.get("/api/health", healthRoutes); + app.use("/api/image", sharedKeyAuth(config.sharedKey), queueMiddleware, createImageRoutes()); + app.use("/api/font", sharedKeyAuth(config.sharedKey), queueMiddleware, createFontRoutes()); + app.use("/api/svg", sharedKeyAuth(config.sharedKey), svgQueueMiddleware, createSvgRoutes()); + app.use(errorHandler); + + return app; +} diff --git a/media-processor/src/index.ts b/media-processor/src/index.ts index 867fe33078..2d4e8c9260 100644 --- a/media-processor/src/index.ts +++ b/media-processor/src/index.ts @@ -1,15 +1,6 @@ -import express, { type Express } from "express"; import { loadConfig } from "./config.js"; import { initLogger, logger, logActiveTransports } from "./logger.js"; -import { healthRoutes } from "./routes/health.js"; -import { createImageRoutes } from "./routes/image.js"; -import { createFontRoutes } from "./routes/font.js"; -import { createSvgRoutes } from "./routes/svg.js"; -import { errorHandler } from "./middleware/error-handler.js"; -import { timeoutMiddleware } from "./middleware/timeout.js"; -import { sharedKeyAuth } from "./middleware/auth.js"; -import { createQueueMiddleware } from "./middleware/queue.js"; -import { loggingMiddleware } from "./middleware/logging.js"; +import { createApp } from "./app.js"; import { configureImageLimits } from "./services/image.js"; import { configureFontLimits } from "./services/font.js"; import { configureSvgLimits } from "./services/svg.js"; @@ -27,7 +18,6 @@ sharp.cache(false); const config = loadConfig(); initLogger(config); -const app: Express = express(); // Configure resource limits configureImageLimits({ @@ -52,16 +42,7 @@ configureSvgPool({ configureUploadLimits({ maxFileSize: config.maxFileSize, memoryThreshold: config.memoryThreshold }); -const queueMiddleware = createQueueMiddleware(config.maxConcurrentRequests); - -app.use(timeoutMiddleware(config.requestTimeout)); -app.use(loggingMiddleware); - -app.get("/api/health", healthRoutes); -app.use("/api/image", sharedKeyAuth(config.sharedKey), queueMiddleware, createImageRoutes()); -app.use("/api/font", sharedKeyAuth(config.sharedKey), queueMiddleware, createFontRoutes()); -app.use("/api/svg", sharedKeyAuth(config.sharedKey), queueMiddleware, createSvgRoutes()); -app.use(errorHandler); +const app = createApp(config); app.listen(config.port, config.host, () => { logActiveTransports(logger); diff --git a/media-processor/test/queue-isolation.test.ts b/media-processor/test/queue-isolation.test.ts new file mode 100644 index 0000000000..6a34f8750e --- /dev/null +++ b/media-processor/test/queue-isolation.test.ts @@ -0,0 +1,74 @@ +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import request from "supertest"; +import sharp from "sharp"; +import { EventEmitter } from "node:events"; +import { createApp } from "../src/app.js"; +import { configureUploadLimits } from "../src/upload.js"; +import { configureSvgPool, shutdownSvgPool, setSvgWorkerFactory, type SvgWorker } from "../src/svg-pool.js"; + +// A worker that never answers, so an SVG request parks on the pool while it +// holds its queue slot. +class HangingWorker extends EventEmitter { + public postMessage(): void {} + public async terminate(): Promise { + this.emit("exit", 0); + return 0; + } +} + +const config = { sharedKey: "test-key", maxConcurrentRequests: 1, requestTimeout: 10000 }; + +describe("queue isolation", () => { + let app: ReturnType; + + beforeAll(() => { + configureUploadLimits({ maxFileSize: 10 * 1024 * 1024, memoryThreshold: 10 }); + setSvgWorkerFactory(() => new HangingWorker() as unknown as SvgWorker); + configureSvgPool({ workers: 1, maxOldGenerationSizeMb: 512, timeout: 5000 }); + app = createApp(config); + }); + + afterAll(async () => { + await shutdownSvgPool(); + setSvgWorkerFactory(null); + }); + + it("a pending SVG request does not block an image request", async () => { + const svgRequest = request(app) + .post("/api/svg/sanitize") + .set("x-shared-key", "test-key") + .attach("file", Buffer.from(``), { + filename: "x.svg", + contentType: "image/svg+xml", + }) + .then((res) => res.status) + .catch(() => 0); + + // Let the SVG request be admitted and reach the hanging worker. + await new Promise((resolve) => setTimeout(resolve, 100)); + + const image = await sharp({ + create: { width: 10, height: 10, channels: 3, background: { r: 1, g: 2, b: 3 } }, + }) + .jpeg() + .toBuffer(); + + const imageRequest = request(app) + .post("/api/image/info") + .set("x-shared-key", "test-key") + .attach("file", image, { filename: "i.jpg", contentType: "image/jpeg" }) + .then((res) => res.status) + .catch(() => 0); + + const outcome = await Promise.race([ + imageRequest, + new Promise<"timeout">((resolve) => setTimeout(() => resolve("timeout"), 2000)), + ]); + + expect(outcome).toBe(200); + + // Release the held SVG request so the test ends clean. + await shutdownSvgPool(); + await svgRequest; + }); +});