diff --git a/__tests__/issueCommentTest/approveMilestoneErrorPaths.test.ts b/__tests__/issueCommentTest/approveMilestoneErrorPaths.test.ts index 7bcc285..482405e 100644 --- a/__tests__/issueCommentTest/approveMilestoneErrorPaths.test.ts +++ b/__tests__/issueCommentTest/approveMilestoneErrorPaths.test.ts @@ -11,8 +11,8 @@ import issueCommentEvent from '../fixtures/issues/issueCommentEvent.json' import pullReqListReviews from '../fixtures/pullReq/pullReqListReviews.json' import * as utils from '../testUtils' -// the auth helpers swallow HTTP failures themselves, so the handlers' own -// "could not check auth" branches are only reachable when the helper rejects +// the auth helpers are stubbed so each error path can be reached +// without routing the membership, collaborator and comment probes vi.mock('../../src/utils/auth', { spy: true }) const server = setupServer() @@ -99,12 +99,4 @@ describe('/milestone error paths', () => { /missing issue number/, ) }) - - it('wraps a failed collaborator check', async () => { - vi.mocked(auth.checkCollaborator).mockRejectedValueOnce(new Error('auth exploded')) - - await expect(milestone(comment('/milestone v1'))).rejects.toThrow( - /could not check commenter auth: Error: auth exploded/, - ) - }) }) diff --git a/__tests__/issueCommentTest/errorPaths.test.ts b/__tests__/issueCommentTest/errorPaths.test.ts index 1991f33..41baf14 100644 --- a/__tests__/issueCommentTest/errorPaths.test.ts +++ b/__tests__/issueCommentTest/errorPaths.test.ts @@ -15,8 +15,8 @@ import * as auth from '../../src/utils/auth' import issueCommentEvent from '../fixtures/issues/issueCommentEvent.json' import * as utils from '../testUtils' -// the auth helpers swallow HTTP failures themselves, so the handlers' own -// "could not check auth" branches are only reachable when the helper rejects +// the auth helpers are stubbed so each error path can be reached +// without routing the membership, collaborator and comment probes vi.mock('../../src/utils/auth', { spy: true }) const server = setupServer() @@ -30,7 +30,6 @@ const issue = `${utils.api}/repos/Codertocat/Hello-World/issues/1` const pull = `${utils.api}/repos/Codertocat/Hello-World/pulls/1` const forbidden = { message: 'Resource not accessible by integration' } const author = issueCommentEvent.issue.user.login -const stranger = 'not-the-author' function comment(body: string, commenter = author) { const payload = structuredClone(issueCommentEvent) @@ -67,14 +66,6 @@ describe('/assign error paths', () => { await expect(assign(comment('/assign'))).rejects.toThrow(/could not self assign/) }) - it('wraps a failed authorization lookup', async () => { - vi.mocked(auth.getOrgCollabCommentUsers).mockRejectedValueOnce(new Error('lookup exploded')) - - await expect(assign(comment('/assign @some-user'))).rejects.toThrow( - /could not get authorized users: Error: lookup exploded/, - ) - }) - it('wraps a failed assignment of authorized users', async () => { vi.mocked(auth.getOrgCollabCommentUsers).mockResolvedValueOnce(['some-user']) failing('post', `${issue}/assignees`) @@ -96,14 +87,6 @@ describe('/unassign error paths', () => { await expect(unassign(comment('/unassign'))).rejects.toThrow(/could not remove assignee/) }) - it('wraps a failed commenter authorization check', async () => { - vi.mocked(auth.checkCommenterAuth).mockRejectedValueOnce(new Error('auth exploded')) - - await expect(unassign(comment('/unassign @some-user'))).rejects.toThrow( - /Error: auth exploded/, - ) - }) - it('wraps a failed unassignment of another user', async () => { vi.mocked(auth.checkCommenterAuth).mockResolvedValueOnce(true) failing('delete', `${issue}/assignees`) @@ -126,14 +109,6 @@ describe('/cc error paths', () => { await expect(cc(comment('/cc'))).rejects.toThrow(/could not self cc/) }) - it('wraps a failed authorization lookup', async () => { - vi.mocked(auth.getOrgCollabCommentUsers).mockRejectedValueOnce(new Error('lookup exploded')) - - await expect(cc(comment('/cc @some-user'))).rejects.toThrow( - /could not get authorized users: Error: lookup exploded/, - ) - }) - it('wraps a failed review request for authorized users', async () => { vi.mocked(auth.getOrgCollabCommentUsers).mockResolvedValueOnce(['some-user']) failing('post', `${pull}/requested_reviewers`) @@ -153,14 +128,6 @@ describe('/uncc error paths', () => { await expect(uncc(comment('/uncc'))).rejects.toThrow(/could not self uncc/) }) - - it('wraps a failed commenter authorization check', async () => { - vi.mocked(auth.checkCommenterAuth).mockRejectedValueOnce(new Error('auth exploded')) - - await expect(uncc(comment('/uncc @some-user'))).rejects.toThrow( - /could not get authorized users: Error: auth exploded/, - ) - }) }) describe('/reopen error paths', () => { @@ -170,14 +137,6 @@ describe('/reopen error paths', () => { ) }) - it('wraps a failed collaborator check for a non-author', async () => { - vi.mocked(auth.checkCollaborator).mockRejectedValueOnce(new Error('auth exploded')) - - await expect(reopen(comment('/reopen', stranger))).rejects.toThrow( - /could not check commentor auth: Error: auth exploded/, - ) - }) - it('wraps a failed reopen', async () => { failing('patch', issue) @@ -192,14 +151,6 @@ describe('/close error paths', () => { ) }) - it('wraps a failed collaborator check for a non-author', async () => { - vi.mocked(auth.checkCollaborator).mockRejectedValueOnce(new Error('auth exploded')) - - await expect(close(comment('/close', stranger))).rejects.toThrow( - /could not check commentor auth: Error: auth exploded/, - ) - }) - it('wraps a failed close', async () => { failing('patch', issue) @@ -214,14 +165,6 @@ describe('/retitle error paths', () => { ) }) - it('wraps a failed collaborator check', async () => { - vi.mocked(auth.checkCollaborator).mockRejectedValueOnce(new Error('auth exploded')) - - await expect(retitle(comment('/retitle new title'))).rejects.toThrow( - /could not check Commentor auth: Error: auth exploded/, - ) - }) - it('wraps a failed title update', async () => { vi.mocked(auth.checkCollaborator).mockResolvedValueOnce(true) failing('patch', issue) @@ -237,14 +180,6 @@ describe('/lock error paths', () => { await expect(lock(commentWithoutIssue('/lock'))).rejects.toThrow(/missing issue number/) }) - it('wraps a failed collaborator check', async () => { - vi.mocked(auth.checkCollaborator).mockRejectedValueOnce(new Error('auth exploded')) - - await expect(lock(comment('/lock'))).rejects.toThrow( - /could not check commenter auth: Error: auth exploded/, - ) - }) - it('wraps a failed lock', async () => { vi.mocked(auth.checkCollaborator).mockResolvedValueOnce(true) failing('put', `${issue}/lock`) diff --git a/__tests__/label/errorPaths.test.ts b/__tests__/label/errorPaths.test.ts index 91a7b9e..8627874 100644 --- a/__tests__/label/errorPaths.test.ts +++ b/__tests__/label/errorPaths.test.ts @@ -14,8 +14,8 @@ import issueCommentEvent from '../fixtures/issues/issueCommentEvent.json' import labelFileContents from '../fixtures/labels/labelFileContentsResp.json' import * as utils from '../testUtils' -// checkCollaborator swallows HTTP failures itself, so /remove's own -// "could not check commenter auth" branch is only reachable when it rejects +// checkCollaborator is stubbed so /remove's own error paths can be +// reached without routing the collaborator probe vi.mock('../../src/utils/auth', { spy: true }) const server = setupServer() @@ -63,14 +63,6 @@ describe('/remove error paths', () => { ) }) - it('wraps a failed commenter authorization check', async () => { - vi.mocked(auth.checkCollaborator).mockRejectedValueOnce(new Error('auth exploded')) - - await expect(remove(comment('/remove some-label'))).rejects.toThrow( - /could not check commenter auth: Error: auth exploded/, - ) - }) - it('wraps a failed label lookup', async () => { vi.mocked(auth.checkCollaborator).mockResolvedValueOnce(true) issueLookupFails() diff --git a/__tests__/utils/auth.test.ts b/__tests__/utils/auth.test.ts index 82d60fe..3f5dfc1 100644 --- a/__tests__/utils/auth.test.ts +++ b/__tests__/utils/auth.test.ts @@ -253,44 +253,6 @@ describe('checkCommenterAuth', () => { await expect(checkCommenterAuth(octokit, context, 1, 'some-user')).resolves.toBe(true) }) - - // Each helper swallows API errors and logs them, so the only way an error - // escapes into checkCommenterAuth's own catch blocks is the logger throwing. - it('wraps an error escaping the org membership check', async () => { - vi.spyOn(core, 'warning').mockImplementation(() => { - throw new Error('logger down') - }) - vi.spyOn(octokit.orgs, 'checkMembershipForUser').mockRejectedValueOnce({ status: 500 }) - - await expect(checkCommenterAuth(octokit, context, 1, 'some-user')).rejects.toThrow( - 'error in checking org member: Error: logger down', - ) - }) - - it('wraps an error escaping the collaborator check', async () => { - vi.spyOn(core, 'warning').mockImplementation(() => { - throw new Error('logger down') - }) - vi.spyOn(octokit.orgs, 'checkMembershipForUser').mockRejectedValueOnce({ status: 404 }) - vi.spyOn(octokit.repos, 'checkCollaborator').mockRejectedValueOnce({ status: 500 }) - - await expect(checkCommenterAuth(octokit, context, 1, 'some-user')).rejects.toThrow( - 'could not check collaborator: Error: logger down', - ) - }) - - it('wraps an error escaping the issue comments check', async () => { - vi.spyOn(core, 'warning').mockImplementation(() => { - throw new Error('logger down') - }) - vi.spyOn(octokit.orgs, 'checkMembershipForUser').mockRejectedValueOnce({ status: 404 }) - vi.spyOn(octokit.repos, 'checkCollaborator').mockRejectedValueOnce({ status: 404 }) - vi.spyOn(octokit.issues, 'listComments').mockRejectedValueOnce({ status: 500 }) - - await expect(checkCommenterAuth(octokit, context, 1, 'some-user')).rejects.toThrow( - 'could not check issue comments: Error: logger down', - ) - }) }) describe('getOrgCollabCommentUsers', () => { @@ -322,17 +284,6 @@ describe('getOrgCollabCommentUsers', () => { getOrgCollabCommentUsers(octokit, context, 1, ['some-user', 'nobody']), ).resolves.toEqual(['some-user']) }) - - it('wraps an error escaping any of the checks', async () => { - vi.spyOn(core, 'warning').mockImplementation(() => { - throw new Error('logger down') - }) - vi.spyOn(octokit.orgs, 'checkMembershipForUser').mockRejectedValueOnce({ status: 500 }) - - await expect( - getOrgCollabCommentUsers(octokit, context, 1, ['some-user']), - ).rejects.toThrow('could not get authorized user: Error: logger down') - }) }) describe('assertAuthorizedByOwnersOrMembership', () => { diff --git a/dist/index.js b/dist/index.js index a087c24..8d73177 100644 --- a/dist/index.js +++ b/dist/index.js @@ -42275,19 +42275,11 @@ async function checkIssueComments(octokit, context, issueNum, user) { */ async function getOrgCollabCommentUsers(octokit, context, issueNum, args) { const toReturn = []; - try { - await Promise.all(args.map(async (arg) => { - const isOrgMember = await checkOrgMember(octokit, context, arg); - const isCollaborator = await checkCollaborator(octokit, context, arg); - const hasCommented = await checkIssueComments(octokit, context, issueNum, arg); - if (isOrgMember || isCollaborator || hasCommented) { - toReturn.push(arg); - } - })); - } - catch (e) { - throw new Error(`could not get authorized user: ${e}`); - } + await Promise.all(args.map(async (arg) => { + if (await checkCommenterAuth(octokit, context, issueNum, arg)) { + toReturn.push(arg); + } + })); return toReturn; } /** @@ -42297,34 +42289,13 @@ async function getOrgCollabCommentUsers(octokit, context, issueNum, args) { * @param octokit - a hydrated github client * @param context - the github actions event context * @param issueNum - the issue or pr number this runtime is associated with - * @param args - the users to check auth on + * @param user - the user to check auth on */ async function checkCommenterAuth(octokit, context, issueNum, user) { - let isOrgMember = false; - let isCollaborator = false; - let hasCommented = false; - try { - isOrgMember = await checkOrgMember(octokit, context, user); - } - catch (e) { - throw new Error(`error in checking org member: ${e}`); - } - try { - isCollaborator = await checkCollaborator(octokit, context, user); - } - catch (e) { - throw new Error(`could not check collaborator: ${e}`); - } - try { - hasCommented = await checkIssueComments(octokit, context, issueNum, user); - } - catch (e) { - throw new Error(`could not check issue comments: ${e}`); - } - if (isOrgMember || isCollaborator || hasCommented) { - return true; - } - return false; + const isOrgMember = await checkOrgMember(octokit, context, user); + const isCollaborator = await checkCollaborator(octokit, context, user); + const hasCommented = await checkIssueComments(octokit, context, issueNum, user); + return isOrgMember || isCollaborator || hasCommented; } /** * When the repository has OWNERS files, use them to authorize the action, @@ -45619,13 +45590,7 @@ async function remove(context = github_context) { } // Only users who: // - are collaborators - let isAuthUser = false; - try { - isAuthUser = await checkCollaborator(octokit, context, commenterId); - } - catch (e) { - throw new Error(`could not check commenter auth: ${e}`); - } + const isAuthUser = await checkCollaborator(octokit, context, commenterId); if (!isAuthUser) { throw new Error(`commenter is not authorized to remove a label. Must be repo collaborator`); } @@ -45823,13 +45788,7 @@ async function assign_assign(context = github_context) { // - are members of the org // - are collaborators // - have previously commented on this issue - let authUsers = []; - try { - authUsers = await getOrgCollabCommentUsers(octokit, context, issueNumber, commentArgs); - } - catch (e) { - throw new Error(`could not get authorized users: ${e}`); - } + const authUsers = await getOrgCollabCommentUsers(octokit, context, issueNumber, commentArgs); switch (authUsers.length) { case 0: throw new Error(`no authorized users found. Only users who are members of the org, are collaborators, or have previously commented on this issue may be assigned`); @@ -45902,13 +45861,7 @@ async function cc(context = github_context) { // - are members of the org // - are collaborators // - have previously commented on this issue - let authUsers = []; - try { - authUsers = await getOrgCollabCommentUsers(octokit, context, pullNumber, commentArgs); - } - catch (e) { - throw new Error(`could not get authorized users: ${e}`); - } + const authUsers = await getOrgCollabCommentUsers(octokit, context, pullNumber, commentArgs); switch (authUsers.length) { case 0: throw new Error(`no authorized users found. Only users who are members of the org, are collaborators, or have previously commented on this issue may be cc'd`); @@ -45971,18 +45924,9 @@ async function close_close(context = github_context) { // - are collaborators // - pass `authorization.close`, which is read only when the first two refuse const isAuthor = commenterId === context.payload.issue?.user?.login; - let isAuthUser = isAuthor; - if (!isAuthor) { - try { - isAuthUser = await checkCollaborator(octokit, context, commenterId); - } - catch (e) { - throw new Error(`could not check commentor auth: ${e}`); - } - } - if (!isAuthUser) { - isAuthUser = await closePolicyAllows(octokit, context, commenterId); - } + const isAuthUser = isAuthor + || (await checkCollaborator(octokit, context, commenterId)) + || (await closePolicyAllows(octokit, context, commenterId)); if (isAuthUser) { const notPlanned = hasKeyword(getCommandArgs('/close', commentBody), 'not-planned'); try { @@ -46038,13 +45982,7 @@ async function lock(context = github_context) { const commentArgs = getCommandArgs('/lock', commentBody); // Only users who: // - are collaborators - let isAuthUser = false; - try { - isAuthUser = await checkCollaborator(octokit, context, commenterId); - } - catch (e) { - throw new Error(`could not check commenter auth: ${e}`); - } + const isAuthUser = await checkCollaborator(octokit, context, commenterId); if (isAuthUser) { let lockReason; if (commentArgs.length > 0) { @@ -46226,13 +46164,7 @@ async function milestone(context = github_context) { } // Only users who: // - are collaborators - let isAuthUser = false; - try { - isAuthUser = await checkCollaborator(octokit, context, commenterId); - } - catch (e) { - throw new Error(`could not check commenter auth: ${e}`); - } + const isAuthUser = await checkCollaborator(octokit, context, commenterId); if (!isAuthUser) { throw new Error(`commenter is not authorized to set a milestone. Must be repo collaborator`); } @@ -46287,18 +46219,9 @@ async function reopen(context = github_context) { // - are collaborators // - pass `authorization.close`, which is read only when the first two refuse const isAuthor = commenterId === context.payload.issue?.user?.login; - let isAuthUser = isAuthor; - if (!isAuthor) { - try { - isAuthUser = await checkCollaborator(octokit, context, commenterId); - } - catch (e) { - throw new Error(`could not check commentor auth: ${e}`); - } - } - if (!isAuthUser) { - isAuthUser = await closePolicyAllows(octokit, context, commenterId); - } + const isAuthUser = isAuthor + || (await checkCollaborator(octokit, context, commenterId)) + || (await closePolicyAllows(octokit, context, commenterId)); if (isAuthUser) { try { await octokit.issues.update({ @@ -46341,13 +46264,7 @@ async function retitle(context = github_context) { } // Only users who: // - are collaborators - let isAuthUser = false; - try { - isAuthUser = await checkCollaborator(octokit, context, commenterId); - } - catch (e) { - throw new Error(`could not check Commentor auth: ${e}`); - } + const isAuthUser = await checkCollaborator(octokit, context, commenterId); if (isAuthUser) { try { await octokit.issues.update({ @@ -46397,13 +46314,7 @@ async function unassign(context = github_context) { } return; } - let isAuthUser = false; - try { - isAuthUser = await checkCommenterAuth(octokit, context, issueNumber, commenterId); - } - catch (e) { - throw new Error(`couldn ot check commentor Auth: ${e}`); - } + const isAuthUser = await checkCommenterAuth(octokit, context, issueNumber, commenterId); if (isAuthUser) { try { await octokit.issues.removeAssignees({ @@ -46453,13 +46364,7 @@ async function uncc(context = github_context) { // - are members of the org // - are collaborators // - have previously commented on this issue - let authUser = false; - try { - authUser = await checkCommenterAuth(octokit, context, pullNumber, commenterId); - } - catch (e) { - throw new Error(`could not get authorized users: ${e}`); - } + const authUser = await checkCommenterAuth(octokit, context, pullNumber, commenterId); if (authUser) { await octokit.pulls.removeRequestedReviewers({ ...context.repo, diff --git a/src/issueComment/assign.ts b/src/issueComment/assign.ts index 9ee0f17..c65e614 100644 --- a/src/issueComment/assign.ts +++ b/src/issueComment/assign.ts @@ -48,18 +48,12 @@ export async function assign(context: Context = github.context): Promise { // - are members of the org // - are collaborators // - have previously commented on this issue - let authUsers: string[] = [] - try { - authUsers = await getOrgCollabCommentUsers( - octokit, - context, - issueNumber, - commentArgs, - ) - } - catch (e) { - throw new Error(`could not get authorized users: ${e}`) - } + const authUsers = await getOrgCollabCommentUsers( + octokit, + context, + issueNumber, + commentArgs, + ) switch (authUsers.length) { case 0: diff --git a/src/issueComment/cc.ts b/src/issueComment/cc.ts index e03b1d3..6d13590 100644 --- a/src/issueComment/cc.ts +++ b/src/issueComment/cc.ts @@ -45,18 +45,12 @@ export async function cc(context: Context = github.context): Promise { // - are members of the org // - are collaborators // - have previously commented on this issue - let authUsers: string[] = [] - try { - authUsers = await getOrgCollabCommentUsers( - octokit, - context, - pullNumber, - commentArgs, - ) - } - catch (e) { - throw new Error(`could not get authorized users: ${e}`) - } + const authUsers = await getOrgCollabCommentUsers( + octokit, + context, + pullNumber, + commentArgs, + ) switch (authUsers.length) { case 0: diff --git a/src/issueComment/close.ts b/src/issueComment/close.ts index cb0553a..6566df9 100644 --- a/src/issueComment/close.ts +++ b/src/issueComment/close.ts @@ -31,18 +31,10 @@ export async function close(context: Context = github.context): Promise { // - are collaborators // - pass `authorization.close`, which is read only when the first two refuse const isAuthor = commenterId === context.payload.issue?.user?.login - let isAuthUser: boolean = isAuthor - if (!isAuthor) { - try { - isAuthUser = await checkCollaborator(octokit, context, commenterId) - } - catch (e) { - throw new Error(`could not check commentor auth: ${e}`) - } - } - if (!isAuthUser) { - isAuthUser = await closePolicyAllows(octokit, context, commenterId) - } + const isAuthUser + = isAuthor + || (await checkCollaborator(octokit, context, commenterId)) + || (await closePolicyAllows(octokit, context, commenterId)) if (isAuthUser) { const notPlanned = hasKeyword(getCommandArgs('/close', commentBody), 'not-planned') diff --git a/src/issueComment/lock.ts b/src/issueComment/lock.ts index 76f853f..53243ef 100644 --- a/src/issueComment/lock.ts +++ b/src/issueComment/lock.ts @@ -39,13 +39,7 @@ export async function lock(context: Context = github.context): Promise { // Only users who: // - are collaborators - let isAuthUser: boolean = false - try { - isAuthUser = await checkCollaborator(octokit, context, commenterId) - } - catch (e) { - throw new Error(`could not check commenter auth: ${e}`) - } + const isAuthUser = await checkCollaborator(octokit, context, commenterId) if (isAuthUser) { let lockReason: LockReason | undefined diff --git a/src/issueComment/milestone.ts b/src/issueComment/milestone.ts index 13dcfdc..1cbe3f6 100644 --- a/src/issueComment/milestone.ts +++ b/src/issueComment/milestone.ts @@ -32,13 +32,7 @@ export async function milestone( // Only users who: // - are collaborators - let isAuthUser: boolean = false - try { - isAuthUser = await checkCollaborator(octokit, context, commenterId) - } - catch (e) { - throw new Error(`could not check commenter auth: ${e}`) - } + const isAuthUser = await checkCollaborator(octokit, context, commenterId) if (!isAuthUser) { throw new Error( diff --git a/src/issueComment/reopen.ts b/src/issueComment/reopen.ts index d22fa6e..441a765 100644 --- a/src/issueComment/reopen.ts +++ b/src/issueComment/reopen.ts @@ -28,18 +28,10 @@ export async function reopen(context: Context = github.context): Promise { // - are collaborators // - pass `authorization.close`, which is read only when the first two refuse const isAuthor = commenterId === context.payload.issue?.user?.login - let isAuthUser: boolean = isAuthor - if (!isAuthor) { - try { - isAuthUser = await checkCollaborator(octokit, context, commenterId) - } - catch (e) { - throw new Error(`could not check commentor auth: ${e}`) - } - } - if (!isAuthUser) { - isAuthUser = await closePolicyAllows(octokit, context, commenterId) - } + const isAuthUser + = isAuthor + || (await checkCollaborator(octokit, context, commenterId)) + || (await closePolicyAllows(octokit, context, commenterId)) if (isAuthUser) { try { diff --git a/src/issueComment/retitle.ts b/src/issueComment/retitle.ts index 9e1203e..b115efa 100644 --- a/src/issueComment/retitle.ts +++ b/src/issueComment/retitle.ts @@ -37,13 +37,7 @@ export async function retitle( // Only users who: // - are collaborators - let isAuthUser: boolean = false - try { - isAuthUser = await checkCollaborator(octokit, context, commenterId) - } - catch (e) { - throw new Error(`could not check Commentor auth: ${e}`) - } + const isAuthUser = await checkCollaborator(octokit, context, commenterId) if (isAuthUser) { try { diff --git a/src/issueComment/unassign.ts b/src/issueComment/unassign.ts index e6ffa08..3e75d97 100644 --- a/src/issueComment/unassign.ts +++ b/src/issueComment/unassign.ts @@ -45,18 +45,12 @@ export async function unassign( return } - let isAuthUser: boolean = false - try { - isAuthUser = await checkCommenterAuth( - octokit, - context, - issueNumber, - commenterId, - ) - } - catch (e) { - throw new Error(`couldn ot check commentor Auth: ${e}`) - } + const isAuthUser = await checkCommenterAuth( + octokit, + context, + issueNumber, + commenterId, + ) if (isAuthUser) { try { diff --git a/src/issueComment/uncc.ts b/src/issueComment/uncc.ts index 6af0b12..fa678dc 100644 --- a/src/issueComment/uncc.ts +++ b/src/issueComment/uncc.ts @@ -44,18 +44,12 @@ export async function uncc(context: Context = github.context): Promise { // - are members of the org // - are collaborators // - have previously commented on this issue - let authUser: boolean = false - try { - authUser = await checkCommenterAuth( - octokit, - context, - pullNumber, - commenterId, - ) - } - catch (e) { - throw new Error(`could not get authorized users: ${e}`) - } + const authUser = await checkCommenterAuth( + octokit, + context, + pullNumber, + commenterId, + ) if (authUser) { await octokit.pulls.removeRequestedReviewers({ diff --git a/src/labels/remove.ts b/src/labels/remove.ts index 91effa7..53abdb4 100644 --- a/src/labels/remove.ts +++ b/src/labels/remove.ts @@ -29,13 +29,7 @@ export async function remove(context: Context = github.context): Promise { // Only users who: // - are collaborators - let isAuthUser: boolean = false - try { - isAuthUser = await checkCollaborator(octokit, context, commenterId) - } - catch (e) { - throw new Error(`could not check commenter auth: ${e}`) - } + const isAuthUser = await checkCollaborator(octokit, context, commenterId) if (!isAuthUser) { throw new Error( diff --git a/src/utils/auth.ts b/src/utils/auth.ts index 6e887fa..db99313 100644 --- a/src/utils/auth.ts +++ b/src/utils/auth.ts @@ -159,27 +159,13 @@ export async function getOrgCollabCommentUsers( ): Promise { const toReturn: string[] = [] - try { - await Promise.all( - args.map(async (arg) => { - const isOrgMember = await checkOrgMember(octokit, context, arg) - const isCollaborator = await checkCollaborator(octokit, context, arg) - const hasCommented = await checkIssueComments( - octokit, - context, - issueNum, - arg, - ) - - if (isOrgMember || isCollaborator || hasCommented) { - toReturn.push(arg) - } - }), - ) - } - catch (e) { - throw new Error(`could not get authorized user: ${e}`) - } + await Promise.all( + args.map(async (arg) => { + if (await checkCommenterAuth(octokit, context, issueNum, arg)) { + toReturn.push(arg) + } + }), + ) return toReturn } @@ -191,7 +177,7 @@ export async function getOrgCollabCommentUsers( * @param octokit - a hydrated github client * @param context - the github actions event context * @param issueNum - the issue or pr number this runtime is associated with - * @param args - the users to check auth on + * @param user - the user to check auth on */ export async function checkCommenterAuth( octokit: Octokit, @@ -199,36 +185,11 @@ export async function checkCommenterAuth( issueNum: number, user: string, ): Promise { - let isOrgMember: boolean = false - let isCollaborator: boolean = false - let hasCommented: boolean = false - - try { - isOrgMember = await checkOrgMember(octokit, context, user) - } - catch (e) { - throw new Error(`error in checking org member: ${e}`) - } - - try { - isCollaborator = await checkCollaborator(octokit, context, user) - } - catch (e) { - throw new Error(`could not check collaborator: ${e}`) - } - - try { - hasCommented = await checkIssueComments(octokit, context, issueNum, user) - } - catch (e) { - throw new Error(`could not check issue comments: ${e}`) - } - - if (isOrgMember || isCollaborator || hasCommented) { - return true - } + const isOrgMember = await checkOrgMember(octokit, context, user) + const isCollaborator = await checkCollaborator(octokit, context, user) + const hasCommented = await checkIssueComments(octokit, context, issueNum, user) - return false + return isOrgMember || isCollaborator || hasCommented } /**