fix(lobby): ensure atomic lobby and canvas creation with cleanup

Author: matteosusca

server/src/models/Lobby.ts

@@ -51,7 +51,6 @@ lobbySchema.statics.createWithCanvas = async function (name: string, ownerId?: s
   const lobbyId = new Types.ObjectId();
   const canvasId = new Types.ObjectId();
 
-  // Initialized to 0 (Transparent/White depending on palette)
   const emptyBuffer = Buffer.alloc(width * height, 0);
 
   const canvas = new Canvas({
@@ -72,9 +71,15 @@ lobbySchema.statics.createWithCanvas = async function (name: string, ownerId?: s
     canvas: canvasId
   });
 
-  // Save both
-  await Promise.all([canvas.save(), lobby.save()]);
-  return lobby;
+  try {
+    await canvas.save();
+    await lobby.save();
+    return lobby;
+  } catch (error) {
+    // Cleanup canvas if lobby save fails
+    await Canvas.findByIdAndDelete(canvasId);
+    throw error;
+  }
 };
 
 export const Lobby = model<ILobby, LobbyModel>('Lobby', lobbySchema);

server/test/lobby.creation.test.ts

@@ -0,0 +1,34 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { Lobby } from '../src/models/Lobby.js';
+import { Canvas } from '../src/models/Canvas.js';
+
+describe('Lobby Model - createWithCanvas Atomicity', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it('should not leave an orphaned Canvas if Lobby save fails', async () => {
+    // 1. Mock Canvas.prototype.save to succeed
+    const canvasSaveSpy = vi.spyOn(Canvas.prototype, 'save').mockResolvedValue({} as any);
+    
+    // 2. Mock Lobby.prototype.save to fail (e.g., duplicate name)
+    const lobbySaveSpy = vi.spyOn(Lobby.prototype, 'save').mockRejectedValue(new Error('Duplicate name'));
+    
+    // 3. Mock Canvas.findByIdAndDelete for cleanup verification
+    const canvasDeleteSpy = vi.spyOn(Canvas, 'findByIdAndDelete').mockResolvedValue({} as any);
+
+    // Attempt creation
+    try {
+      await Lobby.createWithCanvas('Failing Lobby');
+    } catch (error: any) {
+      expect(error.message).toBe('Duplicate name');
+    }
+
+    // Verify it attempted to save both
+    expect(canvasSaveSpy).toHaveBeenCalled();
+    expect(lobbySaveSpy).toHaveBeenCalled();
+
+    // MANDATORY: Check if cleanup was triggered
+    expect(canvasDeleteSpy).toHaveBeenCalled();
+  });
+});