Skip to content

Commit 7e13ed4

Browse files
chenrui333chenrui333
authored
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>
1 parent e6c70a5 commit 7e13ed4

4 files changed

Lines changed: 355 additions & 23 deletions

File tree

‎__tests__/github.test.ts‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,110 @@ describe('github', () => {
793793
});
794794

795795
describe('error handling', () => {
796+
it.each([
797+
{
798+
name: 'a remote repository without a discussion category',
799+
repository: 'remote-owner/release-repo',
800+
category: undefined,
801+
},
802+
{
803+
name: 'the current repository without a discussion category',
804+
repository: 'owner/repo',
805+
category: undefined,
806+
},
807+
{
808+
name: 'a repository with a discussion category',
809+
repository: 'owner/repo',
810+
category: 'Announcements',
811+
},
812+
])('classifies a create-release 404 for $name without retrying', async (testCase) => {
813+
const releaseError = {
814+
status: 404,
815+
message: 'Not Found - create-a-release',
816+
};
817+
const log = vi.spyOn(console, 'log').mockImplementation(() => undefined);
818+
const createRelease = vi.fn().mockRejectedValue(releaseError);
819+
const releaser = createReleaser({
820+
getReleaseByTag: vi.fn().mockRejectedValue({ status: 404 }),
821+
createRelease,
822+
allReleases: async function* () {
823+
yield { data: [] };
824+
},
825+
});
826+
827+
const thrown = await release(
828+
{
829+
...config,
830+
github_repository: testCase.repository,
831+
input_discussion_category_name: testCase.category,
832+
},
833+
releaser,
834+
1,
835+
).catch((error) => error);
836+
837+
expect(thrown).toMatchObject({
838+
name: 'ReleaseCreationError',
839+
status: 404,
840+
cause: releaseError,
841+
});
842+
expect(thrown.message).toContain(
843+
`GitHub returned 404 while creating the release. Verify that ${testCase.repository} exists under the expected owner`,
844+
);
845+
expect(thrown.message).toContain('the token can access it');
846+
expect(thrown.message).toContain('fine-grained PAT');
847+
expect(thrown.message).toContain('Contents: write');
848+
expect(thrown.message).toContain('GitHub response: Not Found - create-a-release');
849+
if (testCase.category) {
850+
expect(thrown.message).toContain('Discussions and the requested category "Announcements"');
851+
} else {
852+
expect(thrown.message).not.toContain('discussion category mismatch');
853+
expect(thrown.message).not.toContain('requested category');
854+
}
855+
expect(createRelease).toHaveBeenCalledOnce();
856+
expect(log).not.toHaveBeenCalledWith(
857+
expect.stringContaining('Unexpected error fetching GitHub release'),
858+
);
859+
});
860+
861+
it('classifies a draft-listing 404 as a repository access failure', async () => {
862+
const listingError = {
863+
status: 404,
864+
message: 'Not Found - list-releases',
865+
};
866+
const createRelease = vi.fn();
867+
const releaser = createReleaser({
868+
getReleaseByTag: vi.fn().mockRejectedValue({ status: 404 }),
869+
allReleases: async function* () {
870+
throw listingError;
871+
},
872+
createRelease,
873+
});
874+
875+
const thrown = await release(
876+
{
877+
...config,
878+
github_repository: 'remote-owner/release-repo',
879+
input_discussion_category_name: undefined,
880+
},
881+
releaser,
882+
1,
883+
).catch((error) => error);
884+
885+
expect(thrown).toMatchObject({
886+
name: 'ReleaseAccessError',
887+
status: 404,
888+
cause: listingError,
889+
});
890+
expect(thrown.message).toContain('GitHub returned 404 while checking existing releases');
891+
expect(thrown.message).toContain('remote-owner/release-repo');
892+
expect(thrown.message).toContain('the token can access it');
893+
expect(thrown.message).toContain('fine-grained PAT');
894+
expect(thrown.message).toContain('Contents: write');
895+
expect(thrown.message).toContain('GitHub response: Not Found - list-releases');
896+
expect(thrown.message).not.toContain('discussion category mismatch');
897+
expect(createRelease).not.toHaveBeenCalled();
898+
});
899+
796900
it('reports a useful create error without assuming response data exists', async () => {
797901
const releaseError = {
798902
status: 403,

‎__tests__/release-create.test.ts‎

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
import { getOctokit } from '@actions/github';
2+
import { createServer, type Server } from 'http';
3+
import { type AddressInfo } from 'net';
4+
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
5+
import { GitHubReleaser, release, type Release } from '../src/github';
6+
import { parseConfig, type Config } from '../src/util';
7+
8+
type CapturedRequest = {
9+
path: string;
10+
authorization: string | undefined;
11+
contentType: string | undefined;
12+
body: Record<string, unknown>;
13+
};
14+
15+
const closeServer = async (server: Server): Promise<void> => {
16+
server.closeIdleConnections();
17+
server.closeAllConnections();
18+
await new Promise<void>((resolve, reject) => {
19+
server.close((error) => (error ? reject(error) : resolve()));
20+
});
21+
};
22+
23+
describe('release creation transport', () => {
24+
let server: Server;
25+
let baseUrl: string;
26+
const requests: CapturedRequest[] = [];
27+
const releases = new Map<string, Release>();
28+
29+
beforeAll(async () => {
30+
server = createServer(async (request, response) => {
31+
const url = new URL(request.url || '/', 'http://127.0.0.1');
32+
const tagMatch = url.pathname.match(/^\/repos\/owner\/remote\/releases\/tags\/(.+)$/);
33+
if (request.method === 'GET' && tagMatch) {
34+
const release = releases.get(decodeURIComponent(tagMatch[1]));
35+
response.writeHead(release ? 200 : 404, { 'content-type': 'application/json' });
36+
response.end(JSON.stringify(release ?? { message: 'Not Found' }));
37+
return;
38+
}
39+
40+
if (request.method === 'GET' && url.pathname === '/repos/owner/remote/releases') {
41+
response.writeHead(200, { 'content-type': 'application/json' });
42+
response.end(JSON.stringify([...releases.values()]));
43+
return;
44+
}
45+
46+
if (request.method === 'POST' && url.pathname === '/repos/owner/remote/releases') {
47+
const chunks: Buffer[] = [];
48+
for await (const chunk of request) {
49+
chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk));
50+
}
51+
const body = JSON.parse(Buffer.concat(chunks).toString('utf8')) as Record<string, unknown>;
52+
requests.push({
53+
path: url.pathname,
54+
authorization: request.headers.authorization,
55+
contentType: request.headers['content-type'],
56+
body,
57+
});
58+
const tag = String(body.tag_name);
59+
const createdRelease: Release = {
60+
id: requests.length,
61+
upload_url: `http://127.0.0.1/uploads/${requests.length}`,
62+
html_url: `http://127.0.0.1/releases/${requests.length}`,
63+
tag_name: tag,
64+
name: String(body.name),
65+
body: typeof body.body === 'string' ? body.body : null,
66+
target_commitish: 'main',
67+
draft: Boolean(body.draft),
68+
prerelease: Boolean(body.prerelease),
69+
assets: [],
70+
};
71+
releases.set(tag, createdRelease);
72+
response.writeHead(201, { 'content-type': 'application/json' });
73+
response.end(JSON.stringify(createdRelease));
74+
return;
75+
}
76+
77+
response.writeHead(500, { 'content-type': 'application/json' });
78+
response.end(
79+
JSON.stringify({ message: `Unexpected route ${request.method} ${url.pathname}` }),
80+
);
81+
});
82+
83+
await new Promise<void>((resolve, reject) => {
84+
server.once('error', reject);
85+
server.listen(0, '127.0.0.1', resolve);
86+
});
87+
const address = server.address() as AddressInfo;
88+
baseUrl = `http://127.0.0.1:${address.port}`;
89+
});
90+
91+
afterAll(async () => {
92+
await closeServer(server);
93+
});
94+
95+
it('serializes user-facing category inputs through the real Octokit request path', async () => {
96+
const parsedEmptyCategory = parseConfig({
97+
INPUT_DISCUSSION_CATEGORY_NAME: '',
98+
}).input_discussion_category_name;
99+
expect(parsedEmptyCategory).toBeUndefined();
100+
101+
const cases: Array<{
102+
name: string;
103+
categoryProperty: 'absent' | 'present';
104+
category: string | undefined;
105+
expectedCategory: string | undefined;
106+
}> = [
107+
{
108+
name: 'absent-category',
109+
categoryProperty: 'absent',
110+
category: undefined,
111+
expectedCategory: undefined,
112+
},
113+
{
114+
name: 'undefined-category',
115+
categoryProperty: 'present',
116+
category: undefined,
117+
expectedCategory: undefined,
118+
},
119+
{
120+
name: 'empty-input-category',
121+
categoryProperty: 'present',
122+
category: parsedEmptyCategory,
123+
expectedCategory: undefined,
124+
},
125+
{
126+
name: 'valid-category',
127+
categoryProperty: 'present',
128+
category: 'Announcements',
129+
expectedCategory: 'Announcements',
130+
},
131+
];
132+
133+
for (const testCase of cases) {
134+
const config: Config = {
135+
github_token: 'not-a-real-token',
136+
github_ref: 'refs/heads/main',
137+
github_repository: 'owner/remote',
138+
input_tag_name: testCase.name,
139+
input_name: `Release ${testCase.name}`,
140+
input_files: [],
141+
input_draft: false,
142+
input_prerelease: true,
143+
input_fail_on_unmatched_files: false,
144+
input_generate_release_notes: false,
145+
input_append_body: false,
146+
input_make_latest: undefined,
147+
};
148+
if (testCase.categoryProperty === 'present') {
149+
config.input_discussion_category_name = testCase.category;
150+
}
151+
152+
const releaser = new GitHubReleaser(
153+
getOctokit(config.github_token, {
154+
baseUrl,
155+
}),
156+
);
157+
await expect(release(config, releaser, 1)).resolves.toMatchObject({
158+
release: { tag_name: testCase.name },
159+
created: true,
160+
});
161+
162+
const request = requests.at(-1);
163+
expect(request).toMatchObject({
164+
path: '/repos/owner/remote/releases',
165+
authorization: 'token not-a-real-token',
166+
contentType: 'application/json; charset=utf-8',
167+
body: {
168+
tag_name: testCase.name,
169+
name: `Release ${testCase.name}`,
170+
draft: false,
171+
prerelease: true,
172+
generate_release_notes: false,
173+
},
174+
});
175+
if (testCase.expectedCategory) {
176+
expect(request?.body.discussion_category_name).toBe(testCase.expectedCategory);
177+
} else {
178+
expect(request?.body).not.toHaveProperty('discussion_category_name');
179+
}
180+
}
181+
});
182+
});

‎dist/index.js‎

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

‎src/github.ts‎

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,43 @@ type ReleaseMutationParams = {
6868
previous_tag_name?: string;
6969
};
7070

71+
class ReleaseCreationError extends Error {
72+
readonly status = 404;
73+
74+
constructor(message: string, cause: unknown) {
75+
super(message, { cause });
76+
this.name = 'ReleaseCreationError';
77+
}
78+
}
79+
80+
class ReleaseAccessError extends Error {
81+
readonly status = 404;
82+
83+
constructor(message: string, cause: unknown) {
84+
super(message, { cause });
85+
this.name = 'ReleaseAccessError';
86+
}
87+
}
88+
89+
const repositoryAccessGuidance = (owner: string, repo: string): string =>
90+
`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.`;
91+
92+
const releaseCreation404Message = (
93+
owner: string,
94+
repo: string,
95+
discussionCategory: string | undefined,
96+
error: unknown,
97+
): string => {
98+
const discussionGuidance = discussionCategory
99+
? ` Also verify that Discussions and the requested category "${discussionCategory}" are enabled.`
100+
: '';
101+
102+
return `GitHub returned 404 while creating the release. ${repositoryAccessGuidance(owner, repo)}${discussionGuidance} GitHub response: ${errorMessage(error)}`;
103+
};
104+
105+
const releaseLookup404Message = (owner: string, repo: string, error: unknown): string =>
106+
`GitHub returned 404 while checking existing releases. ${repositoryAccessGuidance(owner, repo)} GitHub response: ${errorMessage(error)}`;
107+
71108
export interface Releaser {
72109
getReleaseByTag(params: { owner: string; repo: string; tag: string }): Promise<{ data: Release }>;
73110

@@ -566,6 +603,11 @@ export const release = async (
566603
try {
567604
_release = await findTagFromReleases(releaser, owner, repo, tag, maxRetries);
568605
} catch (error) {
606+
if (error.status === 404) {
607+
const diagnostic = releaseLookup404Message(owner, repo, error);
608+
console.log(`⚠️ ${diagnostic}`);
609+
throw new ReleaseAccessError(diagnostic, error);
610+
}
569611
console.log(
570612
`⚠️ Unexpected error fetching GitHub release for tag ${config.github_ref}: ${error}`,
571613
);
@@ -644,6 +686,9 @@ export const release = async (
644686
created: false,
645687
};
646688
} catch (error) {
689+
if (error instanceof ReleaseCreationError) {
690+
throw error;
691+
}
647692
if (error.status !== 404) {
648693
console.log(
649694
`⚠️ Unexpected error fetching GitHub release for tag ${config.github_ref}: ${error}`,
@@ -1029,8 +1074,9 @@ async function createRelease(
10291074
throw error;
10301075

10311076
case 404:
1032-
console.log('Skip retry - discussion category mismatch');
1033-
throw error;
1077+
const diagnostic = releaseCreation404Message(owner, repo, discussion_category_name, error);
1078+
console.log(`Skip retry — ${diagnostic}`);
1079+
throw new ReleaseCreationError(diagnostic, error);
10341080

10351081
case 422:
10361082
// Check if this is a race condition with "already_exists" error

0 commit comments

Comments
 (0)