Skip to content

Commit 5284765

Browse files
api2062Aditya Inamdarchenrui333
authored
fix: handle upload already_exists races across workflows (#745)
* Handle upload already_exists races across workflows * fix: rebase duplicate asset race handling Signed-off-by: Rui Chen <rui@chenrui.dev> --------- Signed-off-by: Rui Chen <rui@chenrui.dev> Co-authored-by: Aditya Inamdar <api2062@Adityas-MacBook-Air.local> Co-authored-by: Rui Chen <rui@chenrui.dev>
1 parent 4aadb0d commit 5284765

3 files changed

Lines changed: 142 additions & 39 deletions

File tree

__tests__/github.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,56 @@ describe('github', () => {
279279
);
280280
});
281281

282+
it('retries upload after deleting conflicting asset on 422 already_exists race', async () => {
283+
const uploadReleaseAsset = vi
284+
.fn()
285+
.mockRejectedValueOnce({
286+
status: 422,
287+
response: { data: { errors: [{ code: 'already_exists' }] } },
288+
})
289+
.mockResolvedValueOnce({
290+
status: 201,
291+
data: { id: 123, name: 'release.txt' },
292+
});
293+
294+
const listReleaseAssets = vi.fn().mockResolvedValue([{ id: 99, name: 'release.txt' }]);
295+
const deleteReleaseAsset = vi.fn().mockResolvedValue(undefined);
296+
297+
const mockReleaser: Releaser = {
298+
getReleaseByTag: () => Promise.reject('Not implemented'),
299+
createRelease: () => Promise.reject('Not implemented'),
300+
updateRelease: () => Promise.reject('Not implemented'),
301+
finalizeRelease: () => Promise.reject('Not implemented'),
302+
allReleases: async function* () {
303+
throw new Error('Not implemented');
304+
},
305+
listReleaseAssets,
306+
deleteReleaseAsset,
307+
uploadReleaseAsset,
308+
};
309+
310+
const result = await upload(
311+
config,
312+
mockReleaser,
313+
'https://uploads.github.com/repos/owner/repo/releases/1/assets',
314+
'__tests__/release.txt',
315+
[],
316+
);
317+
318+
expect(result).toStrictEqual({ id: 123, name: 'release.txt' });
319+
expect(listReleaseAssets).toHaveBeenCalledWith({
320+
owner: 'owner',
321+
repo: 'repo',
322+
release_id: 1,
323+
});
324+
expect(deleteReleaseAsset).toHaveBeenCalledWith({
325+
owner: 'owner',
326+
repo: 'repo',
327+
asset_id: 99,
328+
});
329+
expect(uploadReleaseAsset).toHaveBeenCalledTimes(2);
330+
});
331+
282332
it('handles 422 already_exists error gracefully', async () => {
283333
const existingRelease = {
284334
id: 1,

dist/index.js

Lines changed: 31 additions & 29 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/github.ts

Lines changed: 61 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,8 @@ export const upload = async (
289289
): Promise<any> => {
290290
const [owner, repo] = config.github_repository.split('/');
291291
const { name, mime, size } = asset(path);
292+
const releaseIdMatch = url.match(/\/releases\/(\d+)\/assets/);
293+
const releaseId = releaseIdMatch ? Number(releaseIdMatch[1]) : undefined;
292294
const currentAsset = currentAssets.find(
293295
// 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.
294296
// 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
@@ -312,15 +314,23 @@ export const upload = async (
312314
console.log(`⬆️ Uploading ${name}...`);
313315
const endpoint = new URL(url);
314316
endpoint.searchParams.append('name', name);
315-
const fh = await open(path);
317+
const uploadAsset = async () => {
318+
const fh = await open(path);
319+
try {
320+
return await releaser.uploadReleaseAsset({
321+
url: endpoint.toString(),
322+
size,
323+
mime,
324+
token: config.github_token,
325+
data: fh.readableWebStream({ type: 'bytes' }),
326+
});
327+
} finally {
328+
await fh.close();
329+
}
330+
};
331+
316332
try {
317-
const resp = await releaser.uploadReleaseAsset({
318-
url: endpoint.toString(),
319-
size,
320-
mime,
321-
token: config.github_token,
322-
data: fh.readableWebStream({ type: 'bytes' }),
323-
});
333+
const resp = await uploadAsset();
324334
const json = resp.data;
325335
if (resp.status !== 201) {
326336
throw new Error(
@@ -347,8 +357,49 @@ export const upload = async (
347357
}
348358
console.log(`✅ Uploaded ${name}`);
349359
return json;
350-
} finally {
351-
await fh.close();
360+
} catch (error: any) {
361+
const errorStatus = error?.status ?? error?.response?.status;
362+
const errorData = error?.response?.data;
363+
364+
// Handle race conditions across concurrent workflows uploading the same asset.
365+
if (
366+
config.input_overwrite_files !== false &&
367+
errorStatus === 422 &&
368+
errorData?.errors?.[0]?.code === 'already_exists' &&
369+
releaseId !== undefined
370+
) {
371+
console.log(
372+
`⚠️ Asset ${name} already exists (race condition), refreshing assets and retrying once...`,
373+
);
374+
const latestAssets = await releaser.listReleaseAssets({
375+
owner,
376+
repo,
377+
release_id: releaseId,
378+
});
379+
const latestAsset = latestAssets.find(
380+
({ name: currentName }) => currentName == alignAssetName(name),
381+
);
382+
if (latestAsset) {
383+
await releaser.deleteReleaseAsset({
384+
owner,
385+
repo,
386+
asset_id: latestAsset.id,
387+
});
388+
const retryResp = await uploadAsset();
389+
const retryJson = retryResp.data;
390+
if (retryResp.status !== 201) {
391+
throw new Error(
392+
`Failed to upload release asset ${name}. received status code ${
393+
retryResp.status
394+
}\n${retryJson.message}\n${JSON.stringify(retryJson.errors)}`,
395+
);
396+
}
397+
console.log(`✅ Uploaded ${name}`);
398+
return retryJson;
399+
}
400+
}
401+
402+
throw error;
352403
}
353404
};
354405

0 commit comments

Comments
 (0)