fix: Catch errors when adding a github comment and write to the step summary instead (#15)

Adding a GitHub comment might fail due to invalid rights configured in the workflow / a bot user / ...
We should not fail the step but log an error and fall back to writing to the step summary.

Author: OleRoss

README.md

@@ -3,6 +3,7 @@
 This is a github action which is supposed to be run for a PR and visualizes the test coverage.
 
 Everytime this action is run, we add a comment to the PR showing the current coverage.
+If GitHub does not allow creating or updating a PR comment, the action falls back to the step summary instead.
 
 If the trigger is not `pull_request`, this action does nothing. Also, if the length of the comment would exceed the GitHub limit, lines are trunctuated.
 
@@ -20,6 +21,7 @@ on: pull_request
 
 permissions:
   contents: read
+  issues: write
   pull-requests: write
 
 jobs:

src/action.ts

@@ -20,6 +20,19 @@ const actionLogger: Logger = {
   debug: (message) => core.debug(message),
 }
 
+const formatCommentError = (error: unknown): string => {
+  const message = error instanceof Error ? error.message : String(error)
+
+  if (typeof error === 'object' && error !== null && 'status' in error) {
+    const status = error.status
+    if (typeof status === 'number' || typeof status === 'string') {
+      return `status ${status}: ${message}`
+    }
+  }
+
+  return message
+}
+
 export const run = async (inputs: Inputs, octokit: Octokit, context: Context): Promise<void> => {
   const shas = getComparisonShas(context)
 
@@ -67,13 +80,22 @@ export const run = async (inputs: Inputs, octokit: Octokit, context: Context): P
   }
 
   // Post or update comment
-  const { url, updated } = await postComment(octokit, context, pullNumber, markdown, inputs.updateComment)
+  try {
+    const { url, updated } = await postComment(octokit, context, pullNumber, markdown, inputs.updateComment)
 
-  if (updated) {
-    core.info(`Updated existing comment: ${url}`)
-  } else {
-    core.info(`Created new comment: ${url}`)
-  }
+    if (updated) {
+      core.info(`Updated existing comment: ${url}`)
+    } else {
+      core.info(`Created new comment: ${url}`)
+    }
 
-  core.info('Coverage visualization posted successfully')
+    core.info('Coverage visualization posted successfully')
+  } catch (error) {
+    core.warning(
+      `Failed to post or update PR comment (${formatCommentError(error)}). Falling back to the step summary instead.`,
+    )
+    await core.summary.addRaw(markdown).write()
+    core.info('Coverage visualization written to step summary')
+    return
+  }
 }

tests/action.test.ts

@@ -0,0 +1,121 @@
+import type { Octokit } from '@octokit/action'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { type Inputs, run } from '../src/action.js'
+import type { Context } from '../src/github.js'
+
+const mocks = vi.hoisted(() => ({
+  processCoverage: vi.fn(),
+  findPullRequestNumber: vi.fn(),
+  getComparisonShas: vi.fn(),
+  postComment: vi.fn(),
+  info: vi.fn(),
+  warning: vi.fn(),
+  debug: vi.fn(),
+  setOutput: vi.fn(),
+  summaryAddRaw: vi.fn(),
+  summaryWrite: vi.fn(),
+}))
+
+vi.mock('@actions/core', () => ({
+  info: mocks.info,
+  warning: mocks.warning,
+  debug: mocks.debug,
+  setOutput: mocks.setOutput,
+  summary: {
+    addRaw: mocks.summaryAddRaw,
+  },
+}))
+
+vi.mock('../src/core/index.js', () => ({
+  processCoverage: mocks.processCoverage,
+}))
+
+vi.mock('../src/github.js', () => ({
+  findPullRequestNumber: mocks.findPullRequestNumber,
+  getComparisonShas: mocks.getComparisonShas,
+  postComment: mocks.postComment,
+}))
+
+const inputs: Inputs = {
+  files: 'coverage.xml',
+  updateComment: true,
+  showChangedLinesOnly: true,
+  excludeFilesPattern: '',
+  sourceDir: '/repo',
+}
+
+const context = {
+  repo: {
+    owner: 'owner',
+    repo: 'repo',
+  },
+  sha: 'head-sha',
+  payload: {
+    pull_request: {
+      number: 123,
+      state: 'open',
+    },
+  },
+} as unknown as Context
+
+const octokit = {} as Octokit
+
+describe('run', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mocks.summaryWrite.mockResolvedValue(undefined)
+    mocks.summaryAddRaw.mockReturnValue({ write: mocks.summaryWrite })
+    mocks.getComparisonShas.mockReturnValue({ baseSha: 'base-sha', headSha: 'head-sha' })
+    mocks.processCoverage.mockResolvedValue({
+      markdown: 'coverage markdown',
+      lineCoverage: 0.75,
+      branchCoverage: 0.5,
+      lineCoveragePr: 0.8,
+      branchCoveragePr: undefined,
+    })
+  })
+
+  it('posts a PR comment when comment posting succeeds', async () => {
+    mocks.findPullRequestNumber.mockResolvedValue(123)
+    mocks.postComment.mockResolvedValue({
+      url: 'https://github.com/owner/repo/pull/123#issuecomment-1',
+      updated: false,
+    })
+
+    await run(inputs, octokit, context)
+
+    expect(mocks.postComment).toHaveBeenCalledWith(octokit, context, 123, 'coverage markdown', true)
+    expect(mocks.summaryAddRaw).not.toHaveBeenCalled()
+    expect(mocks.warning).not.toHaveBeenCalled()
+    expect(mocks.info).toHaveBeenCalledWith(
+      'Created new comment: https://github.com/owner/repo/pull/123#issuecomment-1',
+    )
+    expect(mocks.info).toHaveBeenCalledWith('Coverage visualization posted successfully')
+  })
+
+  it('falls back to the step summary when comment posting fails', async () => {
+    const error = Object.assign(new Error('Resource not accessible by integration'), { status: 403 })
+    mocks.findPullRequestNumber.mockResolvedValue(123)
+    mocks.postComment.mockRejectedValue(error)
+
+    await expect(run(inputs, octokit, context)).resolves.toBeUndefined()
+
+    expect(mocks.warning).toHaveBeenCalledWith(
+      'Failed to post or update PR comment (status 403: Resource not accessible by integration). Falling back to the step summary instead.',
+    )
+    expect(mocks.summaryAddRaw).toHaveBeenCalledWith('coverage markdown')
+    expect(mocks.summaryWrite).toHaveBeenCalledTimes(1)
+    expect(mocks.info).toHaveBeenCalledWith('Coverage visualization written to step summary')
+  })
+
+  it('writes to the step summary when no PR is found', async () => {
+    mocks.findPullRequestNumber.mockResolvedValue(null)
+
+    await run(inputs, octokit, context)
+
+    expect(mocks.postComment).not.toHaveBeenCalled()
+    expect(mocks.summaryAddRaw).toHaveBeenCalledWith('coverage markdown')
+    expect(mocks.summaryWrite).toHaveBeenCalledTimes(1)
+    expect(mocks.info).toHaveBeenCalledWith('No pull request found for this commit, writing to step summary instead')
+  })
+})