From 2311f346d4e5ae9769feb5eca5c2a69b4213c4df Mon Sep 17 00:00:00 2001 From: Guillaume Juge Date: Sat, 5 Sep 2026 11:19:26 +0200 Subject: [PATCH] fix(dns): do not report a restored OVH record as lost restoreRecord had the restore POST and its zone refresh inside one catch. That was harmless while refreshZone swallowed failures, but the previous commit made it throw, which brought a new case into that catch: the restore succeeds and only the publication fails. The message then told the user the record "has been deleted" and to recreate it by hand. It exists at OVH, just unpublished, so following that advice duplicates it as soon as the zone is refreshed. The two failures are now reported separately. A failed POST still means the record is really gone and prints what to recreate. A failed refresh after a successful restore says the record is back but not served yet, and explicitly says not to recreate it. Either way the original replacement error is kept, so the user still learns why the type change failed. Reported by Greptile on #5258. Co-Authored-By: Claude Opus 5 (1M context) --- apps/dokploy/__test__/dns/ovh.test.ts | 28 +++++++++++++++++++++++++++ packages/server/src/utils/dns/ovh.ts | 19 ++++++++++++++---- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/apps/dokploy/__test__/dns/ovh.test.ts b/apps/dokploy/__test__/dns/ovh.test.ts index a58665015..744716cbd 100644 --- a/apps/dokploy/__test__/dns/ovh.test.ts +++ b/apps/dokploy/__test__/dns/ovh.test.ts @@ -467,6 +467,34 @@ describe("ovhClient.updateRecord", () => { }), ).rejects.toThrow(/was applied, but refreshing zone "example\.com" failed/); }); + + it("does not tell the user to recreate a record that was restored but not published", async () => { + const cfg = freshConfig(); + mockApi( + ovhSuccess({ + id: 4, + zone: "example.com", + fieldType: "A", + subDomain: "app", + target: "1.1.1.1", + ttl: 60, + }), + ovhSuccess(null), // DELETE de l'ancien + ovhError("Invalid target", 400), // POST de remplacement -> échec + ovhSuccess({ id: 12 }), // POST de restauration -> succès + ovhError("Service unavailable", 503), // refresh -> échec + ); + + const attempt = ovhClient.updateRecord(cfg, "example.com", "4", { + type: "CNAME", + name: "app.example.com", + content: "not a valid target", + }); + + // L'enregistrement existe de nouveau chez OVH : le recréer le dupliquerait. + await expect(attempt).rejects.toThrow(/was restored, but refreshing zone/); + await expect(attempt).rejects.not.toThrow(/Recreate it manually/); + }); }); describe("ovhClient.deleteRecord", () => { diff --git a/packages/server/src/utils/dns/ovh.ts b/packages/server/src/utils/dns/ovh.ts index 73825206d..1e0d0a075 100644 --- a/packages/server/src/utils/dns/ovh.ts +++ b/packages/server/src/utils/dns/ovh.ts @@ -184,12 +184,19 @@ const refreshZone = async (config: OvhConfig, zone: string) => { }; // Used to undo the delete half of a type change when the replacement fails. +// The restore and its publication are reported separately: a failed POST means +// the record is really gone, whereas a failed refresh means it is back but not +// served yet. Collapsing the two would tell the user to recreate a record that +// already exists, which duplicates it as soon as the zone is refreshed. const restoreRecord = async ( config: OvhConfig, zone: string, record: OvhRecord, cause: unknown, ) => { + const causeMessage = cause instanceof Error ? cause.message : String(cause); + const name = toFqdn(record.subDomain, zone); + try { await ovhFetch(config, `/domain/zone/${encodeURIComponent(zone)}/record`, { method: "POST", @@ -200,13 +207,17 @@ const restoreRecord = async ( ...(record.ttl === null ? {} : { ttl: record.ttl }), }, }); + } catch { + throw new Error( + `OVH: could not replace the record and could not restore the original one, which has been deleted. Recreate it manually: ${record.fieldType} ${name} -> ${record.target}. Original failure: ${causeMessage}`, + ); + } + + try { await refreshZone(config, zone); } catch { - const name = toFqdn(record.subDomain, zone); throw new Error( - `OVH: could not replace the record and could not restore the original one, which has been deleted. Recreate it manually: ${record.fieldType} ${name} -> ${record.target}. Original failure: ${ - cause instanceof Error ? cause.message : String(cause) - }`, + `OVH: the replacement failed and the original record was restored, but refreshing zone "${zone}" failed, so the restore is not served yet. Do not recreate it — the next successful change to this zone will publish it. Original failure: ${causeMessage}`, ); } };