diff --git a/src/__snapshots__/markdown-renderer.spec.ts.snap b/src/__snapshots__/markdown-renderer.spec.ts.snap index 550e411..3e27377 100644 --- a/src/__snapshots__/markdown-renderer.spec.ts.snap +++ b/src/__snapshots__/markdown-renderer.spec.ts.snap @@ -54,6 +54,12 @@ exports[`MarkdownRenderer > renderContributionList > renders a list of contribut * [#42](http://github.com/42) My cool PR ([@hzoo](http://hzoo.com))" `; +exports[`MarkdownRenderer > renderContributorList > renders a list mixing a fallback (unfetchable) contributor with a full contributor 1`] = ` +"#### Committers: 2 +- @deleted-user +- Tobias Bieniek (@Turbo87)" +`; + exports[`MarkdownRenderer > renderContributorList > renders a list of GitHub users 1`] = ` "#### Committers: 2 - Tobias Bieniek ([@Turbo87](https://github.com/Turbo87)) diff --git a/src/changelog.spec.js b/src/changelog.spec.js index d0b1fdd..ec13a5d 100644 --- a/src/changelog.spec.js +++ b/src/changelog.spec.js @@ -278,5 +278,196 @@ describe('Changelog', () => { }, ]); }); + it('falls back gracefully when getUserData throws for a bot account (403)', async () => { + fetch.__setMockResponses({ + 'https://api.github.com/users/test-user-1': { + body: { + login: 'test-user-1', + html_url: 'https://github.com/test-user-1', + name: 'Test User 1', + }, + }, + + // Simulate 403 - token lacks scope to fetch this bot's profile + 'https://api.github.com/users/github-actions': { + status: 403, + statusText: 'Forbidden', + ok: false, + body: { + message: 'Resource not accessible by integration', + }, + }, + }); + + const changelog = new Changelog({ ignoreCommitters: [] }); + + const testCommits = [ + { + commitSHA: 'a0000001', + githubIssue: { + user: { + login: 'test-user-1', + html_url: 'https://github.com/test-user-1', + }, + }, + }, + { + commitSHA: 'a0000002', + githubIssue: { + user: { + login: 'github-actions', + html_url: 'https://github.com/apps/github-actions', + }, + }, + }, + ]; + + const committers = await changelog.getCommitters(testCommits); + + expect(committers).toHaveLength(2); + + expect(committers[0]).toEqual({ + login: 'test-user-1', + html_url: 'https://github.com/test-user-1', + name: 'Test User 1', + }); + + // Falls back to login-only — no name, no type, html_url preserved from PR data + expect(committers[1]).toEqual({ + login: 'github-actions', + html_url: 'https://github.com/apps/github-actions', + }); + }); + it('ignoreCommitters still works when mixed with failing getUserData', async () => { + fetch.__setMockResponses({ + 'https://api.github.com/users/real-user': { + body: { + login: 'real-user', + html_url: 'https://github.com/real-user', + name: 'Real User', + }, + }, + }); + + // ignored-bot is in ignoreCommitters: getUserData should never be called for it + const changelog = new Changelog({ + ignoreCommitters: ['ignored-bot'], + }); + + const testCommits = [ + { + commitSHA: 'a0000001', + githubIssue: { + user: { + login: 'real-user', + html_url: 'https://github.com/real-user', + }, + }, + }, + { + commitSHA: 'a0000002', + githubIssue: { + user: { + login: 'ignored-bot', + html_url: 'https://github.com/ignored-bot', + }, + }, + }, + ]; + + const committers = await changelog.getCommitters(testCommits); + + // ignored-bot skipped before getUserData is even attempted + expect(committers).toHaveLength(1); + + expect(committers[0]).toEqual({ + login: 'real-user', + html_url: 'https://github.com/real-user', + name: 'Real User', + }); + }); + it('falls back with empty html_url when PR user data has none', async () => { + fetch.__setMockResponses({}); + + // ignored-bot is in ignoreCommitters: getUserData should never be called for it + const changelog = new Changelog({ + ignoreCommitters: [], + }); + + const testCommits = [ + { + commitSHA: 'a0000001', + githubIssue: { + user: { + login: 'restricted-bot', + }, + }, + }, + ]; + + const committers = await changelog.getCommitters(testCommits); + + // ignored-bot skipped before getUserData is even attempted + expect(committers).toHaveLength(1); + + expect(committers[0]).toEqual({ + login: 'restricted-bot', + html_url: '', + }); + }); + it('falls back with gracefully when getUserData throws for a deleted user', async () => { + fetch.__setMockResponses({ + 'https://api.github.com/users/test-user1': { + body: { + login: 'test-user1', + html_url: 'https://github.com/test-user1', + name: 'Test User 1', + }, + }, + // Simulate 404 - account was deleted aftr the pr was merged + 'https://api.github.com/users/deleted-user': { + status: 404, + statusText: 'Not Found', + ok: false, + body: { message: 'Not Found' }, + }, + }); + const changelog = new Changelog({ ignoreCommitters: [] }); + const testCommits = [ + { + commitSHA: 'a0000001', + githubIssue: { + user: { + login: 'test-user1', + html_url: 'https://github.com/test-user1', + }, + }, + }, + { + commitSHA: 'a0000002', + githubIssue: { + user: { + login: 'deleted-user', + html_url: 'https://github.com/deleted-user', + }, + }, + }, + ]; + + const committers = await changelog.getCommitters(testCommits); + + expect(committers).toHaveLength(2); + + expect(committers[0]).toEqual({ + login: 'test-user1', + html_url: 'https://github.com/test-user1', + name: 'Test User 1', + }); + // Deleted account - falls back to login-only, html-url from PR data, no name + expect(committers[1]).toEqual({ + login: 'deleted-user', + html_url: 'https://github.com/deleted-user', + }); + }); }); }); diff --git a/src/changelog.ts b/src/changelog.ts index f867ea1..e58aee1 100644 --- a/src/changelog.ts +++ b/src/changelog.ts @@ -136,7 +136,17 @@ export default class Changelog { const shouldKeepCommiter = login && !this.ignoreCommitter(login); if (login && shouldKeepCommiter && !committers[login]) { - committers[login] = this.sanitizeCommitter(await this.github.getUserData(user)); + try { + committers[login] = this.sanitizeCommitter(await this.github.getUserData(user)); + } catch { + // getUserData can fail for various reasons (e.g. 403 for restricted bot accounts or 404 for deleted accounts). + // Fall back to a minimal entry using just the login + // so the changelog still generates rather than failing completely. + committers[login] = { + login, + html_url: user.html_url || '', + } as GitHubContributor; + } } } diff --git a/src/functional/__snapshots__/markdown-unfetchable-comitter.spec.js.snap b/src/functional/__snapshots__/markdown-unfetchable-comitter.spec.js.snap new file mode 100644 index 0000000..c79c621 --- /dev/null +++ b/src/functional/__snapshots__/markdown-unfetchable-comitter.spec.js.snap @@ -0,0 +1,13 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`createMarkdown with an unfetchable committer > still generates a readable changelog when a bot committer is unfetchable 1`] = ` +" +## Unreleased (2099-01-01) + +#### :house: Maintenance +* [#2](https://github.com/embroider-build/github-changelog/pull/2) chore: bump dependencies ([@dependabot[bot]](https://github.com/apps/dependabot)) + +#### Committers: 2 +- Real User ([@real-user](https://github.com/real-user)) +- [@dependabot[bot]](https://github.com/apps/dependabot)" +`; diff --git a/src/functional/markdown-unfetchable-comitter.spec.js b/src/functional/markdown-unfetchable-comitter.spec.js new file mode 100644 index 0000000..0490069 --- /dev/null +++ b/src/functional/markdown-unfetchable-comitter.spec.js @@ -0,0 +1,71 @@ +import { vi, describe, beforeEach, afterEach, it, expect } from 'vitest'; + +import * as git from '../git'; +import Changelog from '../changelog'; +import * as fetch from '../fetch'; + +vi.mock('../../src/progress-bar'); +vi.mock('../changelog'); +vi.mock('../../src/github-api'); +vi.mock('../git'); +vi.mock('../fetch'); + +describe('createMarkdown with an unfetchable committer', () => { + beforeEach(() => { + fetch.__resetMockResponses(); + + git.changedPaths.mockImplementation(() => []); + git.lastTag.mockImplementation(() => 'v1.0.0'); + git.listTagNames.mockImplementation(() => ['v1.0.0']); + git.listCommits.mockImplementation(() => [ + { + sha: 'a0000002', + refName: '', + summary: 'Merge pull request #2 from bump-deps', + date: '2024-01-02', + }, + { + sha: 'a0000001', + refName: '', + summary: 'Merge pull request #1 from feature', + date: '2024-01-01', + }, + ]); + + fetch.__setMockResponses({ + 'https://api.github.com/repos/embroider-build/github-changelog/issues/1': { + body: { + number: 1, + title: 'feat: add new feature', + labels: [{ name: 'New Feature' }], + pull_request: { html_url: 'https://github.com/embroider-build/github-changelog/pull/1' }, + user: { login: 'real-user', html_url: 'https://github.com/real-user' }, + }, + }, + 'https://api.github.com/repos/embroider-build/github-changelog/issues/2': { + body: { + number: 2, + title: 'chore: bump dependencies', + labels: [{ name: 'Type: Maintenance' }], + pull_request: { html_url: 'https://github.com/embroider-build/github-changelog/pull/2' }, + user: { login: 'dependabot[bot]', html_url: 'https://github.com/apps/dependabot' }, + }, + }, + 'https://api.github.com/users/real-user': { + body: { login: 'real-user', type: 'User', html_url: 'https://github.com/real-user', name: 'Real User' }, + }, + }); + }); + + afterEach(() => { + vi.resetAllMocks(); + }); + + it('still generates a readable changelog when a bot committer is unfetchable', async () => { + const changelog = new Changelog({ ignoreCommitters: [] }); + + const markdown = await changelog.createMarkdown(); + + expect(markdown).toMatchSnapshot(); + }); +}); diff --git a/src/markdown-renderer.spec.ts b/src/markdown-renderer.spec.ts index 71bbf00..00223e6 100644 --- a/src/markdown-renderer.spec.ts +++ b/src/markdown-renderer.spec.ts @@ -1,4 +1,4 @@ -import type { GithubAppInfo } from './github-api.js'; +import type { GithubAppInfo, GitHubContributor } from './github-api.js'; import { CommitInfo, Release } from './interfaces.js'; import MarkdownRenderer from './markdown-renderer.js'; @@ -136,6 +136,24 @@ describe('MarkdownRenderer', () => { expect(result).toMatchSnapshot(); }); + + it('renders a list mixing a fallback (unfetchable) contributor with a full contributor', () => { + const fallbackContributor = { + login: 'deleted-user', + html_url: '', + } as GitHubContributor; + + const fullContributor = { + login: 'Turbo87', + name: 'Tobias Bieniek', + type: 'User', + html_url: '', + } as GitHubContributor; + + const result = renderer().renderContributorList([fallbackContributor, fullContributor]); + + expect(result).toMatchSnapshot(); + }); }); describe('renderContributor', () => { @@ -171,6 +189,24 @@ describe('MarkdownRenderer', () => { expect(result).toEqual('Copilot [Bot] ([@copilot-swe-agent](https://github.com/apps/copilot-swe-agent))'); }); + + it(`renders a fallback contributor that has an html_url`, () => { + const result = renderer().renderContributor({ + login: 'github-actions', + html_url: 'https://github.com/apps/github-actions', + } as GitHubContributor); + + expect(result).toEqual('[@github-actions](https://github.com/apps/github-actions)'); + }); + + it(`renders a fallback contributor with no html_url as a plain mention`, () => { + const result = renderer().renderContributor({ + login: 'deleted-user', + html_url: '', + } as GitHubContributor); + + expect(result).toEqual('@deleted-user'); + }); }); describe('groupByCategory', () => { diff --git a/src/markdown-renderer.ts b/src/markdown-renderer.ts index 5b76c9e..b9d3b71 100644 --- a/src/markdown-renderer.ts +++ b/src/markdown-renderer.ts @@ -122,7 +122,9 @@ export default class MarkdownRenderer { public renderContributor(contributor: GitHubContributor): string { const userName = (contributor as GithubAppInfo).slug ?? (contributor as GithubUserInfo).login; - const userNameAndLink = `[@${userName}](${contributor.html_url})`; + // html_url can be empty when getCommiters falls back for unfetchable committers (e.g. deleted users, restricted users). + // Render a plain @login metion instead of a markdown link with an empty href. + const userNameAndLink = contributor.html_url ? `[@${userName}](${contributor.html_url})` : `@${userName}`; if (contributor.name) { const name = contributor.name + (!('type' in contributor) ? ' [Bot]' : '');