From 28f71e727a9b0e279e9e889aa8fbf710ac01056f Mon Sep 17 00:00:00 2001 From: Guillaume Juge Date: Fri, 4 Sep 2026 20:23:43 +0200 Subject: [PATCH] fix(dns): report an OVH zone refresh failure for what it is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refreshZone runs after the record write has already succeeded, so a failure there means the change exists at the provider but is not being served yet. It was left unguarded at all six call sites, so the caller saw a bare transport error and would reasonably read it as "nothing was applied" — while a type change had already deleted the old record and created its replacement. Rolling the write back would destroy correct state over a publish failure, and re-deleting after an uncertain network error is how records get lost. The error now says what actually happened and how it resolves: the next successful change to the zone publishes it, or the user can refresh the zone from the OVH manager. Guarding the helper covers upsertRecord and deleteRecord too, not just the type change Greptile pointed at. Reported by Greptile on #5258. Co-Authored-By: Claude Opus 5 (1M context) --- apps/dokploy/__test__/dns/ovh.test.ts | 27 +++++++++++++++++++++++++++ packages/server/src/utils/dns/ovh.ts | 20 ++++++++++++++++---- 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 4f70d4c6c..a58665015 100644 --- a/apps/dokploy/__test__/dns/ovh.test.ts +++ b/apps/dokploy/__test__/dns/ovh.test.ts @@ -440,6 +440,33 @@ describe("ovhClient.updateRecord", () => { /Recreate it manually: A app\.example\.com -> 1\.1\.1\.1/, ); }); + + it("says the change was applied when only the zone refresh fails", async () => { + const cfg = freshConfig(); + mockApi( + ovhSuccess({ + id: 4, + zone: "example.com", + fieldType: "A", + subDomain: "app", + target: "1.1.1.1", + ttl: 60, + }), + ovhSuccess(null), + ovhSuccess({ id: 11 }), + ovhError("Service unavailable", 503), + ); + + // The replacement succeeded, so the record exists at the provider — only + // publishing failed. Rolling back would destroy correct state. + await expect( + ovhClient.updateRecord(cfg, "example.com", "4", { + type: "CNAME", + name: "app.example.com", + content: "example.com", + }), + ).rejects.toThrow(/was applied, but refreshing zone "example\.com" failed/); + }); }); describe("ovhClient.deleteRecord", () => { diff --git a/packages/server/src/utils/dns/ovh.ts b/packages/server/src/utils/dns/ovh.ts index b24666193..73825206d 100644 --- a/packages/server/src/utils/dns/ovh.ts +++ b/packages/server/src/utils/dns/ovh.ts @@ -164,11 +164,23 @@ const mapWithConcurrency = async ( return results; }; -// OVH only applies zone changes once the zone is explicitly refreshed. +// OVH only applies zone changes once the zone is explicitly refreshed. This runs +// after the record write has already succeeded, so a failure here means the +// change exists at the provider but is not being served yet. Rolling the write +// back would destroy correct state over a publish failure, so say what actually +// happened instead of letting the caller read it as "nothing was applied". const refreshZone = async (config: OvhConfig, zone: string) => { - await ovhFetch(config, `/domain/zone/${encodeURIComponent(zone)}/refresh`, { - method: "POST", - }); + try { + await ovhFetch(config, `/domain/zone/${encodeURIComponent(zone)}/refresh`, { + method: "POST", + }); + } catch (error) { + throw new Error( + `OVH: the record change was applied, but refreshing zone "${zone}" failed, so it is not served yet. The next successful change to this zone will publish it, or you can refresh the zone from the OVH manager. Cause: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } }; // Used to undo the delete half of a type change when the replacement fails.