mirror of
https://gh-proxy.org/https://github.com/softprops/action-gh-release.git
synced 2026-07-15 23:13:02 +08:00
fix: clarify release creation 404 errors (#817)
* fix: clarify release creation 404 errors Signed-off-by: Rui Chen <rui@chenrui.dev> * fix: clarify inaccessible release targets Signed-off-by: Rui Chen <rui@chenrui.dev> --------- Signed-off-by: Rui Chen <rui@chenrui.dev>
This commit is contained in:
@@ -793,6 +793,110 @@ describe('github', () => {
|
||||
});
|
||||
|
||||
describe('error handling', () => {
|
||||
it.each([
|
||||
{
|
||||
name: 'a remote repository without a discussion category',
|
||||
repository: 'remote-owner/release-repo',
|
||||
category: undefined,
|
||||
},
|
||||
{
|
||||
name: 'the current repository without a discussion category',
|
||||
repository: 'owner/repo',
|
||||
category: undefined,
|
||||
},
|
||||
{
|
||||
name: 'a repository with a discussion category',
|
||||
repository: 'owner/repo',
|
||||
category: 'Announcements',
|
||||
},
|
||||
])('classifies a create-release 404 for $name without retrying', async (testCase) => {
|
||||
const releaseError = {
|
||||
status: 404,
|
||||
message: 'Not Found - create-a-release',
|
||||
};
|
||||
const log = vi.spyOn(console, 'log').mockImplementation(() => undefined);
|
||||
const createRelease = vi.fn().mockRejectedValue(releaseError);
|
||||
const releaser = createReleaser({
|
||||
getReleaseByTag: vi.fn().mockRejectedValue({ status: 404 }),
|
||||
createRelease,
|
||||
allReleases: async function* () {
|
||||
yield { data: [] };
|
||||
},
|
||||
});
|
||||
|
||||
const thrown = await release(
|
||||
{
|
||||
...config,
|
||||
github_repository: testCase.repository,
|
||||
input_discussion_category_name: testCase.category,
|
||||
},
|
||||
releaser,
|
||||
1,
|
||||
).catch((error) => error);
|
||||
|
||||
expect(thrown).toMatchObject({
|
||||
name: 'ReleaseCreationError',
|
||||
status: 404,
|
||||
cause: releaseError,
|
||||
});
|
||||
expect(thrown.message).toContain(
|
||||
`GitHub returned 404 while creating the release. Verify that ${testCase.repository} exists under the expected owner`,
|
||||
);
|
||||
expect(thrown.message).toContain('the token can access it');
|
||||
expect(thrown.message).toContain('fine-grained PAT');
|
||||
expect(thrown.message).toContain('Contents: write');
|
||||
expect(thrown.message).toContain('GitHub response: Not Found - create-a-release');
|
||||
if (testCase.category) {
|
||||
expect(thrown.message).toContain('Discussions and the requested category "Announcements"');
|
||||
} else {
|
||||
expect(thrown.message).not.toContain('discussion category mismatch');
|
||||
expect(thrown.message).not.toContain('requested category');
|
||||
}
|
||||
expect(createRelease).toHaveBeenCalledOnce();
|
||||
expect(log).not.toHaveBeenCalledWith(
|
||||
expect.stringContaining('Unexpected error fetching GitHub release'),
|
||||
);
|
||||
});
|
||||
|
||||
it('classifies a draft-listing 404 as a repository access failure', async () => {
|
||||
const listingError = {
|
||||
status: 404,
|
||||
message: 'Not Found - list-releases',
|
||||
};
|
||||
const createRelease = vi.fn();
|
||||
const releaser = createReleaser({
|
||||
getReleaseByTag: vi.fn().mockRejectedValue({ status: 404 }),
|
||||
allReleases: async function* () {
|
||||
throw listingError;
|
||||
},
|
||||
createRelease,
|
||||
});
|
||||
|
||||
const thrown = await release(
|
||||
{
|
||||
...config,
|
||||
github_repository: 'remote-owner/release-repo',
|
||||
input_discussion_category_name: undefined,
|
||||
},
|
||||
releaser,
|
||||
1,
|
||||
).catch((error) => error);
|
||||
|
||||
expect(thrown).toMatchObject({
|
||||
name: 'ReleaseAccessError',
|
||||
status: 404,
|
||||
cause: listingError,
|
||||
});
|
||||
expect(thrown.message).toContain('GitHub returned 404 while checking existing releases');
|
||||
expect(thrown.message).toContain('remote-owner/release-repo');
|
||||
expect(thrown.message).toContain('the token can access it');
|
||||
expect(thrown.message).toContain('fine-grained PAT');
|
||||
expect(thrown.message).toContain('Contents: write');
|
||||
expect(thrown.message).toContain('GitHub response: Not Found - list-releases');
|
||||
expect(thrown.message).not.toContain('discussion category mismatch');
|
||||
expect(createRelease).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('reports a useful create error without assuming response data exists', async () => {
|
||||
const releaseError = {
|
||||
status: 403,
|
||||
|
||||
@@ -0,0 +1,182 @@
|
||||
import { getOctokit } from '@actions/github';
|
||||
import { createServer, type Server } from 'http';
|
||||
import { type AddressInfo } from 'net';
|
||||
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
|
||||
import { GitHubReleaser, release, type Release } from '../src/github';
|
||||
import { parseConfig, type Config } from '../src/util';
|
||||
|
||||
type CapturedRequest = {
|
||||
path: string;
|
||||
authorization: string | undefined;
|
||||
contentType: string | undefined;
|
||||
body: Record<string, unknown>;
|
||||
};
|
||||
|
||||
const closeServer = async (server: Server): Promise<void> => {
|
||||
server.closeIdleConnections();
|
||||
server.closeAllConnections();
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
server.close((error) => (error ? reject(error) : resolve()));
|
||||
});
|
||||
};
|
||||
|
||||
describe('release creation transport', () => {
|
||||
let server: Server;
|
||||
let baseUrl: string;
|
||||
const requests: CapturedRequest[] = [];
|
||||
const releases = new Map<string, Release>();
|
||||
|
||||
beforeAll(async () => {
|
||||
server = createServer(async (request, response) => {
|
||||
const url = new URL(request.url || '/', 'http://127.0.0.1');
|
||||
const tagMatch = url.pathname.match(/^\/repos\/owner\/remote\/releases\/tags\/(.+)$/);
|
||||
if (request.method === 'GET' && tagMatch) {
|
||||
const release = releases.get(decodeURIComponent(tagMatch[1]));
|
||||
response.writeHead(release ? 200 : 404, { 'content-type': 'application/json' });
|
||||
response.end(JSON.stringify(release ?? { message: 'Not Found' }));
|
||||
return;
|
||||
}
|
||||
|
||||
if (request.method === 'GET' && url.pathname === '/repos/owner/remote/releases') {
|
||||
response.writeHead(200, { 'content-type': 'application/json' });
|
||||
response.end(JSON.stringify([...releases.values()]));
|
||||
return;
|
||||
}
|
||||
|
||||
if (request.method === 'POST' && url.pathname === '/repos/owner/remote/releases') {
|
||||
const chunks: Buffer[] = [];
|
||||
for await (const chunk of request) {
|
||||
chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk));
|
||||
}
|
||||
const body = JSON.parse(Buffer.concat(chunks).toString('utf8')) as Record<string, unknown>;
|
||||
requests.push({
|
||||
path: url.pathname,
|
||||
authorization: request.headers.authorization,
|
||||
contentType: request.headers['content-type'],
|
||||
body,
|
||||
});
|
||||
const tag = String(body.tag_name);
|
||||
const createdRelease: Release = {
|
||||
id: requests.length,
|
||||
upload_url: `http://127.0.0.1/uploads/${requests.length}`,
|
||||
html_url: `http://127.0.0.1/releases/${requests.length}`,
|
||||
tag_name: tag,
|
||||
name: String(body.name),
|
||||
body: typeof body.body === 'string' ? body.body : null,
|
||||
target_commitish: 'main',
|
||||
draft: Boolean(body.draft),
|
||||
prerelease: Boolean(body.prerelease),
|
||||
assets: [],
|
||||
};
|
||||
releases.set(tag, createdRelease);
|
||||
response.writeHead(201, { 'content-type': 'application/json' });
|
||||
response.end(JSON.stringify(createdRelease));
|
||||
return;
|
||||
}
|
||||
|
||||
response.writeHead(500, { 'content-type': 'application/json' });
|
||||
response.end(
|
||||
JSON.stringify({ message: `Unexpected route ${request.method} ${url.pathname}` }),
|
||||
);
|
||||
});
|
||||
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
server.once('error', reject);
|
||||
server.listen(0, '127.0.0.1', resolve);
|
||||
});
|
||||
const address = server.address() as AddressInfo;
|
||||
baseUrl = `http://127.0.0.1:${address.port}`;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await closeServer(server);
|
||||
});
|
||||
|
||||
it('serializes user-facing category inputs through the real Octokit request path', async () => {
|
||||
const parsedEmptyCategory = parseConfig({
|
||||
INPUT_DISCUSSION_CATEGORY_NAME: '',
|
||||
}).input_discussion_category_name;
|
||||
expect(parsedEmptyCategory).toBeUndefined();
|
||||
|
||||
const cases: Array<{
|
||||
name: string;
|
||||
categoryProperty: 'absent' | 'present';
|
||||
category: string | undefined;
|
||||
expectedCategory: string | undefined;
|
||||
}> = [
|
||||
{
|
||||
name: 'absent-category',
|
||||
categoryProperty: 'absent',
|
||||
category: undefined,
|
||||
expectedCategory: undefined,
|
||||
},
|
||||
{
|
||||
name: 'undefined-category',
|
||||
categoryProperty: 'present',
|
||||
category: undefined,
|
||||
expectedCategory: undefined,
|
||||
},
|
||||
{
|
||||
name: 'empty-input-category',
|
||||
categoryProperty: 'present',
|
||||
category: parsedEmptyCategory,
|
||||
expectedCategory: undefined,
|
||||
},
|
||||
{
|
||||
name: 'valid-category',
|
||||
categoryProperty: 'present',
|
||||
category: 'Announcements',
|
||||
expectedCategory: 'Announcements',
|
||||
},
|
||||
];
|
||||
|
||||
for (const testCase of cases) {
|
||||
const config: Config = {
|
||||
github_token: 'not-a-real-token',
|
||||
github_ref: 'refs/heads/main',
|
||||
github_repository: 'owner/remote',
|
||||
input_tag_name: testCase.name,
|
||||
input_name: `Release ${testCase.name}`,
|
||||
input_files: [],
|
||||
input_draft: false,
|
||||
input_prerelease: true,
|
||||
input_fail_on_unmatched_files: false,
|
||||
input_generate_release_notes: false,
|
||||
input_append_body: false,
|
||||
input_make_latest: undefined,
|
||||
};
|
||||
if (testCase.categoryProperty === 'present') {
|
||||
config.input_discussion_category_name = testCase.category;
|
||||
}
|
||||
|
||||
const releaser = new GitHubReleaser(
|
||||
getOctokit(config.github_token, {
|
||||
baseUrl,
|
||||
}),
|
||||
);
|
||||
await expect(release(config, releaser, 1)).resolves.toMatchObject({
|
||||
release: { tag_name: testCase.name },
|
||||
created: true,
|
||||
});
|
||||
|
||||
const request = requests.at(-1);
|
||||
expect(request).toMatchObject({
|
||||
path: '/repos/owner/remote/releases',
|
||||
authorization: 'token not-a-real-token',
|
||||
contentType: 'application/json; charset=utf-8',
|
||||
body: {
|
||||
tag_name: testCase.name,
|
||||
name: `Release ${testCase.name}`,
|
||||
draft: false,
|
||||
prerelease: true,
|
||||
generate_release_notes: false,
|
||||
},
|
||||
});
|
||||
if (testCase.expectedCategory) {
|
||||
expect(request?.body.discussion_category_name).toBe(testCase.expectedCategory);
|
||||
} else {
|
||||
expect(request?.body).not.toHaveProperty('discussion_category_name');
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
Vendored
+21
-21
File diff suppressed because one or more lines are too long
+48
-2
@@ -68,6 +68,43 @@ type ReleaseMutationParams = {
|
||||
previous_tag_name?: string;
|
||||
};
|
||||
|
||||
class ReleaseCreationError extends Error {
|
||||
readonly status = 404;
|
||||
|
||||
constructor(message: string, cause: unknown) {
|
||||
super(message, { cause });
|
||||
this.name = 'ReleaseCreationError';
|
||||
}
|
||||
}
|
||||
|
||||
class ReleaseAccessError extends Error {
|
||||
readonly status = 404;
|
||||
|
||||
constructor(message: string, cause: unknown) {
|
||||
super(message, { cause });
|
||||
this.name = 'ReleaseAccessError';
|
||||
}
|
||||
}
|
||||
|
||||
const repositoryAccessGuidance = (owner: string, repo: string): string =>
|
||||
`Verify that ${owner}/${repo} exists under the expected owner, the token can access it, the repository is selected when using a fine-grained PAT, and the token has Contents: write permission.`;
|
||||
|
||||
const releaseCreation404Message = (
|
||||
owner: string,
|
||||
repo: string,
|
||||
discussionCategory: string | undefined,
|
||||
error: unknown,
|
||||
): string => {
|
||||
const discussionGuidance = discussionCategory
|
||||
? ` Also verify that Discussions and the requested category "${discussionCategory}" are enabled.`
|
||||
: '';
|
||||
|
||||
return `GitHub returned 404 while creating the release. ${repositoryAccessGuidance(owner, repo)}${discussionGuidance} GitHub response: ${errorMessage(error)}`;
|
||||
};
|
||||
|
||||
const releaseLookup404Message = (owner: string, repo: string, error: unknown): string =>
|
||||
`GitHub returned 404 while checking existing releases. ${repositoryAccessGuidance(owner, repo)} GitHub response: ${errorMessage(error)}`;
|
||||
|
||||
export interface Releaser {
|
||||
getReleaseByTag(params: { owner: string; repo: string; tag: string }): Promise<{ data: Release }>;
|
||||
|
||||
@@ -566,6 +603,11 @@ export const release = async (
|
||||
try {
|
||||
_release = await findTagFromReleases(releaser, owner, repo, tag, maxRetries);
|
||||
} catch (error) {
|
||||
if (error.status === 404) {
|
||||
const diagnostic = releaseLookup404Message(owner, repo, error);
|
||||
console.log(`⚠️ ${diagnostic}`);
|
||||
throw new ReleaseAccessError(diagnostic, error);
|
||||
}
|
||||
console.log(
|
||||
`⚠️ Unexpected error fetching GitHub release for tag ${config.github_ref}: ${error}`,
|
||||
);
|
||||
@@ -644,6 +686,9 @@ export const release = async (
|
||||
created: false,
|
||||
};
|
||||
} catch (error) {
|
||||
if (error instanceof ReleaseCreationError) {
|
||||
throw error;
|
||||
}
|
||||
if (error.status !== 404) {
|
||||
console.log(
|
||||
`⚠️ Unexpected error fetching GitHub release for tag ${config.github_ref}: ${error}`,
|
||||
@@ -1029,8 +1074,9 @@ async function createRelease(
|
||||
throw error;
|
||||
|
||||
case 404:
|
||||
console.log('Skip retry - discussion category mismatch');
|
||||
throw error;
|
||||
const diagnostic = releaseCreation404Message(owner, repo, discussion_category_name, error);
|
||||
console.log(`Skip retry — ${diagnostic}`);
|
||||
throw new ReleaseCreationError(diagnostic, error);
|
||||
|
||||
case 422:
|
||||
// Check if this is a race condition with "already_exists" error
|
||||
|
||||
Reference in New Issue
Block a user