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) <noreply@anthropic.com>
This commit is contained in:
Guillaume Juge 2026-09-05 11:19:26 +02:00
parent 28f71e727a
commit 2311f346d4
2 changed files with 43 additions and 4 deletions

View File

@ -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", () => {

View File

@ -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}`,
);
}
};