Skip to content

release(v3): [3/4] Remove dead error handling code #172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lasith-kg merged 1 commit into from
Jul 14, 2026
Merged

release(v3): [3/4] Remove dead error handling code #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 46 additions & 50 deletions __tests__/api.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,22 +126,35 @@
})
})

it('should throw if a non-200 status is returned', async () => {
const errorStatus = 401
// On instances where `return_run_details` is not honoured (older API versions / outdated GitHub Enterprise
// Server), createWorkflowDispatch falls back to its legacy 204 response, which Octokit resolves rather than
// throwing. The 200 guard turns that into a clear error instead of silently returning an undefined run ID.
it('should throw if a 204 status is returned because return_run_details was not honoured', async () => {
jest
.spyOn(mockOctokit.rest.actions, 'createWorkflowDispatch')
.mockReturnValue(
Promise.resolve({
data: undefined,
status: errorStatus
data: '',
status: 204
})
)

await expect(workflowDispatch()).rejects.toThrow(
`Failed to dispatch action, expected 200 but received ${errorStatus}`
`Failed to dispatch action, expected 200 but received 204`
)
})

// Octokit throws (rejects) for any status >= 400 rather than resolving with it. This test locks in that we let
// such errors propagate instead of swallowing them.
it('should propagate an error thrown by Octokit', async () => {
const error = new Error('createWorkflowDispatch failed')
jest
.spyOn(mockOctokit.rest.actions, 'createWorkflowDispatch')
.mockRejectedValue(error)

await expect(workflowDispatch()).rejects.toThrow(error)
})

// Regression test: returning run details currently requires explicitly passing `return_run_details: true`. From the
// `2026-03-10` API version onwards this becomes the default behaviour and the flag no longer needs to be passed, so
// this test can be removed once we adopt that API version.
Expand Down Expand Up @@ -197,7 +210,7 @@
init(mockActionConfig)
})

it('should resolve after a successful dispatch', async () => {

Check warning on line 213 in __tests__/api.test.ts

View workflow job for this annotation

GitHub Actions / integration-tests 

Test has no assertions

Check warning on line 213 in __tests__/api.test.ts

View workflow job for this annotation

GitHub Actions / integration-tests 

Test has no assertions

Check warning on line 213 in __tests__/api.test.ts

View workflow job for this annotation

GitHub Actions / integration-tests 

Test has no assertions
jest.spyOn(mockOctokit.rest.repos, 'createDispatchEvent').mockReturnValue(
Promise.resolve({
data: undefined,
Expand All @@ -208,18 +221,15 @@
await repositoryDispatch('')
})

it('should throw if a non-204 status is returned', async () => {
const errorStatus = 422
jest.spyOn(mockOctokit.rest.repos, 'createDispatchEvent').mockReturnValue(
Promise.resolve({
data: undefined,
status: errorStatus
})
)
// Octokit throws (rejects) for any status >= 400 rather than resolving with it, so there is no non-204 resolved
// response to guard against. This test locks in that we let such errors propagate instead of swallowing them.
it('should propagate an error thrown by Octokit', async () => {
const error = new Error('repository_dispatch failed')
jest
.spyOn(mockOctokit.rest.repos, 'createDispatchEvent')
.mockRejectedValue(error)

await expect(repositoryDispatch('')).rejects.toThrow(
`Failed to dispatch action, expected 204 but received ${errorStatus}`
)
await expect(repositoryDispatch('')).rejects.toThrow(error)
})

it('should dispatch with a distinctId in the inputs', async () => {
Expand Down Expand Up @@ -304,18 +314,15 @@
)
})

it('should throw if a non-200 status is returned', async () => {
const errorStatus = 401
jest.spyOn(mockOctokit.rest.actions, 'listRepoWorkflows').mockReturnValue(
Promise.resolve({
data: undefined,
status: errorStatus
})
)
// Octokit throws (rejects) for any status >= 400 rather than resolving with it. This test locks in that we let
// such errors propagate instead of swallowing them.
it('should propagate an error thrown by Octokit', async () => {
const error = new Error('listRepoWorkflows failed')
jest
.spyOn(mockOctokit.rest.actions, 'listRepoWorkflows')
.mockRejectedValue(error)

await expect(getWorkflowId('implode')).rejects.toThrow(
`Failed to get workflows, expected 200 but received ${errorStatus}`
)
await expect(getWorkflowId('implode')).rejects.toThrow(error)
})

it('should throw if a given workflow name cannot be found in the response', async () => {
Expand Down Expand Up @@ -350,18 +357,13 @@
expect(await getDefaultBranch()).toStrictEqual('main')
})

it('should throw if a non-200 status is returned', async () => {
const errorStatus = 404
jest.spyOn(mockOctokit.rest.repos, 'get').mockReturnValue(
Promise.resolve({
data: undefined,
status: errorStatus
})
)
// Octokit throws (rejects) for any status >= 400 rather than resolving with it. This test locks in that we let
// such errors propagate instead of swallowing them.
it('should propagate an error thrown by Octokit', async () => {
const error = new Error('repos.get failed')
jest.spyOn(mockOctokit.rest.repos, 'get').mockRejectedValue(error)

await expect(getDefaultBranch()).rejects.toThrow(
`Failed to get repository information, expected 200 but received ${errorStatus}`
)
await expect(getDefaultBranch()).rejects.toThrow(error)
})
})

Expand Down Expand Up @@ -413,21 +415,15 @@
expect(workflowRuns.length).toStrictEqual(mockData.workflow_runs.length)
})

it('should throw if a non-200 status is returned', async () => {
const errorStatus = 404

// Octokit throws (rejects) for any status >= 400 rather than resolving with it. This test locks in that we let
// such errors propagate instead of swallowing them.
it('should propagate an error thrown by Octokit', async () => {
const error = new Error('listWorkflowRunsForRepo failed')
jest
.spyOn(mockOctokit.rest.actions, 'listWorkflowRunsForRepo')
.mockReturnValue(
Promise.resolve({
data: undefined,
status: errorStatus
})
)
.mockRejectedValue(error)

await expect(getWorkflowRuns()).rejects.toThrow(
`Failed to get workflow runs, expected 200 but received ${errorStatus}`
)
await expect(getWorkflowRuns()).rejects.toThrow(error)
})
})
})
17 changes: 4 additions & 13 deletions dist/index.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/index.js.map
View file
Open in desktop

Large diffs are not rendered by default.

29 changes: 4 additions & 25 deletions src/api/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ export async function workflowDispatch(): Promise<WorkflowDispatch> {
return_run_details: true
})) as unknown as OctokitResponse<DispatchWorkflowResponse, 200>

// On instances where `return_run_details` is not honoured (outdated GitHub Enterprise Server),
// this endpoint falls back to its legacy `204 No Content` response, which Octokit resolves rather than throwing.
// Without this check we would silently return an undefined run ID and html_url.
if (response.status !== 200) {
throw new Error(
`workflow_dispatch: Failed to dispatch action, expected 200 but received ${response.status}`
Expand Down Expand Up @@ -75,19 +78,13 @@ export async function repositoryDispatch(distinctId: string): Promise<void> {
)
}
// https://docs.github.com/en/rest/reference/actions#create-a-workflow-dispatch-event
const response = await octokit.rest.repos.createDispatchEvent({
await octokit.rest.repos.createDispatchEvent({
owner: config.owner,
repo: config.repo,
event_type: config.eventType,
client_payload: clientPayload
})

if (response.status !== 204) {
throw new Error(
`repository_dispatch: Failed to dispatch action, expected 204 but received ${response.status}`
)
}

core.info(`✅ Successfully dispatched workflow using repository_dispatch method:
repository: ${config.owner}/${config.repo}
event-type: ${config.eventType}
Expand All @@ -102,12 +99,6 @@ export async function getWorkflowId(workflowFilename: string): Promise<number> {
repo: config.repo
})

if (response.status !== 200) {
throw new Error(
`Failed to get workflows, expected 200 but received ${response.status}`
)
}

const workflow = response.data.workflows.find((workflow) =>
workflow.path.includes(workflowFilename)
)
Expand Down Expand Up @@ -136,12 +127,6 @@ export async function getWorkflowRuns(): Promise<WorkflowRun[]> {
per_page: 5
})

if (response.status !== 200) {
throw new Error(
`getWorkflowRuns: Failed to get workflow runs, expected 200 but received ${response.status}`
)
}

const workflowRuns: WorkflowRun[] = response.data.workflow_runs.map(
(workflowRun) => ({
id: workflowRun.id,
Expand All @@ -165,12 +150,6 @@ export async function getDefaultBranch(): Promise<string> {
repo: config.repo
})

if (response.status !== 200) {
throw new Error(
`getDefaultBranch: Failed to get repository information, expected 200 but received ${response.status}`
)
}

core.debug(`
Fetched Repository Information
Repository: ${config.owner}/${config.repo}
Expand Down
Loading