fix: recover concurrent asset metadata 404s (#760)

Signed-off-by: Rui Chen <rui@chenrui.dev>

__tests__/github.test.ts

@@ -614,6 +614,78 @@ describe('github', () => {
       expect(uploadReleaseAsset).toHaveBeenCalledTimes(2);
     });
 
+    it('retries upload after deleting a conflicting renamed asset matched by label', async () => {
+      const tempDir = mkdtempSync(join(tmpdir(), 'gh-release-race-dotfile-'));
+      const dotfilePath = join(tempDir, '.config');
+      writeFileSync(dotfilePath, 'config');
+
+      const uploadReleaseAsset = vi
+        .fn()
+        .mockRejectedValueOnce({
+          status: 422,
+          response: { data: { errors: [{ code: 'already_exists' }] } },
+        })
+        .mockResolvedValueOnce({
+          status: 201,
+          data: { id: 123, name: 'default.config', label: '.config' },
+        });
+
+      const listReleaseAssets = vi
+        .fn()
+        .mockResolvedValue([{ id: 99, name: 'default.config', label: '.config' }]);
+      const deleteReleaseAsset = vi.fn().mockResolvedValue(undefined);
+      const updateReleaseAsset = vi.fn().mockResolvedValue({
+        data: { id: 123, name: 'default.config', label: '.config' },
+      });
+
+      const mockReleaser: Releaser = {
+        getReleaseByTag: () => Promise.reject('Not implemented'),
+        createRelease: () => Promise.reject('Not implemented'),
+        updateRelease: () => Promise.reject('Not implemented'),
+        finalizeRelease: () => Promise.reject('Not implemented'),
+        allReleases: async function* () {
+          throw new Error('Not implemented');
+        },
+        listReleaseAssets,
+        deleteReleaseAsset,
+        deleteRelease: () => Promise.reject('Not implemented'),
+        updateReleaseAsset,
+        uploadReleaseAsset,
+      };
+
+      try {
+        const result = await upload(
+          config,
+          mockReleaser,
+          'https://uploads.github.com/repos/owner/repo/releases/1/assets',
+          dotfilePath,
+          [],
+        );
+
+        expect(result).toStrictEqual({ id: 123, name: 'default.config', label: '.config' });
+        expect(listReleaseAssets).toHaveBeenCalledWith({
+          owner: 'owner',
+          repo: 'repo',
+          release_id: 1,
+        });
+        expect(deleteReleaseAsset).toHaveBeenCalledWith({
+          owner: 'owner',
+          repo: 'repo',
+          asset_id: 99,
+        });
+        expect(updateReleaseAsset).toHaveBeenCalledWith({
+          owner: 'owner',
+          repo: 'repo',
+          asset_id: 123,
+          name: 'default.config',
+          label: '.config',
+        });
+        expect(uploadReleaseAsset).toHaveBeenCalledTimes(2);
+      } finally {
+        rmSync(tempDir, { recursive: true, force: true });
+      }
+    });
+
     it('handles 422 already_exists error gracefully', async () => {
       const existingRelease = {
         id: 1,
@@ -963,6 +1035,263 @@ describe('github', () => {
       }
     });
 
+    it('refreshes release assets when the uploaded renamed asset is not immediately patchable', async () => {
+      const tempDir = mkdtempSync(join(tmpdir(), 'gh-release-dotfile-'));
+      const dotfilePath = join(tempDir, '.config');
+      writeFileSync(dotfilePath, 'config');
+
+      const updateReleaseAssetSpy = vi
+        .fn()
+        .mockRejectedValueOnce({ status: 404 })
+        .mockResolvedValueOnce({
+          data: {
+            id: 2,
+            name: 'default.config',
+            label: '.config',
+          },
+        });
+      const listReleaseAssetsSpy = vi.fn().mockResolvedValue([
+        {
+          id: 2,
+          name: 'default.config',
+          label: '',
+        },
+      ]);
+      const releaser: Releaser = {
+        getReleaseByTag: () => Promise.reject('Not implemented'),
+        createRelease: () => Promise.reject('Not implemented'),
+        updateRelease: () => Promise.reject('Not implemented'),
+        finalizeRelease: () => Promise.reject('Not implemented'),
+        allReleases: async function* () {
+          throw new Error('Not implemented');
+        },
+        listReleaseAssets: listReleaseAssetsSpy,
+        deleteReleaseAsset: () => Promise.reject('Not implemented'),
+        deleteRelease: () => Promise.reject('Not implemented'),
+        updateReleaseAsset: updateReleaseAssetSpy,
+        uploadReleaseAsset: () =>
+          Promise.resolve({
+            status: 201,
+            data: {
+              id: 1,
+              name: 'default.config',
+              label: '',
+            },
+          }),
+      };
+
+      try {
+        const result = await upload(
+          config,
+          releaser,
+          'https://uploads.github.com/repos/owner/repo/releases/1/assets',
+          dotfilePath,
+          [],
+        );
+
+        expect(updateReleaseAssetSpy).toHaveBeenNthCalledWith(1, {
+          owner: 'owner',
+          repo: 'repo',
+          asset_id: 1,
+          name: 'default.config',
+          label: '.config',
+        });
+        expect(listReleaseAssetsSpy).toHaveBeenCalledWith({
+          owner: 'owner',
+          repo: 'repo',
+          release_id: 1,
+        });
+        expect(updateReleaseAssetSpy).toHaveBeenNthCalledWith(2, {
+          owner: 'owner',
+          repo: 'repo',
+          asset_id: 2,
+          name: 'default.config',
+          label: '.config',
+        });
+        expect(result).toEqual({
+          id: 2,
+          name: 'default.config',
+          label: '.config',
+        });
+      } finally {
+        rmSync(tempDir, { recursive: true, force: true });
+      }
+    });
+
+    it('treats update-a-release-asset 404 as success when a matching asset is present after refresh', async () => {
+      const tempDir = mkdtempSync(join(tmpdir(), 'gh-release-dotfile-'));
+      const dotfilePath = join(tempDir, '.config');
+      writeFileSync(dotfilePath, 'config');
+
+      const listReleaseAssetsSpy = vi.fn().mockResolvedValue([
+        {
+          id: 2,
+          name: 'default.config',
+          label: '.config',
+        },
+      ]);
+      const releaser: Releaser = {
+        getReleaseByTag: () => Promise.reject('Not implemented'),
+        createRelease: () => Promise.reject('Not implemented'),
+        updateRelease: () => Promise.reject('Not implemented'),
+        finalizeRelease: () => Promise.reject('Not implemented'),
+        allReleases: async function* () {
+          throw new Error('Not implemented');
+        },
+        listReleaseAssets: listReleaseAssetsSpy,
+        deleteReleaseAsset: () => Promise.reject('Not implemented'),
+        deleteRelease: () => Promise.reject('Not implemented'),
+        updateReleaseAsset: () => Promise.reject('Not implemented'),
+        uploadReleaseAsset: () =>
+          Promise.reject({
+            status: 404,
+            message:
+              'Not Found - https://docs.github.com/rest/releases/assets#update-a-release-asset',
+          }),
+      };
+
+      try {
+        const result = await upload(
+          config,
+          releaser,
+          'https://uploads.github.com/repos/owner/repo/releases/1/assets',
+          dotfilePath,
+          [],
+        );
+
+        expect(listReleaseAssetsSpy).toHaveBeenCalledWith({
+          owner: 'owner',
+          repo: 'repo',
+          release_id: 1,
+        });
+        expect(result).toEqual({
+          id: 2,
+          name: 'default.config',
+          label: '.config',
+        });
+      } finally {
+        rmSync(tempDir, { recursive: true, force: true });
+      }
+    });
+
+    it('treats upload-endpoint 404s as release asset metadata failures when the docs link matches', async () => {
+      const tempDir = mkdtempSync(join(tmpdir(), 'gh-release-dotfile-'));
+      const dotfilePath = join(tempDir, '.config');
+      writeFileSync(dotfilePath, 'config');
+
+      const listReleaseAssetsSpy = vi.fn().mockResolvedValue([
+        {
+          id: 2,
+          name: 'default.config',
+          label: '.config',
+        },
+      ]);
+      const releaser: Releaser = {
+        getReleaseByTag: () => Promise.reject('Not implemented'),
+        createRelease: () => Promise.reject('Not implemented'),
+        updateRelease: () => Promise.reject('Not implemented'),
+        finalizeRelease: () => Promise.reject('Not implemented'),
+        allReleases: async function* () {
+          throw new Error('Not implemented');
+        },
+        listReleaseAssets: listReleaseAssetsSpy,
+        deleteReleaseAsset: () => Promise.reject('Not implemented'),
+        deleteRelease: () => Promise.reject('Not implemented'),
+        updateReleaseAsset: () => Promise.reject('Not implemented'),
+        uploadReleaseAsset: () =>
+          Promise.reject({
+            status: 404,
+            message:
+              'Not Found - https://docs.github.com/rest/releases/assets#update-a-release-asset',
+            request: {
+              url: 'https://uploads.github.com/repos/owner/repo/releases/1/assets?name=.config',
+            },
+          }),
+      };
+
+      try {
+        const result = await upload(
+          config,
+          releaser,
+          'https://uploads.github.com/repos/owner/repo/releases/1/assets',
+          dotfilePath,
+          [],
+        );
+
+        expect(listReleaseAssetsSpy).toHaveBeenCalledWith({
+          owner: 'owner',
+          repo: 'repo',
+          release_id: 1,
+        });
+        expect(result).toEqual({
+          id: 2,
+          name: 'default.config',
+          label: '.config',
+        });
+      } finally {
+        rmSync(tempDir, { recursive: true, force: true });
+      }
+    });
+
+    it('polls for a matching asset after update-a-release-asset 404 before failing', async () => {
+      const tempDir = mkdtempSync(join(tmpdir(), 'gh-release-dotfile-'));
+      const dotfilePath = join(tempDir, '.config');
+      writeFileSync(dotfilePath, 'config');
+
+      const listReleaseAssetsSpy = vi
+        .fn()
+        .mockResolvedValueOnce([])
+        .mockResolvedValueOnce([
+          {
+            id: 2,
+            name: 'default.config',
+            label: '.config',
+          },
+        ]);
+      const releaser: Releaser = {
+        getReleaseByTag: () => Promise.reject('Not implemented'),
+        createRelease: () => Promise.reject('Not implemented'),
+        updateRelease: () => Promise.reject('Not implemented'),
+        finalizeRelease: () => Promise.reject('Not implemented'),
+        allReleases: async function* () {
+          throw new Error('Not implemented');
+        },
+        listReleaseAssets: listReleaseAssetsSpy,
+        deleteReleaseAsset: () => Promise.reject('Not implemented'),
+        deleteRelease: () => Promise.reject('Not implemented'),
+        updateReleaseAsset: () => Promise.reject('Not implemented'),
+        uploadReleaseAsset: () =>
+          Promise.reject({
+            status: 404,
+            message:
+              'Not Found - https://docs.github.com/rest/releases/assets#update-a-release-asset',
+          }),
+      };
+
+      try {
+        const resultPromise = upload(
+          config,
+          releaser,
+          'https://uploads.github.com/repos/owner/repo/releases/1/assets',
+          dotfilePath,
+          [],
+        );
+
+        await new Promise((resolve) => setTimeout(resolve, 1100));
+
+        const result = await resultPromise;
+
+        expect(listReleaseAssetsSpy).toHaveBeenCalledTimes(2);
+        expect(result).toEqual({
+          id: 2,
+          name: 'default.config',
+          label: '.config',
+        });
+      } finally {
+        rmSync(tempDir, { recursive: true, force: true });
+      }
+    });
+
     it('matches an existing asset by label when overwriting a dotfile', async () => {
       const tempDir = mkdtempSync(join(tmpdir(), 'gh-release-dotfile-'));
       const dotfilePath = join(tempDir, '.config');

src/github.ts

@@ -263,6 +263,26 @@ export const mimeOrDefault = (path: string): string => {
   return lookup(path) || 'application/octet-stream';
 };
 
+const releaseAssetMatchesName = (
+  name: string,
+  asset: { name: string; label?: string | null },
+): boolean => asset.name === name || asset.name === alignAssetName(name) || asset.label === name;
+
+const isReleaseAssetUpdateNotFound = (error: any): boolean => {
+  const errorStatus = error?.status ?? error?.response?.status;
+  const requestUrl = error?.request?.url;
+  const errorMessage = error?.message;
+  const isReleaseAssetRequest =
+    typeof requestUrl === 'string' &&
+    (/\/releases\/assets\//.test(requestUrl) || /\/releases\/\d+\/assets(?:\?|$)/.test(requestUrl));
+
+  return (
+    errorStatus === 404 &&
+    (isReleaseAssetRequest ||
+      (typeof errorMessage === 'string' && errorMessage.includes('update-a-release-asset')))
+  );
+};
+
 export const upload = async (
   config: Config,
   releaser: Releaser,
@@ -275,11 +295,9 @@ export const upload = async (
   const releaseIdMatch = url.match(/\/releases\/(\d+)\/assets/);
   const releaseId = releaseIdMatch ? Number(releaseIdMatch[1]) : undefined;
   const currentAsset = currentAssets.find(
-    // note: GitHub renames asset filenames that have special characters, non-alphanumeric characters, and leading or trailing periods. The "List release assets" endpoint lists the renamed filenames.
+    // GitHub can rewrite uploaded asset names, so compare against both the raw name
-    // due to this renaming we need to be mindful when we compare the file name we're uploading with a name github may already have rewritten for logical comparison
+    // GitHub returns and the restored label we set when available.
-    // see https://docs.github.com/en/rest/releases/assets?apiVersion=2022-11-28#upload-a-release-asset
+    (currentAsset) => releaseAssetMatchesName(name, currentAsset),
-    ({ name: currentName, label: currentLabel }) =>
-      currentName === name || currentName === alignAssetName(name) || currentLabel === name,
   );
   if (currentAsset) {
     if (config.input_overwrite_files === false) {
@@ -297,6 +315,32 @@ export const upload = async (
   console.log(`⬆️ Uploading ${name}...`);
   const endpoint = new URL(url);
   endpoint.searchParams.append('name', name);
+  const findReleaseAsset = async (
+    matches: (asset: { id: number; name: string; label?: string | null }) => boolean,
+    attempts: number = 3,
+  ) => {
+    if (releaseId === undefined) {
+      return undefined;
+    }
+
+    for (let attempt = 1; attempt <= attempts; attempt++) {
+      const latestAssets = await releaser.listReleaseAssets({
+        owner,
+        repo,
+        release_id: releaseId,
+      });
+      const latestAsset = latestAssets.find(matches);
+      if (latestAsset) {
+        return latestAsset;
+      }
+
+      if (attempt < attempts) {
+        await new Promise((resolve) => setTimeout(resolve, 1000));
+      }
+    }
+
+    return undefined;
+  };
   const uploadAsset = async () => {
     const fh = await open(path);
     try {
@@ -312,8 +356,54 @@ export const upload = async (
     }
   };
 
-  try {
+  const maybeRestoreAssetLabel = async (uploadedAsset: {
-    const resp = await uploadAsset();
+    id?: number;
+    name?: string;
+    label?: string | null;
+    [key: string]: any;
+  }) => {
+    if (!uploadedAsset.name || uploadedAsset.name === name || !uploadedAsset.id) {
+      return uploadedAsset;
+    }
+
+    console.log(`✏️ Restoring asset label to ${name}...`);
+
+    const updateAssetLabel = async (assetId: number) => {
+      const { data } = await releaser.updateReleaseAsset({
+        owner,
+        repo,
+        asset_id: assetId,
+        name: uploadedAsset.name!,
+        label: name,
+      });
+      return data;
+    };
+
+    try {
+      return await updateAssetLabel(uploadedAsset.id);
+    } catch (error: any) {
+      const errorStatus = error?.status ?? error?.response?.status;
+
+      if (errorStatus === 404 && releaseId !== undefined) {
+        try {
+          const latestAsset = await findReleaseAsset(
+            (currentAsset) =>
+              currentAsset.id === uploadedAsset.id || currentAsset.name === uploadedAsset.name,
+          );
+          if (latestAsset) {
+            return await updateAssetLabel(latestAsset.id);
+          }
+        } catch (refreshError) {
+          console.warn(`error refreshing release assets for ${name}: ${refreshError}`);
+        }
+      }
+
+      console.warn(`error updating release asset label for ${name}: ${error}`);
+      return uploadedAsset;
+    }
+  };
+
+  const handleUploadedAsset = async (resp: { status: number; data: any }) => {
     const json = resp.data;
     if (resp.status !== 201) {
       throw new Error(
@@ -322,28 +412,35 @@ export const upload = async (
         }\n${json.message}\n${JSON.stringify(json.errors)}`,
       );
     }
-    if (json.name && json.name !== name && json.id) {
+    const assetWithLabel = await maybeRestoreAssetLabel(json);
-      console.log(`✏️ Restoring asset label to ${name}...`);
-      try {
-        const { data } = await releaser.updateReleaseAsset({
-          owner,
-          repo,
-          asset_id: json.id,
-          name: json.name,
-          label: name,
-        });
-        console.log(`✅ Uploaded ${name}`);
-        return data;
-      } catch (error) {
-        console.warn(`error updating release asset label for ${name}: ${error}`);
-      }
-    }
     console.log(`✅ Uploaded ${name}`);
-    return json;
+    return assetWithLabel;
+  };
+
+  try {
+    return await handleUploadedAsset(await uploadAsset());
   } catch (error: any) {
     const errorStatus = error?.status ?? error?.response?.status;
     const errorData = error?.response?.data;
 
+    if (releaseId !== undefined && isReleaseAssetUpdateNotFound(error)) {
+      try {
+        const latestAsset = await findReleaseAsset((currentAsset) =>
+          releaseAssetMatchesName(name, currentAsset),
+        );
+        if (latestAsset) {
+          console.warn(
+            `error updating release asset metadata for ${name}: ${error}. Matching asset is present after refresh; continuing...`,
+          );
+          return latestAsset;
+        }
+      } catch (refreshError) {
+        console.warn(
+          `error refreshing release assets after metadata update failure: ${refreshError}`,
+        );
+      }
+    }
+
     // Handle race conditions across concurrent workflows uploading the same asset.
     if (
       config.input_overwrite_files !== false &&
@@ -359,8 +456,8 @@ export const upload = async (
         repo,
         release_id: releaseId,
       });
-      const latestAsset = latestAssets.find(
+      const latestAsset = latestAssets.find((currentAsset) =>
-        ({ name: currentName }) => currentName == alignAssetName(name),
+        releaseAssetMatchesName(name, currentAsset),
       );
       if (latestAsset) {
         await releaser.deleteReleaseAsset({
@@ -368,17 +465,7 @@ export const upload = async (
           repo,
           asset_id: latestAsset.id,
         });
-        const retryResp = await uploadAsset();
+        return await handleUploadedAsset(await uploadAsset());
-        const retryJson = retryResp.data;
-        if (retryResp.status !== 201) {
-          throw new Error(
-            `Failed to upload release asset ${name}. received status code ${
-              retryResp.status
-            }\n${retryJson.message}\n${JSON.stringify(retryJson.errors)}`,
-          );
-        }
-        console.log(`✅ Uploaded ${name}`);
-        return retryJson;
       }
     }