Github/action-gh-release
1
0
Fork 0
mirror of https://github.com/softprops/action-gh-release.git synced 2026-08-14 23:12:15 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

refactor: harden GitHub error classification

Browse Source 
Signed-off-by: Rui Chen <rui@chenrui.dev>
This commit is contained in:
Rui Chen
2026-07-15 07:22:33 -04:00
parent d100e75a45
commit 1150e0c3c8
5 changed files with 249 additions and 100 deletions
Download Patch File Download Diff File Expand all files Collapse all files
__tests__/github.test.ts
+150 -31
View File
@@ -346,11 +346,20 @@ describe('github', () => {
expect(request).not.toHaveBeenCalled();
});
it('falls back to release-scoped asset deletion after a standard 404', async () => {
const deleteReleaseAsset = vi.fn().mockRejectedValue({
status: 404,
message: 'standard route not found',
});
it.each([
{
name: 'top-level status',
deletionError: { status: 404, message: 'standard route not found' },
},
{
name: 'nested response status',
deletionError: {
response: { status: 404 },
message: 'standard route not found',
},
},
])('falls back to release-scoped asset deletion after a 404 with $name', async (testCase) => {
const deleteReleaseAsset = vi.fn().mockRejectedValue(testCase.deletionError);
const request = vi.fn().mockResolvedValue({ status: 204 });
const releaser = new GitHubReleaser({
rest: { repos: { deleteReleaseAsset } },
@@ -376,6 +385,30 @@ describe('github', () => {
);
});
it.each([
['a string', 'not found'],
['null', null],
['an arbitrary object', { response: { data: 'not found' } }],
])('does not misclassify %s as a fallback 404', async (_name, deletionError) => {
const request = vi.fn();
const releaser = new GitHubReleaser({
rest: {
repos: { deleteReleaseAsset: vi.fn().mockRejectedValue(deletionError) },
},
request,
} as any);
await expect(
releaser.deleteReleaseAsset({
owner: 'owner',
repo: 'repo',
release_id: 42,
asset_id: 99,
}),
).rejects.toBe(deletionError);
expect(request).not.toHaveBeenCalled();
});
it('does not mask non-404 asset deletion failures', async () => {
const deletionError = { status: 403, message: 'forbidden' };
const request = vi.fn();
@@ -790,6 +823,38 @@ describe('github', () => {
expect(finalizeReleaseSpy).toHaveBeenCalledTimes(1);
expect(deleteReleaseSpy).not.toHaveBeenCalled();
});
it('retries malformed tag-rule errors without deleting the draft', async () => {
const finalizeReleaseSpy = vi.fn().mockRejectedValue({
status: 422,
response: {
data: {
errors: [null, 'invalid', { field: 'pre_receive', message: 42 }],
},
},
});
const deleteReleaseSpy = vi.fn();
const releaser = createReleaser({
finalizeRelease: finalizeReleaseSpy,
deleteRelease: deleteReleaseSpy,
});
await expect(
finalizeRelease(
{
...config,
input_draft: false,
},
releaser,
draftRelease,
true,
1,
),
).rejects.toThrow('Too many retries.');
expect(finalizeReleaseSpy).toHaveBeenCalledOnce();
expect(deleteReleaseSpy).not.toHaveBeenCalled();
});
});
describe('error handling', () => {
@@ -917,6 +982,42 @@ describe('github', () => {
expect(log).not.toHaveBeenCalledWith('undefined');
});
it.each([
['non-object response data', { status: 422, response: { data: 'invalid' } }],
['missing validation errors', { status: 422, response: { data: {} } }],
['malformed validation errors', { status: 422, response: { data: { errors: [null] } } }],
])('does not retry a permanent 422 with %s', async (_name, releaseError) => {
const createRelease = vi.fn().mockRejectedValue(releaseError);
const releaser = createReleaser({
getReleaseByTag: vi.fn().mockRejectedValue({ status: 404 }),
createRelease,
allReleases: async function* () {
yield { data: [] };
},
});
await expect(release(config, releaser, 1)).rejects.toBe(releaseError);
expect(createRelease).toHaveBeenCalledOnce();
});
it.each([
['a string', 'transport failed'],
['null', null],
['an arbitrary object', { reason: 'transport failed' }],
])('preserves retry handling when release creation throws %s', async (_name, releaseError) => {
const createRelease = vi.fn().mockRejectedValue(releaseError);
const releaser = createReleaser({
getReleaseByTag: vi.fn().mockRejectedValue({ status: 404 }),
createRelease,
allReleases: async function* () {
yield { data: [] };
},
});
await expect(release(config, releaser, 1)).rejects.toThrow('Too many retries.');
expect(createRelease).toHaveBeenCalledOnce();
});
it('passes previous_tag_name through when creating a release with generated notes', async () => {
const createReleaseSpy = vi.fn(async () => ({
data: {
@@ -1263,19 +1364,37 @@ describe('github', () => {
expect(uploadReleaseAsset).not.toHaveBeenCalled();
});
it('surfaces an actionable immutable-release error for prerelease uploads', async () => {
it.each([
{
name: 'standard release with a nested status',
prerelease: undefined,
uploadError: {
response: { status: 422 },
message: 'Cannot upload assets to an immutable release.',
},
expected:
'Cannot upload asset draft-false.txt to an immutable release. GitHub only allows asset uploads before a release is published, so upload assets to a draft release before you publish it.',
},
{
name: 'prerelease with response data',
prerelease: true,
uploadError: {
status: 422,
response: {
data: {
message: 'Cannot upload assets to an immutable release.',
},
},
},
expected:
'Cannot upload asset draft-false.txt to an immutable release. GitHub only allows asset uploads before a release is published, but draft prereleases publish with the release.published event instead of release.prereleased.',
},
])('surfaces an actionable immutable-release error for a $name', async (testCase) => {
const tempDir = mkdtempSync(join(tmpdir(), 'gh-release-immutable-'));
const assetPath = join(tempDir, 'draft-false.txt');
writeFileSync(assetPath, 'hello');
const uploadReleaseAsset = vi.fn().mockRejectedValue({
status: 422,
response: {
data: {
message: 'Cannot upload assets to an immutable release.',
},
},
});
const uploadReleaseAsset = vi.fn().mockRejectedValue(testCase.uploadError);
const mockReleaser: Releaser = {
getReleaseByTag: () => Promise.reject('Not implemented'),
@@ -1292,23 +1411,23 @@ describe('github', () => {
uploadReleaseAsset,
};
await expect(
upload(
{
...config,
input_prerelease: true,
},
mockReleaser,
'https://uploads.github.com/repos/owner/repo/releases/1/assets',
assetPath,
[],
1,
),
).rejects.toThrow(
'Cannot upload asset draft-false.txt to an immutable release. GitHub only allows asset uploads before a release is published, but draft prereleases publish with the release.published event instead of release.prereleased.',
);
rmSync(tempDir, { recursive: true, force: true });
try {
await expect(
upload(
{
...config,
input_prerelease: testCase.prerelease,
},
mockReleaser,
'https://uploads.github.com/repos/owner/repo/releases/1/assets',
assetPath,
[],
1,
),
).rejects.toThrow(testCase.expected);
} finally {
rmSync(tempDir, { recursive: true, force: true });
}
});
it('retries upload after deleting a conflicting renamed asset matched by label', async () => {
dist/index.js
Vendored
+25 -25
View File
File diff suppressed because one or more lines are too long
src/github.ts
+69 -39
View File
@@ -9,6 +9,43 @@ type GitHub = InstanceType<typeof GitHub>;
type UploadChunk = ArrayBuffer | Uint8Array<ArrayBufferLike>;
type UploadBody = ReadableStream<Uint8Array<ArrayBufferLike>>;
type UnknownRecord = Record<string, unknown>;
const asRecord = (value: unknown): UnknownRecord | undefined =>
typeof value === 'object' && value !== null ? (value as UnknownRecord) : undefined;
const getErrorStatus = (error: unknown): number | undefined => {
const errorRecord = asRecord(error);
if (typeof errorRecord?.status === 'number') {
return errorRecord.status;
}
const response = asRecord(errorRecord?.response);
return typeof response?.status === 'number' ? response.status : undefined;
};
const getResponseData = (error: unknown): UnknownRecord | undefined => {
const response = asRecord(asRecord(error)?.response);
return asRecord(response?.data);
};
const getErrorMessage = (error: unknown): string | undefined => {
const message = asRecord(error)?.message;
return typeof message === 'string' ? message : undefined;
};
const getRequestUrl = (error: unknown): string | undefined => {
const request = asRecord(asRecord(error)?.request);
return typeof request?.url === 'string' ? request.url : undefined;
};
const getValidationErrors = (error: unknown): unknown[] => {
const errors = getResponseData(error)?.errors;
return Array.isArray(errors) ? errors : [];
};
const hasValidationErrorCode = (error: unknown, code: string): boolean =>
asRecord(getValidationErrors(error)[0])?.code === code;
const fileUploadStream = (fileHandle: FileHandle): UploadBody => {
const source = fileHandle.readableWebStream() as ReadableStream<UploadChunk>;
@@ -278,10 +315,7 @@ export class GitHubReleaser implements Releaser {
try {
await this.github.rest.repos.deleteReleaseAsset(githubParams);
} catch (error: unknown) {
const status =
(error as { status?: number; response?: { status?: number } })?.status ??
(error as { response?: { status?: number } })?.response?.status;
if (status !== 404) {
if (getErrorStatus(error) !== 404) {
throw error;
}
@@ -352,10 +386,10 @@ const releaseAssetMatchesName = (
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 isReleaseAssetUpdateNotFound = (error: unknown): boolean => {
const errorStatus = getErrorStatus(error);
const requestUrl = getRequestUrl(error);
const message = getErrorMessage(error);
const isReleaseAssetRequest =
typeof requestUrl === 'string' &&
(/\/releases\/assets\//.test(requestUrl) || /\/releases\/\d+\/assets(?:\?|$)/.test(requestUrl));
@@ -363,15 +397,15 @@ const isReleaseAssetUpdateNotFound = (error: any): boolean => {
return (
errorStatus === 404 &&
(isReleaseAssetRequest ||
(typeof errorMessage === 'string' && errorMessage.includes('update-a-release-asset')))
(typeof message === 'string' && message.includes('update-a-release-asset')))
);
};
const isImmutableReleaseAssetUploadFailure = (error: any): boolean => {
const errorStatus = error?.status ?? error?.response?.status;
const errorMessage = error?.response?.data?.message ?? error?.message;
const isImmutableReleaseAssetUploadFailure = (error: unknown): boolean => {
const errorStatus = getErrorStatus(error);
const message = getResponseData(error)?.message ?? getErrorMessage(error);
return errorStatus === 422 && /immutable release/i.test(String(errorMessage));
return errorStatus === 422 && /immutable release/i.test(String(message));
};
const immutableReleaseAssetUploadMessage = (
@@ -480,8 +514,8 @@ export const upload = async (
try {
return await updateAssetLabel(uploadedAsset.id);
} catch (error: any) {
const errorStatus = error?.status ?? error?.response?.status;
} catch (error: unknown) {
const errorStatus = getErrorStatus(error);
if (errorStatus === 404 && releaseId !== undefined) {
try {
@@ -518,9 +552,8 @@ export const upload = async (
try {
return await handleUploadedAsset(await uploadAsset());
} catch (error: any) {
const errorStatus = error?.status ?? error?.response?.status;
const errorData = error?.response?.data;
} catch (error: unknown) {
const errorStatus = getErrorStatus(error);
if (isImmutableReleaseAssetUploadFailure(error)) {
throw new Error(immutableReleaseAssetUploadMessage(name, config.input_prerelease));
@@ -548,7 +581,7 @@ export const upload = async (
if (
config.input_overwrite_files !== false &&
errorStatus === 422 &&
errorData?.errors?.[0]?.code === 'already_exists' &&
hasValidationErrorCode(error, 'already_exists') &&
releaseId !== undefined
) {
console.log(
@@ -603,7 +636,7 @@ export const release = async (
try {
_release = await findTagFromReleases(releaser, owner, repo, tag, maxRetries);
} catch (error) {
if (error.status === 404) {
if (getErrorStatus(error) === 404) {
const diagnostic = releaseLookup404Message(owner, repo, error);
console.log(`⚠️ ${diagnostic}`);
throw new ReleaseAccessError(diagnostic, error);
@@ -689,7 +722,7 @@ export const release = async (
if (error instanceof ReleaseCreationError) {
throw error;
}
if (error.status !== 404) {
if (getErrorStatus(error) !== 404) {
console.log(
`⚠️ Unexpected error fetching GitHub release for tag ${config.github_ref}: ${error}`,
);
@@ -840,7 +873,7 @@ export async function findTagFromReleases(
const { data: release } = await releaser.getReleaseByTag({ owner, repo, tag });
return release;
} catch (error) {
if (error.status !== 404) {
if (getErrorStatus(error) !== 404) {
throw error;
}
}
@@ -1058,15 +1091,12 @@ async function createRelease(
created: canonicalRelease.id === createdRelease.data.id,
};
} catch (error: unknown) {
const githubError = error as {
status?: number;
response?: { data?: { errors?: Array<{ code?: string }> } };
};
const errorStatus = getErrorStatus(error);
// presume a race with competing matrix runs
console.log(`⚠️ GitHub release failed with status: ${githubError.status}`);
console.log(`⚠️ GitHub release failed with status: ${errorStatus}`);
console.log(errorMessage(error));
switch (githubError.status) {
switch (errorStatus) {
case 403:
console.log(
'Skip retry — your GitHub token/PAT does not have the required permission to create a release',
@@ -1080,8 +1110,7 @@ async function createRelease(
case 422:
// Check if this is a race condition with "already_exists" error
const errorData = githubError.response?.data;
if (errorData?.errors?.[0]?.code === 'already_exists') {
if (hasValidationErrorCode(error, 'already_exists')) {
console.log(
'⚠️ Release already exists (race condition detected), retrying to find and update existing release...',
);
@@ -1098,16 +1127,17 @@ async function createRelease(
}
}
function isTagCreationBlockedError(error: any): boolean {
const errors = error?.response?.data?.errors;
if (!Array.isArray(errors) || error?.status !== 422) {
function isTagCreationBlockedError(error: unknown): boolean {
if (getErrorStatus(error) !== 422) {
return false;
}
return errors.some(
({ field, message }: { field?: string; message?: string }) =>
field === 'pre_receive' &&
typeof message === 'string' &&
message.includes('creations being restricted'),
);
return getValidationErrors(error).some((validationError) => {
const errorRecord = asRecord(validationError);
return (
errorRecord?.field === 'pre_receive' &&
typeof errorRecord.message === 'string' &&
errorRecord.message.includes('creations being restricted')
);
});
}
tsconfig.json
+1 -1
View File
@@ -1,6 +1,6 @@
{
"compilerOptions": {
"useUnknownInCatchVariables": false,
"useUnknownInCatchVariables": true,
/* Basic Options */
// "incremental": true, /* Enable incremental compilation */
"target": "es2022",
vitest.config.ts
+4 -4
View File
@@ -7,10 +7,10 @@ export default defineConfig({
reporter: ['text', 'json-summary', 'lcov'],
include: ['src/**/*.ts'],
thresholds: {
statements: 88,
branches: 83,
functions: 86,
lines: 88,
statements: 93,
branches: 89,
functions: 95,
lines: 93,
},
},
include: ['__tests__/**/*.ts'],