Repository: firebase/firebase-tools (ts)

Issue:
Frequent failures when deploying functions (Unhandled error cleaning up build images)
<!-- DO NOT DELETE
validate_template=true
template_path=.github/ISSUE_TEMPLATE/bug_report.md
-->

###  Environment info
**firebase-tools:** 9.16.0

**Platform:**macOS

### [REQUIRED] Test case

Try deploying functions
```
firebase deploy --only functions
```
functions fail with "Unhandled error cleaning up build images."
```
 functions: cleaning up build files...
⚠  functions: Unhandled error cleaning up build images. This could result in a small monthly bill if not corrected. You can attempt to delete these images by redeploying or you can delete them manually at https://console.cloud.google.com/gcr/images/api-dev/us/gcf
```

### [REQUIRED] Expected behavior

Should not fail deploying this frequently.

### [REQUIRED] Actual behavior

For deploying all my functions(around 50) per environment, I need to redeploy nearly 6 times to finish it. It keeps failing frequently from past few days. 

 I'm currently having 4 environments and attempting nearly **30 times** to finish deploying!






Pull request description:
Add retries for Docker APIs
Blind attempt to fix #3639 before I get debug logs.

Normally I'd worry that we hit a rate limit, but according to [docs](https://cloud.google.com/container-registry/quotas) the rate limit is 50K requests per 10m. Even with an API this chatty it's unlikely to hit the limit...

Without knowing any more info, we can realize that an API as chatty as Docker is likely to fail just through statistics. Deleting 15 functions would probably require 60-75 API calls. If gcr.io is a 3-9s service, that's a 6% chance of failure.


Patch:
diff --git a/src/deploy/functions/containerCleaner.ts b/src/deploy/functions/containerCleaner.ts
index ece756e5a..dc5d38e46 100644
--- a/src/deploy/functions/containerCleaner.ts
+++ b/src/deploy/functions/containerCleaner.ts
@@ -40,6 +40,25 @@ export const SUBDOMAIN_MAPPING: Record<string, string> = {
   "australia-southeast1": "asia",
 };
 
+async function retry<Return>(func: () => Promise<Return>): Promise<Return> {
+  const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
+  const MAX_RETRIES = 3;
+  const INITIAL_BACKOFF = 100;
+  let retry = 0;
+  while (true) {
+    try {
+      return await func();
+    } catch (error) {
+      logger.debug("Failed docker command with error", error);
+      retry += 1;
+      if (retry >= MAX_RETRIES) {
+        throw new FirebaseError("Failed to clean up artifacts", { original: error });
+      }
+      await sleep(Math.pow(INITIAL_BACKOFF, retry - 1));
+    }
+  }
+}
+
 export async function cleanupBuildImages(functions: backend.FunctionSpec[]): Promise<void> {
   utils.logBullet(clc.bold.cyan("functions: ") + "cleaning up build files...");
   const gcrCleaner = new ContainerRegistryCleaner();
@@ -258,7 +277,7 @@ export class DockerHelper {
 
   async ls(path: string): Promise<Stat> {
     if (!this.cache[path]) {
-      const raw = await this.client.listTags(path);
+      const raw = await retry(() => this.client.listTags(path));
       this.cache[path] = {
         tags: raw.tags,
         digests: Object.keys(raw.manifest),
@@ -291,7 +310,7 @@ export class DockerHelper {
     const deleteTags = stat.tags.map((tag) =>
       (async () => {
         try {
-          await this.client.deleteTag(path, tag);
+          await retry(() => this.client.deleteTag(path, tag));
           stat.tags.splice(stat.tags.indexOf(tag), 1);
         } catch (err) {
           logger.debug("Got error trying to remove docker tag:", err);
@@ -304,7 +323,7 @@ export class DockerHelper {
     const deleteImages = stat.digests.map((digest) =>
       (async () => {
         try {
-          await this.client.deleteImage(path, digest);
+          await retry(() => this.client.deleteImage(path, digest));
           stat.digests.splice(stat.digests.indexOf(digest), 1);
         } catch (err) {
           logger.debug("Got error trying to remove docker image:", err);
diff --git a/templates/init/functions/typescript/_eslintrc b/templates/init/functions/typescript/_eslintrc
index 3cc63325d..a7f90a9dd 100644
--- a/templates/init/functions/typescript/_eslintrc
+++ b/templates/init/functions/typescript/_eslintrc
@@ -24,6 +24,7 @@ module.exports = {
     "@typescript-eslint",
   ],
   rules: {
-    quotes: ["error", "double"],
+    "quotes": ["error", "double"],
+    "import/no-unresolved": 0,
   },
 };


Test patch:
diff --git a/src/test/deploy/functions/containerCleaner.spec.ts b/src/test/deploy/functions/containerCleaner.spec.ts
index 2028fbc31..7f843eafa 100644
--- a/src/test/deploy/functions/containerCleaner.spec.ts
+++ b/src/test/deploy/functions/containerCleaner.spec.ts
@@ -12,14 +12,14 @@ describe("DockerHelper", () => {
   let deleteImage: sinon.SinonStub;
   let helper: containerCleaner.DockerHelper;
 
-  before(() => {
+  beforeEach(() => {
     helper = new containerCleaner.DockerHelper("us");
     listTags = sinon.stub(helper.client, "listTags").rejects("Unexpected call");
     deleteTag = sinon.stub(helper.client, "deleteTag").rejects("Unexpected call");
     deleteImage = sinon.stub(helper.client, "deleteImage").rejects("Unexpected call");
   });
 
-  after(() => {
+  afterEach(() => {
     sinon.verifyAndRestore();
   });
 
@@ -43,22 +43,27 @@ describe("DockerHelper", () => {
   };
 
   it("Fetches tags with caching", async () => {
-    listTags.withArgs("foo/bar").resolves(FOO_BAR);
+    listTags
+      .withArgs("foo/bar")
+      .onFirstCall()
+      .rejects("I'm flaky!")
+      .onSecondCall()
+      .resolves(FOO_BAR);
 
     await expect(helper.ls("foo/bar")).to.eventually.deep.equal({
       digests: ["sha256:hash1", "sha256:hash2"],
       tags: ["tag1", "tag2"],
       children: ["baz"],
     });
+    expect(listTags).to.have.been.calledTwice;
+    expect(listTags).to.not.have.been.calledWith("foo");
 
     await expect(helper.ls("foo/bar")).to.eventually.deep.equal({
       digests: ["sha256:hash1", "sha256:hash2"],
       tags: ["tag1", "tag2"],
       children: ["baz"],
     });
-
-    // This also verifies that we haven't called at "/foo" to ls "/foo/bar"
-    expect(listTags).to.have.been.calledOnce;
+    expect(listTags).to.have.been.calledTwice;
   });
 
   it("Deletes recursively", async () => {
@@ -69,13 +74,24 @@ describe("DockerHelper", () => {
       "foo/bar": ["tag1", "tag2"],
       "foo/bar/baz": ["tag3"],
     };
+    let firstDeleteTag = true;
     deleteTag.callsFake((path: string, tag: string) => {
+      if (firstDeleteTag) {
+        firstDeleteTag = false;
+        throw new Error("I'm flaky");
+      }
       if (!remainingTags[path].includes(tag)) {
         throw new Error("Cannot remove tag twice");
       }
       remainingTags[path].splice(remainingTags[path].indexOf(tag), 1);
     });
+
+    let firstDeleteImage = true;
     deleteImage.callsFake((path: string, digest: string) => {
+      if (firstDeleteImage) {
+        firstDeleteImage = false;
+        throw new Error("I'm flaky");
+      }
       if (remainingTags[path].length) {
         throw new Error("Cannot remove image while tags still pin it");
       }
@@ -87,12 +103,12 @@ describe("DockerHelper", () => {
     expect(listTags).to.have.been.calledWith("foo/bar");
     expect(listTags).to.have.been.calledWith("foo/bar/baz");
 
-    expect(deleteTag).to.have.been.calledThrice;
+    expect(deleteTag).to.have.been.callCount(4);
     expect(deleteTag).to.have.been.calledWith("foo/bar/baz", "tag3");
     expect(deleteTag).to.have.been.calledWith("foo/bar", "tag1");
     expect(deleteTag).to.have.been.calledWith("foo/bar", "tag2");
 
-    expect(deleteImage).to.have.been.calledThrice;
+    expect(deleteImage).to.have.been.callCount(4);
     expect(deleteImage).to.have.been.calledWith("foo/bar/baz", "sha256:hash3");
     expect(deleteImage).to.have.been.calledWith("foo/bar", "sha256:hash1");
     expect(deleteImage).to.have.been.calledWith("foo/bar", "sha256:hash2");