Skip to content

Commit 0271c06

Browse files
gjarzebak95gjarzebak95
committed
feat(SA-675): add error handling and surface API errors
- Add OperationError interface to track individual failures - Add OperationResult interface to collect successes and errors - Wrap GitHub API calls in try/catch blocks - Parse specific error codes (422, 403, 404) with helpful messages - Surface max user count errors (422) clearly - Collect all errors and display summary at end of run - Exit with non-zero when errors occur
1 parent 2cbad69 commit 0271c06

6 files changed

Lines changed: 228 additions & 91 deletions

File tree

‎index.ts‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { getGithubUsersFromGoogle } from './src/google'
2-
import { getGithubUsersFromGithub, addUsersToGitHubOrg, removeUsersFromGitHubOrg } from './src/github'
2+
import { getGithubUsersFromGithub, addUsersToGitHubOrg, removeUsersFromGitHubOrg, OperationError } from './src/github'
33
import { config } from './src/config'
44

55
export async function run(): Promise<void> {
@@ -13,11 +13,15 @@ export async function run(): Promise<void> {
1313

1414
const usersNotInGoogle = new Set(Array.from(gitHubUsers).filter((x) => !googleUsers.has(x)))
1515
let unfixedMismatch = false
16+
const allErrors: OperationError[] = []
1617

1718
if (usersNotInGithub.size > 0) {
1819
console.log(`Users not in github: ${Array.from(usersNotInGithub).join(', ')}`)
1920
if (config.addUsers) {
20-
await addUsersToGitHubOrg(usersNotInGithub)
21+
const result = await addUsersToGitHubOrg(usersNotInGithub)
22+
if (result.errors.length > 0) {
23+
allErrors.push(...result.errors)
24+
}
2125
} else {
2226
unfixedMismatch = true
2327
}
@@ -26,13 +30,25 @@ export async function run(): Promise<void> {
2630
if (usersNotInGoogle.size > 0) {
2731
console.log(`Users not in google: ${Array.from(usersNotInGoogle).join(', ')}`)
2832
if (config.removeUsers) {
29-
await removeUsersFromGitHubOrg(usersNotInGoogle)
33+
const result = await removeUsersFromGitHubOrg(usersNotInGoogle)
34+
if (result.errors.length > 0) {
35+
allErrors.push(...result.errors)
36+
}
3037
} else {
3138
unfixedMismatch = true
3239
}
3340
}
3441

35-
const exitCode = unfixedMismatch ? config.exitCodeOnMissmatch : 0
42+
if (allErrors.length > 0) {
43+
console.error(`\n--- ERRORS SUMMARY ---`)
44+
for (const err of allErrors) {
45+
console.error(`[${err.operation.toUpperCase()}] ${err.user}: ${err.message}`)
46+
}
47+
console.error(`Total errors: ${allErrors.length}`)
48+
}
49+
50+
const hasErrors = allErrors.length > 0
51+
const exitCode = unfixedMismatch || hasErrors ? config.exitCodeOnMissmatch : 0
3652

3753
process.exit(exitCode)
3854
}

‎src/github.ts‎

Lines changed: 71 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,18 @@ import { createAppAuth } from '@octokit/auth-app'
22
import { Octokit } from '@octokit/rest'
33
import * as mod from './github'
44
import { config } from './config'
5-
import { GetResponseTypeFromEndpointMethod } from '@octokit/types'
5+
6+
export interface OperationError {
7+
user: string
8+
operation: 'add' | 'remove'
9+
message: string
10+
status?: number
11+
}
12+
13+
export interface OperationResult {
14+
success: string[]
15+
errors: OperationError[]
16+
}
617

718
export function getAuthenticatedOctokit(): Octokit {
819
return new Octokit({
@@ -57,47 +68,85 @@ export async function getUserIdFromUsername(username: string): Promise<number> {
5768
return user.data.id
5869
}
5970

60-
export async function addUsersToGitHubOrg(users: Set<string>): Promise<void> {
71+
export async function addUsersToGitHubOrg(users: Set<string>): Promise<OperationResult> {
72+
const result: OperationResult = { success: [], errors: [] }
6173
for (const user of users) {
62-
await mod.addUserToGitHubOrg(user)
74+
const outcome = await mod.addUserToGitHubOrg(user)
75+
if (outcome === true) {
76+
result.success.push(user)
77+
} else if (outcome !== false && 'error' in outcome) {
78+
result.errors.push(outcome.error)
79+
}
6380
}
81+
return result
6482
}
6583

6684
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
67-
export async function addUserToGitHubOrg(
68-
user: string,
69-
): Promise<GetResponseTypeFromEndpointMethod<typeof octokit.orgs.createInvitation> | boolean> {
85+
export async function addUserToGitHubOrg(user: string): Promise<{ error: OperationError } | boolean> {
7086
const octokit = mod.getAuthenticatedOctokit()
7187
if (config.ignoredUsers.includes(user.toLowerCase())) {
7288
console.log(`Ignoring add for ${user}`)
7389
return false
7490
}
75-
const userId = await mod.getUserIdFromUsername(user)
76-
console.log(`Inviting ${user} (${userId} to ${config.githubOrg})`)
77-
return await octokit.orgs.createInvitation({
78-
org: config.githubOrg,
79-
invitee_id: userId,
80-
})
91+
try {
92+
const userId = await mod.getUserIdFromUsername(user)
93+
console.log(`Inviting ${user} (${userId}) to ${config.githubOrg}`)
94+
await octokit.orgs.createInvitation({
95+
org: config.githubOrg,
96+
invitee_id: userId,
97+
})
98+
return true
99+
} catch (error) {
100+
const status = error?.status || error?.response?.status
101+
let message = error?.message || String(error)
102+
if (status === 422) {
103+
message = `Validation failed: ${message} (user may already be invited, or org is at max capacity)`
104+
} else if (status === 404) {
105+
message = `User not found: ${user}`
106+
} else if (status === 403) {
107+
message = `Permission denied or rate limited`
108+
}
109+
console.error(`Error adding ${user}: ${message}`)
110+
return { error: { user, operation: 'add', message, status } }
111+
}
81112
}
82113

83-
export async function removeUsersFromGitHubOrg(users: Set<string>): Promise<void> {
114+
export async function removeUsersFromGitHubOrg(users: Set<string>): Promise<OperationResult> {
115+
const result: OperationResult = { success: [], errors: [] }
84116
for (const user of users) {
85-
await mod.removeUserFromGitHubOrg(user)
117+
const outcome = await mod.removeUserFromGitHubOrg(user)
118+
if (outcome === true) {
119+
result.success.push(user)
120+
} else if (outcome !== false && 'error' in outcome) {
121+
result.errors.push(outcome.error)
122+
}
86123
}
124+
return result
87125
}
88126

89127
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
90-
export async function removeUserFromGitHubOrg(
91-
user: string,
92-
): Promise<GetResponseTypeFromEndpointMethod<typeof octokit.orgs.removeMembershipForUser> | boolean> {
128+
export async function removeUserFromGitHubOrg(user: string): Promise<{ error: OperationError } | boolean> {
93129
const octokit = mod.getAuthenticatedOctokit()
94130
if (config.ignoredUsers.includes(user.toLowerCase())) {
95131
console.log(`Ignoring remove for ${user}`)
96132
return false
97133
}
98-
console.log(`Removing user/invitation ${user} from ${config.githubOrg}`)
99-
return octokit.orgs.removeMembershipForUser({
100-
org: config.githubOrg,
101-
username: user,
102-
})
134+
try {
135+
console.log(`Removing user/invitation ${user} from ${config.githubOrg}`)
136+
await octokit.orgs.removeMembershipForUser({
137+
org: config.githubOrg,
138+
username: user,
139+
})
140+
return true
141+
} catch (error) {
142+
const status = error?.status || error?.response?.status
143+
let message = error?.message || String(error)
144+
if (status === 404) {
145+
message = `User not found or not a member: ${user}`
146+
} else if (status === 403) {
147+
message = `Permission denied or rate limited`
148+
}
149+
console.error(`Error removing ${user}: ${message}`)
150+
return { error: { user, operation: 'remove', message, status } }
151+
}
103152
}

‎tests/__snapshots__/github.spec.ts.snap‎

Lines changed: 2 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3-
exports[`github integration addUserToGitHubOrg 1`] = `
3+
exports[`github integration addUserToGitHubOrg success 1`] = `
44
[MockFunction] {
55
"calls": Array [
66
Array [
@@ -19,29 +19,6 @@ exports[`github integration addUserToGitHubOrg 1`] = `
1919
}
2020
`;
2121

22-
exports[`github integration addUsersToGitHubOrg 1`] = `
23-
[MockFunction] {
24-
"calls": Array [
25-
Array [
26-
"foo",
27-
],
28-
Array [
29-
"bar",
30-
],
31-
],
32-
"results": Array [
33-
Object {
34-
"type": "return",
35-
"value": Promise {},
36-
},
37-
Object {
38-
"type": "return",
39-
"value": Promise {},
40-
},
41-
],
42-
}
43-
`;
44-
4522
exports[`github integration getAuthenticatedOctokit 1`] = `
4623
[MockFunction] {
4724
"calls": Array [
@@ -79,7 +56,7 @@ exports[`github integration getUserIdFromUsername found 1`] = `123`;
7956

8057
exports[`github integration getUserIdFromUsername notfound 1`] = `"Unable to find user id for foo"`;
8158

82-
exports[`github integration removeUserFromGitHubOrg 1`] = `
59+
exports[`github integration removeUserFromGitHubOrg success 1`] = `
8360
[MockFunction] {
8461
"calls": Array [
8562
Array [
@@ -97,26 +74,3 @@ exports[`github integration removeUserFromGitHubOrg 1`] = `
9774
],
9875
}
9976
`;
100-
101-
exports[`github integration removeUsersFromGitHubOrg 1`] = `
102-
[MockFunction] {
103-
"calls": Array [
104-
Array [
105-
"foo",
106-
],
107-
Array [
108-
"bar",
109-
],
110-
],
111-
"results": Array [
112-
Object {
113-
"type": "return",
114-
"value": Promise {},
115-
},
116-
Object {
117-
"type": "return",
118-
"value": Promise {},
119-
},
120-
],
121-
}
122-
`;

‎tests/__snapshots__/index.spec.ts.snap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ exports[`missmatch should add users if set to 1`] = `
3535
"results": Array [
3636
Object {
3737
"type": "return",
38-
"value": undefined,
38+
"value": Promise {},
3939
},
4040
],
4141
}
@@ -128,7 +128,7 @@ exports[`missmatch should remove users if set to 1`] = `
128128
"results": Array [
129129
Object {
130130
"type": "return",
131-
"value": undefined,
131+
"value": Promise {},
132132
},
133133
],
134134
}

0 commit comments

Comments
 (0)