From 6c62331b023b27bda387d0354d1d251b4172b6a9 Mon Sep 17 00:00:00 2001 From: Gregor MacLennan Date: Mon, 23 Feb 2026 18:18:13 +0000 Subject: [PATCH 1/3] failing test for write error cleanup --- test/maps.test.ts | 47 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/test/maps.test.ts b/test/maps.test.ts index 3319341..18bc06b 100644 --- a/test/maps.test.ts +++ b/test/maps.test.ts @@ -6,7 +6,7 @@ import { setTimeout as delay } from 'node:timers/promises' import { fileURLToPath } from 'node:url' import { Reader } from 'styled-map-package' -import { describe, it, expect } from 'vitest' +import { describe, it, expect, vi } from 'vitest' import { DEMOTILES_Z2, @@ -570,6 +570,51 @@ describe('Map Upload', () => { expect(style).toEqual(expectedStyle) }) + it('should clean up temp file when write errors during upload', async (t) => { + const { localBaseUrl, customMapPath } = await startServer(t, { + customMapPath: nonExistentPath, + }) + const tmpDir = path.dirname(customMapPath) + const customMapBasename = path.basename(customMapPath) + + // Mock fs.createWriteStream to return a writable that errors on write, + // simulating a disk-full scenario + const originalCreateWriteStream = fs.createWriteStream + const spy = vi.spyOn(fs, 'createWriteStream').mockImplementation( + (...args: any[]) => { + const stream = originalCreateWriteStream.apply(fs, args) + stream._write = ( + _chunk: any, + _encoding: BufferEncoding, + callback: (error?: Error | null) => void, + ) => { + callback(new Error('ENOSPC: no space left on device')) + } + return stream + }, + ) + t.onTestFinished(() => spy.mockRestore()) + + const fileBuffer = fs.readFileSync(OSM_BRIGHT_Z6) + const response = await fetch(`${localBaseUrl}/maps/custom`, { + method: 'PUT', + body: fileBuffer, + headers: { 'Content-Type': 'application/octet-stream' }, + }) + + expect(response.status).toBe(500) + + // Allow time for async cleanup to complete + await delay(100) + + // Verify temp file was cleaned up + const filesInDir = fs.readdirSync(tmpDir) + const tempFiles = filesInDir.filter( + (f) => f.startsWith(customMapBasename) && f.includes('.download-'), + ) + expect(tempFiles).toHaveLength(0) + }) + it('should not leave temp files after successful upload', async (t) => { const { localBaseUrl, customMapPath } = await startServer(t, { customMapPath: nonExistentPath, From 0d27384e0da5827414db13f5dc782db03d16995b Mon Sep 17 00:00:00 2001 From: Gregor MacLennan Date: Mon, 23 Feb 2026 18:28:52 +0000 Subject: [PATCH 2/3] cleanup temp files on write error during import --- src/context.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/context.ts b/src/context.ts index 1b3a127..d55ccfb 100644 --- a/src/context.ts +++ b/src/context.ts @@ -130,11 +130,21 @@ export class Context { const writer = writable.getWriter() return new WritableStream({ async write(chunk) { - await writer.write(chunk) + try { + await writer.write(chunk) + } catch (err) { + await fsPromises.unlink(tempPath).catch(noop) + throw err + } }, close: async () => { // Finish writing to the temp file - await writer.close() + try { + await writer.close() + } catch (err) { + await fsPromises.unlink(tempPath).catch(noop) + throw err + } // Validate the uploaded map file BEFORE replacing the existing one const tempReader = new Reader(tempPath) From 726922d92e7ca0ea435c18a2da07bab511b62541 Mon Sep 17 00:00:00 2001 From: Gregor MacLennan Date: Mon, 23 Feb 2026 18:34:32 +0000 Subject: [PATCH 3/3] also test cleanup on close error --- test/maps.test.ts | 45 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/test/maps.test.ts b/test/maps.test.ts index 18bc06b..11c2d3c 100644 --- a/test/maps.test.ts +++ b/test/maps.test.ts @@ -582,7 +582,7 @@ describe('Map Upload', () => { const originalCreateWriteStream = fs.createWriteStream const spy = vi.spyOn(fs, 'createWriteStream').mockImplementation( (...args: any[]) => { - const stream = originalCreateWriteStream.apply(fs, args) + const stream = originalCreateWriteStream.apply(fs, args as any) stream._write = ( _chunk: any, _encoding: BufferEncoding, @@ -615,6 +615,49 @@ describe('Map Upload', () => { expect(tempFiles).toHaveLength(0) }) + it('should clean up temp file when close errors during upload', async (t) => { + const { localBaseUrl, customMapPath } = await startServer(t, { + customMapPath: nonExistentPath, + }) + const tmpDir = path.dirname(customMapPath) + const customMapBasename = path.basename(customMapPath) + + // Mock fs.createWriteStream to return a writable that writes successfully + // but errors on close (_final), simulating a flush/sync failure + const originalCreateWriteStream = fs.createWriteStream + const spy = vi.spyOn(fs, 'createWriteStream').mockImplementation( + (...args: any[]) => { + const stream = originalCreateWriteStream.apply(fs, args as any) + stream._final = ( + callback: (error?: Error | null) => void, + ) => { + callback(new Error('EIO: i/o error')) + } + return stream + }, + ) + t.onTestFinished(() => spy.mockRestore()) + + const fileBuffer = fs.readFileSync(OSM_BRIGHT_Z6) + const response = await fetch(`${localBaseUrl}/maps/custom`, { + method: 'PUT', + body: fileBuffer, + headers: { 'Content-Type': 'application/octet-stream' }, + }) + + expect(response.status).toBe(500) + + // Allow time for async cleanup to complete + await delay(100) + + // Verify temp file was cleaned up + const filesInDir = fs.readdirSync(tmpDir) + const tempFiles = filesInDir.filter( + (f) => f.startsWith(customMapBasename) && f.includes('.download-'), + ) + expect(tempFiles).toHaveLength(0) + }) + it('should not leave temp files after successful upload', async (t) => { const { localBaseUrl, customMapPath } = await startServer(t, { customMapPath: nonExistentPath,