From 3e3d4c760933f87bb361ecf194eca22317722be6 Mon Sep 17 00:00:00 2001 From: Carsten Hensiek Date: Tue, 19 May 2026 11:19:02 +0200 Subject: [PATCH] fix(pull-zone): Roll back bunny zone on creation/hostname failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wraps createPullZone and addCustomHostname multi-step flows in best-effort rollback try/catches. If any step after the initial bunny.createPullZone fails (setEuMode, addHostname, enableFreeSsl, or the DB-insert), the just-created bunny zone is removed so the user can retry cleanly — except when the zone was adopted (already existed in the user's account; we leave their data alone). Happy path is byte-identical; 5 new unit tests cover rollback, adoption skip, and cleanup-failure-doesn't-mask-original. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/domain/pull-zone.ts | 104 ++++++++++++------- tests/unit/create-pull-zone.test.ts | 150 +++++++++++++++++++++++++++- 2 files changed, 216 insertions(+), 38 deletions(-) diff --git a/src/domain/pull-zone.ts b/src/domain/pull-zone.ts index fbb085f..b5acb83 100644 --- a/src/domain/pull-zone.ts +++ b/src/domain/pull-zone.ts @@ -151,38 +151,54 @@ export async function createPullZone( log.info(`Creating pull zone for instance ${extensionInstanceId} (apiKey: [REDACTED])`) const result = await bunny.createPullZone({ name: data.name, originUrl: data.originUrl, apiKey }) - if (data.euOnly) { - await bunny.setEuMode(result.id, true, apiKey) - } - - const customHostname = resolveCustomHostname(data) - if (customHostname) { - await registerCustomHostnameAtBunny(result.id, data.cdnMode, customHostname, apiKey) - } + try { + if (data.euOnly) { + await bunny.setEuMode(result.id, true, apiKey) + } - const dnsConfigured = customHostname - ? await tryConfigureDnsCname(dnsClient, contextId, data.domain, customHostname, result.cdnDomain) - : false + const customHostname = resolveCustomHostname(data) + if (customHostname) { + await registerCustomHostnameAtBunny(result.id, data.cdnMode, customHostname, apiKey) + } - db.insert(pullZones) - .values({ + const dnsConfigured = customHostname + ? await tryConfigureDnsCname(dnsClient, contextId, data.domain, customHostname, result.cdnDomain) + : false + + db.insert(pullZones) + .values({ + id: result.id, + instanceId: extensionInstanceId, + cdnDomain: result.cdnDomain, + originUrl: data.originUrl, + cdnMode: data.cdnMode, + customHostname, + createdAt: new Date(), + }) + .run() + + return { id: result.id, - instanceId: extensionInstanceId, cdnDomain: result.cdnDomain, originUrl: data.originUrl, cdnMode: data.cdnMode, + dnsConfigured, customHostname, - createdAt: new Date(), - }) - .run() - - return { - id: result.id, - cdnDomain: result.cdnDomain, - originUrl: data.originUrl, - cdnMode: data.cdnMode, - dnsConfigured, - customHostname, + } + } catch (err) { + // Adopted zones pre-date our call — leave them alone, the user already owns them. + if (!result.adopted) { + try { + await bunny.deletePullZone(result.id, apiKey) + log.info(`Rolled back bunny pull zone ${result.id} after creation failure`) + } catch (cleanupErr) { + log.error( + `Rollback of bunny pull zone ${result.id} failed — manual cleanup required`, + cleanupErr instanceof Error ? cleanupErr.message : cleanupErr, + ) + } + } + throw err } } @@ -215,23 +231,37 @@ export async function addCustomHostname( const customHostname = `cdn.${domain}` await bunny.addHostname(pullZone.id, customHostname, apiKey) - await bunny.enableFreeSsl(pullZone.id, customHostname, apiKey) - let dnsConfigured = false - if (dnsClient) { + try { + await bunny.enableFreeSsl(pullZone.id, customHostname, apiKey) + + let dnsConfigured = false + if (dnsClient) { + try { + dnsConfigured = await configureDnsCname(dnsClient, contextId, domain, customHostname, pullZone.cdnDomain) + } catch (dnsError) { + log.warn( + '[domain] addCustomHostname auto-DNS failed (non-fatal):', + dnsError instanceof Error ? dnsError.message : dnsError, + ) + } + } + + db.update(pullZones).set({ customHostname }).where(eq(pullZones.instanceId, extensionInstanceId)).run() + + return { customHostname, dnsConfigured } + } catch (err) { try { - dnsConfigured = await configureDnsCname(dnsClient, contextId, domain, customHostname, pullZone.cdnDomain) - } catch (dnsError) { - log.warn( - '[domain] addCustomHostname auto-DNS failed (non-fatal):', - dnsError instanceof Error ? dnsError.message : dnsError, + await bunny.removeHostname(pullZone.id, customHostname, apiKey) + log.info(`Rolled back custom hostname ${customHostname} on pull zone ${pullZone.id} after failure`) + } catch (cleanupErr) { + log.error( + `Rollback of custom hostname ${customHostname} failed — manual cleanup required`, + cleanupErr instanceof Error ? cleanupErr.message : cleanupErr, ) } + throw err } - - db.update(pullZones).set({ customHostname }).where(eq(pullZones.instanceId, extensionInstanceId)).run() - - return { customHostname, dnsConfigured } } /** diff --git a/tests/unit/create-pull-zone.test.ts b/tests/unit/create-pull-zone.test.ts index bd5f5db..54c1b6a 100644 --- a/tests/unit/create-pull-zone.test.ts +++ b/tests/unit/create-pull-zone.test.ts @@ -5,13 +5,14 @@ import { extensionInstances, pullZones } from '~/server/db/schema' import { createTestDb, seedInstance } from '../helpers/db' vi.mock('~/server/bunnycdn.js', () => ({ - createPullZone: vi.fn().mockResolvedValue({ id: 42, name: 'test', cdnDomain: 'test.b-cdn.net' }), + createPullZone: vi.fn().mockResolvedValue({ id: 42, name: 'test', cdnDomain: 'test.b-cdn.net', adopted: false }), setupFullSiteCdn: vi.fn().mockResolvedValue(undefined), setEuOnly: vi.fn().mockResolvedValue(undefined), setEuMode: vi.fn().mockResolvedValue(undefined), addHostname: vi.fn().mockResolvedValue(undefined), removeHostname: vi.fn().mockResolvedValue(undefined), enableFreeSsl: vi.fn().mockResolvedValue(undefined), + deletePullZone: vi.fn().mockResolvedValue(undefined), })) process.env.ENCRYPTION_MASTER_PASSWORD = 'test-password' @@ -150,6 +151,120 @@ describe('domain/createPullZone', () => { }) }) +describe('domain/createPullZone rollback on failure', () => { + beforeEach(async () => { + vi.clearAllMocks() + }) + + async function seedWithApiKey() { + const db = createTestDb() + seedInstance(db) + const { encrypt } = await import('~/server/crypto') + db.update(extensionInstances) + .set({ encryptedApiKey: encrypt('test-key') }) + .where(eq(extensionInstances.id, 'inst-1')) + .run() + return db + } + + it('rolls back bunny zone when setEuMode fails', async () => { + const db = await seedWithApiKey() + const bunny = await import('~/server/bunnycdn') + vi.mocked(bunny.createPullZone).mockResolvedValueOnce({ + id: 42, + name: 'test', + cdnDomain: 'test.b-cdn.net', + adopted: false, + }) + vi.mocked(bunny.setEuMode).mockRejectedValueOnce(new Error('bunny 500')) + + await expect( + createPullZone(db, 'inst-1', 'project-1', { + name: 'testzone', + originUrl: 'https://example.com', + cdnMode: 'asset', + euOnly: true, + customHostnameEnabled: false, + }), + ).rejects.toThrow('bunny 500') + + expect(bunny.deletePullZone).toHaveBeenCalledWith(42, expect.any(String)) + expect(db.select().from(pullZones).where(eq(pullZones.instanceId, 'inst-1')).get()).toBeUndefined() + }) + + it('rolls back when addHostname fails', async () => { + const db = await seedWithApiKey() + const bunny = await import('~/server/bunnycdn') + vi.mocked(bunny.createPullZone).mockResolvedValueOnce({ + id: 42, + name: 'test', + cdnDomain: 'test.b-cdn.net', + adopted: false, + }) + vi.mocked(bunny.addHostname).mockRejectedValueOnce(new Error('hostname conflict')) + + await expect( + createPullZone(db, 'inst-1', 'project-1', { + name: 'testzone', + originUrl: 'https://example.com', + cdnMode: 'asset', + domain: 'example.com', + }), + ).rejects.toThrow('hostname conflict') + + expect(bunny.deletePullZone).toHaveBeenCalledWith(42, expect.any(String)) + }) + + it('does NOT roll back when zone was adopted (pre-existed at bunny)', async () => { + const db = await seedWithApiKey() + const bunny = await import('~/server/bunnycdn') + vi.mocked(bunny.createPullZone).mockResolvedValueOnce({ + id: 42, + name: 'test', + cdnDomain: 'test.b-cdn.net', + adopted: true, + }) + vi.mocked(bunny.setEuMode).mockRejectedValueOnce(new Error('bunny 500')) + + await expect( + createPullZone(db, 'inst-1', 'project-1', { + name: 'testzone', + originUrl: 'https://example.com', + cdnMode: 'asset', + euOnly: true, + customHostnameEnabled: false, + }), + ).rejects.toThrow('bunny 500') + + expect(bunny.deletePullZone).not.toHaveBeenCalled() + }) + + it('cleanup failure does not mask the original error', async () => { + const db = await seedWithApiKey() + const bunny = await import('~/server/bunnycdn') + vi.mocked(bunny.createPullZone).mockResolvedValueOnce({ + id: 42, + name: 'test', + cdnDomain: 'test.b-cdn.net', + adopted: false, + }) + vi.mocked(bunny.setEuMode).mockRejectedValueOnce(new Error('original failure')) + vi.mocked(bunny.deletePullZone).mockRejectedValueOnce(new Error('cleanup also failed')) + + await expect( + createPullZone(db, 'inst-1', 'project-1', { + name: 'testzone', + originUrl: 'https://example.com', + cdnMode: 'asset', + euOnly: true, + customHostnameEnabled: false, + }), + ).rejects.toThrow('original failure') + + expect(bunny.deletePullZone).toHaveBeenCalledWith(42, expect.any(String)) + }) +}) + describe('domain/createPullZone custom hostname toggle', () => { beforeEach(async () => { vi.clearAllMocks() @@ -296,6 +411,39 @@ describe('domain/addCustomHostname', () => { }) }) + it('rolls back addHostname when enableFreeSsl fails', async () => { + const db = createTestDb() + seedInstance(db) + + const { encrypt } = await import('~/server/crypto') + db.update(extensionInstances) + .set({ encryptedApiKey: encrypt('test-key') }) + .where(eq(extensionInstances.id, 'inst-1')) + .run() + + db.insert(pullZones) + .values({ + id: 99, + instanceId: 'inst-1', + cdnDomain: 'xyz.b-cdn.net', + originUrl: 'https://example.com', + cdnMode: 'asset', + customHostname: null, + createdAt: new Date(), + }) + .run() + + const bunny = await import('~/server/bunnycdn') + vi.mocked(bunny.enableFreeSsl).mockRejectedValueOnce(new Error('ssl provisioning failed')) + + await expect(addCustomHostname(db, 'inst-1', 'project-1', 'example.com')).rejects.toThrow('ssl provisioning failed') + + expect(bunny.addHostname).toHaveBeenCalledWith(99, 'cdn.example.com', expect.any(String)) + expect(bunny.removeHostname).toHaveBeenCalledWith(99, 'cdn.example.com', expect.any(String)) + const row = db.select().from(pullZones).where(eq(pullZones.instanceId, 'inst-1')).get() + expect(row?.customHostname).toBeNull() + }) + it('is a no-op when the pull zone already has a custom hostname', async () => { const db = createTestDb() seedInstance(db)