From f5f44c25115a5b1e14fc4c55a8eeeed727974968 Mon Sep 17 00:00:00 2001 From: tdurieux Date: Sun, 6 Sep 2026 09:16:17 +0200 Subject: [PATCH] fix: preserve removal intent when Redis enqueue fails --- src/queue/index.ts | 2 +- src/server/routes/repository-private.ts | 11 ++----- test/backend-reliability.test.js | 38 +++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/queue/index.ts b/src/queue/index.ts index 13c1f02..474c3ee 100644 --- a/src/queue/index.ts +++ b/src/queue/index.ts @@ -203,7 +203,7 @@ export async function recoverStuckRemoving() { ...serializeError(e), repoId: doc.repoId, }); - await markErrorIfRemoving(doc.repoId, "removal_interrupted"); + // Keep REMOVING so another recovery pass can retry the enqueue. } } } catch (e) { diff --git a/src/server/routes/repository-private.ts b/src/server/routes/repository-private.ts index eceb26c..d7d598e 100644 --- a/src/server/routes/repository-private.ts +++ b/src/server/routes/repository-private.ts @@ -241,14 +241,9 @@ router.delete( const user = await getUser(req); isOwnerOrAdmin([repo.owner.id], user); await repo.updateStatus(RepositoryStatus.REMOVING); - try { - await addRemovalJob(repo.repoId); - } catch (error) { - const message = - error instanceof Error ? error.message : "removal_enqueue_failed"; - await repo.updateStatus(RepositoryStatus.ERROR, message); - throw error; - } + // Keep removal intent durable if Redis is unavailable. Recovery can + // enqueue it later, and public requests must remain blocked meanwhile. + await addRemovalJob(repo.repoId); return res.json({ status: repo.status }); } catch (error) { handleError(error, res, req); diff --git a/test/backend-reliability.test.js b/test/backend-reliability.test.js index 33c0997..2538f08 100644 --- a/test/backend-reliability.test.js +++ b/test/backend-reliability.test.js @@ -110,6 +110,44 @@ describe("removal recovery", function () { expect(added[1]).to.deep.equal({ repoId: repo.repoId }); }); } + + it("preserves deletion across a failed request enqueue and recovery enqueue", async function () { + const repo = new Repository(new AnonymizedRepositoryModel({ + repoId: "repo-1", status: "ready", + owner: "507f1f77bcf86cd799439011", + options: { expirationMode: "never" }, source: {}, + })); + routeUtils.getRepo = async () => repo; + routeUtils.getUser = async () => ({ model: { id: "admin" }, isAdmin: true }); + let responseError; + routeUtils.handleError = (error) => { responseError = error; }; + const failure = new Error("Redis unavailable"); + queueModule.removeQueue = { + getJobs: async () => [], + getJob: async () => undefined, + add: async () => { throw failure; }, + }; + const router = require("../src/server/routes/repository-private").default; + const handler = router.stack.find((layer) => + layer.route?.path === "/:repoId/" && layer.route.methods.delete + ).route.stack[0].handle; + await handler({ params: { repoId: repo.repoId } }, {}); + expect(responseError).to.equal(failure); + expect(repo.status).to.equal("removing"); + let publicError; + try { await repo.check(); } catch (error) { publicError = error; } + expect(publicError.message).to.equal("repository_expired"); + + AnonymizedRepositoryModel.find = (filter) => ({ + lean: async () => repo.status === filter.status ? [repo.model] : [], + }); + await queueModule.recoverStuckRemoving(); + expect(repo.status).to.equal("removing"); + let added; + queueModule.removeQueue.add = async (...args) => { added = args; }; + await queueModule.recoverStuckRemoving(); + expect(added[1]).to.deep.equal({ repoId: repo.repoId }); + }); }); describe("conference edits", function () {