fix(sessions): isolate folder save temp files (#1195)
* fix(sessions): isolate folder save temp files * fix(sessions): remove failed save temp files --------- Co-authored-by: Isaac Sanchez <isanchez-hawkins@arize.com>
This commit is contained in:
committed by
GitHub
co-authored by
Isaac Sanchez
parent
736e33ab67
commit
51e1967f68
@@ -43,13 +43,19 @@ export const registerSessionFoldersRoutes = (app, dependencies) => {
|
||||
if (Buffer.byteLength(serialized, 'utf8') > MAX_BODY_BYTES) {
|
||||
return res.status(413).json({ error: 'Payload too large' });
|
||||
}
|
||||
let tmp;
|
||||
let saved = false;
|
||||
try {
|
||||
await ensureDir();
|
||||
const tmp = `${filePath}.tmp`;
|
||||
tmp = `${filePath}.tmp-${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}`;
|
||||
await fsPromises.writeFile(tmp, serialized, 'utf8');
|
||||
await fsPromises.rename(tmp, filePath);
|
||||
saved = true;
|
||||
return res.json({ success: true });
|
||||
} catch (error) {
|
||||
if (tmp && !saved) {
|
||||
await fsPromises.unlink(tmp).catch(() => {});
|
||||
}
|
||||
const message = error instanceof Error ? error.message : 'Failed to write session folders';
|
||||
return res.status(500).json({ error: message });
|
||||
}
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import path from 'path';
|
||||
|
||||
import { registerSessionFoldersRoutes } from './routes.js';
|
||||
|
||||
const createRouteRegistry = () => {
|
||||
const routes = new Map();
|
||||
|
||||
return {
|
||||
app: {
|
||||
get(routePath, handler) {
|
||||
routes.set(`GET ${routePath}`, handler);
|
||||
},
|
||||
post(routePath, handler) {
|
||||
routes.set(`POST ${routePath}`, handler);
|
||||
},
|
||||
},
|
||||
getRoute(method, routePath) {
|
||||
return routes.get(`${method} ${routePath}`);
|
||||
},
|
||||
};
|
||||
};
|
||||
|
||||
const createMockResponse = () => {
|
||||
let statusCode = 200;
|
||||
let body = null;
|
||||
|
||||
return {
|
||||
status(code) {
|
||||
statusCode = code;
|
||||
return this;
|
||||
},
|
||||
json(payload) {
|
||||
body = payload;
|
||||
return this;
|
||||
},
|
||||
get statusCode() {
|
||||
return statusCode;
|
||||
},
|
||||
get body() {
|
||||
return body;
|
||||
},
|
||||
};
|
||||
};
|
||||
|
||||
describe('session folders routes', () => {
|
||||
it('uses unique temp files for concurrent saves', async () => {
|
||||
const { app, getRoute } = createRouteRegistry();
|
||||
const tempPaths = [];
|
||||
const fsPromises = {
|
||||
mkdir: vi.fn(async () => {}),
|
||||
writeFile: vi.fn(async (tempPath) => {
|
||||
tempPaths.push(tempPath);
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
}),
|
||||
rename: vi.fn(async () => {}),
|
||||
};
|
||||
|
||||
registerSessionFoldersRoutes(app, {
|
||||
fsPromises,
|
||||
path,
|
||||
openchamberDataDir: '/tmp/openchamber-test',
|
||||
});
|
||||
|
||||
const handler = getRoute('POST', '/api/session-folders');
|
||||
|
||||
await Promise.all([
|
||||
handler({ body: { version: 1, updatedAt: 1 } }, createMockResponse()),
|
||||
handler({ body: { version: 1, updatedAt: 2 } }, createMockResponse()),
|
||||
]);
|
||||
|
||||
expect(tempPaths).toHaveLength(2);
|
||||
expect(new Set(tempPaths).size).toBe(2);
|
||||
expect(tempPaths.every((tempPath) => tempPath.includes('sessions-directories.json.tmp-'))).toBe(true);
|
||||
});
|
||||
|
||||
it('removes the temp file when rename fails', async () => {
|
||||
const { app, getRoute } = createRouteRegistry();
|
||||
const fsPromises = {
|
||||
mkdir: vi.fn(async () => {}),
|
||||
writeFile: vi.fn(async () => {}),
|
||||
rename: vi.fn(async () => {
|
||||
throw new Error('rename failed');
|
||||
}),
|
||||
unlink: vi.fn(async () => {}),
|
||||
};
|
||||
|
||||
registerSessionFoldersRoutes(app, {
|
||||
fsPromises,
|
||||
path,
|
||||
openchamberDataDir: '/tmp/openchamber-test',
|
||||
});
|
||||
|
||||
const handler = getRoute('POST', '/api/session-folders');
|
||||
const response = createMockResponse();
|
||||
|
||||
await handler({ body: { version: 1, updatedAt: 1 } }, response);
|
||||
|
||||
expect(response.statusCode).toBe(500);
|
||||
expect(fsPromises.unlink).toHaveBeenCalledWith(expect.stringContaining('sessions-directories.json.tmp-'));
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user