From 0165c3d9405908465147cb1cade9a8374b45089d Mon Sep 17 00:00:00 2001
From: Derek Cormier <derek@aspect.dev>
Date: Sun, 17 Mar 2024 18:19:03 -0700
Subject: [PATCH] feat: make git pushes more robust

---
 package.json                    |  1 +
 src/domain/create-entry.spec.ts | 54 +++++++++++++++++++++++++++++++++
 src/domain/create-entry.ts      |  9 +++++-
 yarn.lock                       |  5 +++
 4 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/package.json b/package.json
index 134530b..2551c59 100644
--- a/package.json
+++ b/package.json
@@ -25,6 +25,7 @@
     "axios": "^1.4.0",
     "axios-retry": "^4.0.0",
     "diff": "^5.1.0",
+    "exponential-backoff": "3.1.1",
     "extract-zip": "^2.0.1",
     "gcp-metadata": "^6.0.0",
     "nodemailer": "^6.7.8",
diff --git a/src/domain/create-entry.spec.ts b/src/domain/create-entry.spec.ts
index 629fdea..f933ac6 100644
--- a/src/domain/create-entry.spec.ts
+++ b/src/domain/create-entry.spec.ts
@@ -1,4 +1,5 @@
 import { createTwoFilesPatch } from "diff";
+import { BackoffOptions, backOff } from "exponential-backoff";
 import { Mocked, mocked } from "jest-mock";
 import { randomUUID } from "node:crypto";
 import fs, { PathLike } from "node:fs";
@@ -37,6 +38,9 @@ jest.mock("../infrastructure/github");
 jest.mock("./integrity-hash");
 jest.mock("./release-archive");
 jest.mock("node:fs");
+jest.mock("exponential-backoff");
+
+const realExponentialBackoff = jest.requireActual("exponential-backoff");
 
 const mockedFileReads: { [path: string]: string } = {};
 const EXTRACTED_MODULE_PATH = "/fake/path/to/MODULE.bazel";
@@ -1061,6 +1065,17 @@ describe("commitEntryToNewBranch", () => {
 });
 
 describe("pushEntryToFork", () => {
+  beforeEach(() => {
+    // Reduce the exponential-backoff delay to 0 for tests
+    (backOff as unknown as jest.SpyInstance).mockImplementation(
+      (request: () => Promise<void>, options?: BackoffOptions) =>
+        realExponentialBackoff.backOff(request, {
+          ...options,
+          startingDelay: 0,
+        })
+    );
+  });
+
   test("acquires an authenticated remote url for the bcr fork", async () => {
     const bcrRepo = CANONICAL_BCR;
     const bcrForkRepo = new Repository("bazel-central-registry", "aspect");
@@ -1136,6 +1151,45 @@ describe("pushEntryToFork", () => {
       branchName
     );
   });
+
+  test("retries 5 times if it fails", async () => {
+    const bcrRepo = CANONICAL_BCR;
+    const bcrForkRepo = new Repository("bazel-central-registry", "aspect");
+    const branchName = `repo/owner@v1.2.3`;
+
+    (backOff as unknown as jest.SpyInstance).mockImplementation(
+      (request: () => Promise<any>, options?: BackoffOptions) => {
+        return realExponentialBackoff.backOff(request, {
+          ...options,
+          startingDelay: 0,
+        });
+      }
+    );
+
+    mockGitClient.push
+      .mockRejectedValueOnce(new Error("failed push"))
+      .mockRejectedValueOnce(new Error("failed push"))
+      .mockRejectedValueOnce(new Error("failed push"))
+      .mockRejectedValueOnce(new Error("failed push"))
+      .mockResolvedValueOnce(undefined);
+
+    await createEntryService.pushEntryToFork(bcrForkRepo, bcrRepo, branchName);
+
+    expect(mockGitClient.push).toHaveBeenCalledTimes(5);
+  });
+
+  test("fails after the 5th retry", async () => {
+    const bcrRepo = CANONICAL_BCR;
+    const bcrForkRepo = new Repository("bazel-central-registry", "aspect");
+    const branchName = `repo/owner@v1.2.3`;
+
+    mockGitClient.push.mockRejectedValue(new Error("failed push"));
+
+    await expect(
+      createEntryService.pushEntryToFork(bcrForkRepo, bcrRepo, branchName)
+    ).rejects.toThrow();
+    expect(mockGitClient.push).toHaveBeenCalledTimes(5);
+  });
 });
 
 function mockRulesetFiles(
diff --git a/src/domain/create-entry.ts b/src/domain/create-entry.ts
index 1cd0e38..d02a1c7 100644
--- a/src/domain/create-entry.ts
+++ b/src/domain/create-entry.ts
@@ -1,4 +1,5 @@
 import { createTwoFilesPatch, parsePatch } from "diff";
+import { backOff } from "exponential-backoff";
 import { randomBytes } from "node:crypto";
 import fs, { readFileSync } from "node:fs";
 import path from "node:path";
@@ -170,7 +171,13 @@ export class CreateEntryService {
       await this.gitClient.push(bcr.diskPath, "origin", branch);
       return;
     }
-    await this.gitClient.push(bcr.diskPath, "authed-fork", branch);
+
+    await backOff(
+      () => this.gitClient.push(bcr.diskPath, "authed-fork", branch),
+      {
+        numOfAttempts: 5,
+      }
+    );
   }
 
   private addPatches(
diff --git a/yarn.lock b/yarn.lock
index aaef61a..c87d0d6 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -2408,6 +2408,11 @@ expect@^28.0.0, expect@^28.1.3:
     jest-message-util "^28.1.3"
     jest-util "^28.1.3"
 
+exponential-backoff@3.1.1:
+  version "3.1.1"
+  resolved "https://registry.yarnpkg.com/exponential-backoff/-/exponential-backoff-3.1.1.tgz#64ac7526fe341ab18a39016cd22c787d01e00bf6"
+  integrity sha512-dX7e/LHVJ6W3DE1MHWi9S1EYzDESENfLrYohG2G++ovZrYOkm4Knwa0mc1cn84xJOR4KEU0WSchhLbd0UklbHw==
+
 express@^4.14.0, express@^4.16.4:
   version "4.18.2"
   resolved "https://registry.yarnpkg.com/express/-/express-4.18.2.tgz#3fabe08296e930c796c19e3c516979386ba9fd59"