From 8bdfdb0c179c5537dda4281ac93d6b11e6520b18 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 11 Aug 2026 21:32:46 -0700 Subject: [PATCH 1/2] fix(files): restore CSV preview cancellation --- .../files/[fileId]/csv-preview/route.test.ts | 61 +++++++++++++++++++ .../[id]/files/[fileId]/csv-preview/route.ts | 3 +- .../file-parsers/csv-preview-slice.test.ts | 28 +++++++++ .../sim/lib/file-parsers/csv-preview-slice.ts | 4 ++ 4 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 apps/sim/app/api/workspaces/[id]/files/[fileId]/csv-preview/route.test.ts diff --git a/apps/sim/app/api/workspaces/[id]/files/[fileId]/csv-preview/route.test.ts b/apps/sim/app/api/workspaces/[id]/files/[fileId]/csv-preview/route.test.ts new file mode 100644 index 00000000000..af088a4cb0a --- /dev/null +++ b/apps/sim/app/api/workspaces/[id]/files/[fileId]/csv-preview/route.test.ts @@ -0,0 +1,61 @@ +/** + * @vitest-environment node + */ +import { authMockFns } from '@sim/testing' +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + getSlice: vi.fn(), + readFile: vi.fn(), +})) + +vi.mock('@/lib/file-parsers/csv-preview-slice', () => ({ + getCsvPreviewSlice: mocks.getSlice, +})) + +vi.mock('@/lib/workspace-files/application/read-workspace-file-record', () => ({ + readWorkspaceFileContentRecord: { + operation: { id: 'files.read_content', minimumRole: 'read', workspaceApiKey: 'allow' }, + execute: mocks.readFile, + }, +})) + +import { GET } from '@/app/api/workspaces/[id]/files/[fileId]/csv-preview/route' + +const WORKSPACE_ID = '7727ef3f-8cf6-4686-b063-2bb006a10785' +const FILE_ID = 'wf_csv' +const KEY = `workspace/${WORKSPACE_ID}/large.csv` +const USER = { id: 'user-1' } +const context = { params: Promise.resolve({ id: WORKSPACE_ID, fileId: FILE_ID }) } + +function request(): NextRequest { + return new NextRequest( + `http://localhost/api/workspaces/${WORKSPACE_ID}/files/${FILE_ID}/csv-preview?key=${encodeURIComponent(KEY)}` + ) +} + +describe('GET /api/workspaces/[id]/files/[fileId]/csv-preview', () => { + beforeEach(() => { + vi.clearAllMocks() + authMockFns.mockGetSession.mockResolvedValue({ user: USER, session: { id: 'session-1' } }) + mocks.readFile.mockResolvedValue({ file: { id: FILE_ID, key: KEY } }) + mocks.getSlice.mockResolvedValue({ + headers: ['name'], + rows: [['Ada']], + truncated: false, + }) + }) + + it('propagates client cancellation to the storage preview read', async () => { + const req = request() + const response = await GET(req, context) + + expect(response.status).toBe(200) + expect(mocks.getSlice).toHaveBeenCalledWith({ + key: KEY, + context: 'workspace', + signal: req.signal, + }) + }) +}) diff --git a/apps/sim/app/api/workspaces/[id]/files/[fileId]/csv-preview/route.ts b/apps/sim/app/api/workspaces/[id]/files/[fileId]/csv-preview/route.ts index 802191d380f..3fe39b1360b 100644 --- a/apps/sim/app/api/workspaces/[id]/files/[fileId]/csv-preview/route.ts +++ b/apps/sim/app/api/workspaces/[id]/files/[fileId]/csv-preview/route.ts @@ -15,10 +15,11 @@ export const GET = defineInternalJsonRoute({ operation: csvPreviewWorkspaceFile.operation, rateLimit: internalRateLimits.none({ reason: 'Preserve existing internal CSV preview behavior' }), errorPolicy: internalFileErrorPolicies.default, - mapInput: ({ params, query }) => ({ + mapInput: ({ params, query }, { request }) => ({ fileId: params.fileId, assertedWorkspaceId: params.id, key: query.key, + signal: request.signal, }), useCase: csvPreviewWorkspaceFile, onSuccess: ({ result }) => { diff --git a/apps/sim/lib/file-parsers/csv-preview-slice.test.ts b/apps/sim/lib/file-parsers/csv-preview-slice.test.ts index 51d8574e46d..a69b6adba1b 100644 --- a/apps/sim/lib/file-parsers/csv-preview-slice.test.ts +++ b/apps/sim/lib/file-parsers/csv-preview-slice.test.ts @@ -100,4 +100,32 @@ describe('getCsvPreviewSlice', () => { expect(slice.truncated).toBe(true) expect(destroySpy).toHaveBeenCalled() }) + + it('destroys a source acquired after the request was already aborted', async () => { + const source = streamOf('a,b\n1,2\n') + const destroySpy = vi.spyOn(source, 'destroy') + const controller = new AbortController() + controller.abort() + mockDownloadFileStream.mockResolvedValue(source) + + await expect(getCsvPreviewSlice({ ...args, signal: controller.signal })).rejects.toMatchObject({ + name: 'AbortError', + }) + expect(destroySpy).toHaveBeenCalled() + }) + + it('destroys an active source when the request is aborted', async () => { + const read = vi.fn() + const source = new Readable({ read }) + const destroySpy = vi.spyOn(source, 'destroy') + const controller = new AbortController() + mockDownloadFileStream.mockResolvedValue(source) + + const preview = getCsvPreviewSlice({ ...args, signal: controller.signal }) + await vi.waitFor(() => expect(read).toHaveBeenCalled()) + controller.abort() + + await expect(preview).rejects.toMatchObject({ code: 'ERR_STREAM_PREMATURE_CLOSE' }) + expect(destroySpy).toHaveBeenCalled() + }) }) diff --git a/apps/sim/lib/file-parsers/csv-preview-slice.ts b/apps/sim/lib/file-parsers/csv-preview-slice.ts index f7053119950..f9c9f07c47f 100644 --- a/apps/sim/lib/file-parsers/csv-preview-slice.ts +++ b/apps/sim/lib/file-parsers/csv-preview-slice.ts @@ -52,6 +52,10 @@ export async function getCsvPreviewSlice({ signal, }: CsvPreviewSliceArgs): Promise { const source = await downloadFileStream({ key, context }) + if (signal?.aborted) { + source.destroy() + signal.throwIfAborted() + } const onAbort = () => source.destroy() signal?.addEventListener('abort', onAbort, { once: true }) From 932a7d4068bc29eea09237509177e937afc07fd8 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 11 Aug 2026 21:44:18 -0700 Subject: [PATCH 2/2] fix --- .../files/[fileId]/csv-preview/route.test.ts | 24 ++++++++++- .../api/server/routes/internal-json-route.ts | 4 ++ .../api/server/routes/v2-json-route.test.ts | 22 +++++++++- .../lib/api/server/routes/v2-json-route.ts | 1 + .../lib/core/utils/with-route-handler.test.ts | 42 +++++++++++++++++++ apps/sim/lib/core/utils/with-route-handler.ts | 12 ++++++ .../file-parsers/csv-preview-slice.test.ts | 2 +- .../sim/lib/file-parsers/csv-preview-slice.ts | 3 ++ 8 files changed, 106 insertions(+), 4 deletions(-) diff --git a/apps/sim/app/api/workspaces/[id]/files/[fileId]/csv-preview/route.test.ts b/apps/sim/app/api/workspaces/[id]/files/[fileId]/csv-preview/route.test.ts index af088a4cb0a..4bb7e2aa1d7 100644 --- a/apps/sim/app/api/workspaces/[id]/files/[fileId]/csv-preview/route.test.ts +++ b/apps/sim/app/api/workspaces/[id]/files/[fileId]/csv-preview/route.test.ts @@ -29,9 +29,10 @@ const KEY = `workspace/${WORKSPACE_ID}/large.csv` const USER = { id: 'user-1' } const context = { params: Promise.resolve({ id: WORKSPACE_ID, fileId: FILE_ID }) } -function request(): NextRequest { +function request(signal?: AbortSignal): NextRequest { return new NextRequest( - `http://localhost/api/workspaces/${WORKSPACE_ID}/files/${FILE_ID}/csv-preview?key=${encodeURIComponent(KEY)}` + `http://localhost/api/workspaces/${WORKSPACE_ID}/files/${FILE_ID}/csv-preview?key=${encodeURIComponent(KEY)}`, + { signal } ) } @@ -58,4 +59,23 @@ describe('GET /api/workspaces/[id]/files/[fileId]/csv-preview', () => { signal: req.signal, }) }) + + it('returns a cancellation response when the client disconnects during the storage read', async () => { + const controller = new AbortController() + const req = request(controller.signal) + mocks.getSlice.mockImplementation(async () => { + controller.abort() + throw Object.assign(new Error('Premature close'), { + code: 'ERR_STREAM_PREMATURE_CLOSE', + }) + }) + + const response = await GET(req, context) + + expect(response.status).toBe(499) + await expect(response.json()).resolves.toMatchObject({ + error: 'Client cancelled request', + requestId: expect.any(String), + }) + }) }) diff --git a/apps/sim/lib/api/server/routes/internal-json-route.ts b/apps/sim/lib/api/server/routes/internal-json-route.ts index 4d85af2eb51..70d6bb0601c 100644 --- a/apps/sim/lib/api/server/routes/internal-json-route.ts +++ b/apps/sim/lib/api/server/routes/internal-json-route.ts @@ -365,6 +365,10 @@ export function defineInternalJsonRoute< } }, { + clientAbortResponse: ({ requestId }) => + createJsonErrorResponse( + internalErrorResponse(499, { error: 'Client cancelled request', requestId }) + ), typedErrorResponse: ({ error, status, requestId }) => NextResponse.json({ error: error.message, requestId }, { status }), unhandledErrorResponse: () => diff --git a/apps/sim/lib/api/server/routes/v2-json-route.test.ts b/apps/sim/lib/api/server/routes/v2-json-route.test.ts index 62b6e8f6edf..909418135e5 100644 --- a/apps/sim/lib/api/server/routes/v2-json-route.test.ts +++ b/apps/sim/lib/api/server/routes/v2-json-route.test.ts @@ -116,11 +116,12 @@ function createHandler(overrides: HandlerOverrides = {}) { }) } -function request(body: unknown = { value: 'ok' }): NextRequest { +function request(body: unknown = { value: 'ok' }, signal?: AbortSignal): NextRequest { return new NextRequest('http://localhost/api/v2/widgets', { method: 'POST', headers: { 'content-type': 'application/json', 'x-api-key': 'secret' }, body: JSON.stringify(body), + signal, }) } @@ -395,6 +396,25 @@ describe('defineV2JsonRoute', () => { expect(response.headers.get('X-RateLimit-Remaining')).toBe('99') }) + it('renders a client disconnect through the v2 cancellation envelope', async () => { + const controller = new AbortController() + const response = await createHandler({ + execute: async () => { + controller.abort() + throw Object.assign(new Error('Premature close'), { + code: 'ERR_STREAM_PREMATURE_CLOSE', + }) + }, + })(request(undefined, controller.signal)) + + expect(response.status).toBe(499) + await expect(response.json()).resolves.toEqual({ + error: { code: 'CLIENT_CLOSED_REQUEST', message: 'Client cancelled request' }, + }) + expect(response.headers.get('Cache-Control')).toBe('private, no-store') + expect(response.headers.get('X-RateLimit-Remaining')).toBe('99') + }) + it('validates the presented response before onSuccess', async () => { const onSuccess = vi.fn() const response = await createHandler({ diff --git a/apps/sim/lib/api/server/routes/v2-json-route.ts b/apps/sim/lib/api/server/routes/v2-json-route.ts index 434d46c9918..20261375568 100644 --- a/apps/sim/lib/api/server/routes/v2-json-route.ts +++ b/apps/sim/lib/api/server/routes/v2-json-route.ts @@ -287,6 +287,7 @@ export function defineV2JsonRoute< } }, { + clientAbortResponse: () => v2Error('CLIENT_CLOSED_REQUEST', 'Client cancelled request'), typedErrorResponse: ({ error }) => v2HttpError(error), unhandledErrorResponse: ({ error }) => error instanceof V2RouteInfrastructureError diff --git a/apps/sim/lib/core/utils/with-route-handler.test.ts b/apps/sim/lib/core/utils/with-route-handler.test.ts index c69b4f38fdc..0ac7b9cd6d6 100644 --- a/apps/sim/lib/core/utils/with-route-handler.test.ts +++ b/apps/sim/lib/core/utils/with-route-handler.test.ts @@ -2,6 +2,7 @@ * @vitest-environment node */ +import { loggerMock } from '@sim/testing' import { NextRequest, NextResponse } from 'next/server' import { describe, expect, it, vi } from 'vitest' import { HttpError } from '@/lib/core/utils/http-error' @@ -17,6 +18,47 @@ class TestHttpError extends HttpError { } describe('withRouteHandler', () => { + it('classifies errors after a client disconnect without using the unhandled fallback', async () => { + const routeHandlerLogger = vi.mocked(loggerMock.createLogger).mock.results[ + vi.mocked(loggerMock.createLogger).mock.calls.findIndex(([name]) => name === 'RouteHandler') + ]?.value + routeHandlerLogger?.info.mockClear() + routeHandlerLogger?.error.mockClear() + + const controller = new AbortController() + const clientAbortResponse = vi.fn(() => + NextResponse.json({ error: 'Client cancelled request' }, { status: 499 }) + ) + const unhandledErrorResponse = vi.fn(() => + NextResponse.json({ error: 'Internal server error' }, { status: 500 }) + ) + const handler = withRouteHandler( + async () => { + controller.abort() + throw Object.assign(new Error('Premature close'), { + code: 'ERR_STREAM_PREMATURE_CLOSE', + }) + }, + { clientAbortResponse, unhandledErrorResponse } + ) + + const response = await handler( + new NextRequest('http://localhost/api/test', { signal: controller.signal }), + undefined + ) + + expect(response.status).toBe(499) + await expect(response.json()).resolves.toEqual({ error: 'Client cancelled request' }) + expect(clientAbortResponse).toHaveBeenCalledOnce() + expect(unhandledErrorResponse).not.toHaveBeenCalled() + expect(routeHandlerLogger?.error).not.toHaveBeenCalled() + expect(routeHandlerLogger?.info).toHaveBeenCalledWith('Client closed request', { + duration: expect.any(Number), + status: 499, + }) + expect(response.headers.get('x-request-id')).toBeTruthy() + }) + it('lets a route family render a typed error before its generic fallback', async () => { const unhandledErrorResponse = vi.fn(() => NextResponse.json({ family: 'generic' }, { status: 500 }) diff --git a/apps/sim/lib/core/utils/with-route-handler.ts b/apps/sim/lib/core/utils/with-route-handler.ts index 6313437e6a4..0b38c86467c 100644 --- a/apps/sim/lib/core/utils/with-route-handler.ts +++ b/apps/sim/lib/core/utils/with-route-handler.ts @@ -25,6 +25,7 @@ interface RouteHandlerTypedErrorContext { } interface RouteHandlerOptions { + clientAbortResponse?: (context: RouteHandlerErrorContext) => NextResponse | Response typedErrorResponse?: (context: RouteHandlerTypedErrorContext) => NextResponse | Response unhandledErrorResponse?: (context: RouteHandlerErrorContext) => NextResponse | Response } @@ -78,7 +79,9 @@ function applyResponseHeaders( * - Generates a unique request ID and stores it in AsyncLocalStorage so every * logger in the request lifecycle automatically includes it * - Logs all 4xx and 5xx responses with method, path, status, duration + * - Classifies errors after a client disconnect as a normal 499 cancellation * - Catches unhandled errors, logs them, and returns a 500 with the request ID + * - Supports a route-family-specific client-abort response envelope * - Supports a route-family-specific unhandled-error response envelope * - Attaches `x-request-id`, plus the rate-limit headers when the route * recorded a snapshot for the request @@ -101,6 +104,15 @@ export function withRouteHandler( } catch (error) { const duration = Date.now() - startTime const message = getErrorMessage(error, 'Unknown error') + if (request.signal.aborted) { + logger.info('Client closed request', { duration, status: 499 }) + response = options.clientAbortResponse + ? options.clientAbortResponse({ error, requestId }) + : new Response(null, { status: 499 }) + applyResponseHeaders(response, request, requestId) + return response + } + const typedError = readTypedError(error) if (typedError) { const typedStatus = typedError.statusCode diff --git a/apps/sim/lib/file-parsers/csv-preview-slice.test.ts b/apps/sim/lib/file-parsers/csv-preview-slice.test.ts index a69b6adba1b..ee58d4a8282 100644 --- a/apps/sim/lib/file-parsers/csv-preview-slice.test.ts +++ b/apps/sim/lib/file-parsers/csv-preview-slice.test.ts @@ -125,7 +125,7 @@ describe('getCsvPreviewSlice', () => { await vi.waitFor(() => expect(read).toHaveBeenCalled()) controller.abort() - await expect(preview).rejects.toMatchObject({ code: 'ERR_STREAM_PREMATURE_CLOSE' }) + await expect(preview).rejects.toMatchObject({ name: 'AbortError' }) expect(destroySpy).toHaveBeenCalled() }) }) diff --git a/apps/sim/lib/file-parsers/csv-preview-slice.ts b/apps/sim/lib/file-parsers/csv-preview-slice.ts index f9c9f07c47f..2f4c9369ed9 100644 --- a/apps/sim/lib/file-parsers/csv-preview-slice.ts +++ b/apps/sim/lib/file-parsers/csv-preview-slice.ts @@ -137,6 +137,9 @@ export async function getCsvPreviewSlice({ piped.destroy() parser.destroy() return { headers, rows, truncated } + } catch (error) { + if (signal?.aborted) signal.throwIfAborted() + throw error } finally { signal?.removeEventListener('abort', onAbort) source.destroy()