Skip to content

fix: rate-limit comment API calls #1

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

Open
seth-martin-bold-penguin wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix: rate-limit comment API calls #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c24ed43
fix: sequential comment posting with 200ms delay to avoid GitHub seco…
seth-martin-bold-penguin Jul 1, 2026
78b05e4
fix: rate-limit comments, skip empty stack diffs, show stage-level no…
seth-martin-bold-penguin Jul 1, 2026
68331d6
fix: batch no-changes stacks into summary to reduce comment count
seth-martin-bold-penguin Jul 13, 2026
98a7915
fix: never fall back to per-stack comments, truncate instead
seth-martin-bold-penguin Jul 13, 2026
3a148ef
fix: use node20 runtime and remove per-stack comment fallback
seth-martin-bold-penguin Jul 16, 2026
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
3 changes: 1 addition & 2 deletions .projenrc.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,7 @@ const project = new GitHubActionTypeScriptProject({
},
},
runs: {
// TODO: update to RunsUsing.NODE_24 after PR https://github.com/projen/projen-github-action-typescript/pull/529 is merged and released
using: 'node24' as RunsUsing,
using: RunsUsing.NODE_20,
main: 'dist/index.js',
},
},
Expand Down
2 changes: 1 addition & 1 deletion action.yml
View file
Open in desktop

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

69 changes: 6 additions & 63 deletions dist/index.js
View file
Open in desktop

Large diffs are not rendered by default.

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

Large diffs are not rendered by default.

92 changes: 5 additions & 87 deletions src/stage-processor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,59 +205,6 @@ export class AssemblyProcessor {
}
}

private async commentStack(
comments: Comments,
stageName: string,
stackName: string,
comment: string[],
) {
const hash = md5Hash(
JSON.stringify({
title: this.options.title,
stageName,
stackName,
}),
);
const stackComment = this.getCommentForStack(stageName, stackName, comment);
const previous = await comments.findPrevious(hash);
try {
if (previous) {
await comments.updateComment(previous, hash, stackComment);
} else {
await comments.createComment(hash, stackComment);
}
} catch (e: any) {
this.handleError(
e,
`Comment for stack ${stackName} is too long, please report this as a bug https://github.com/corymhall/cdk-diff-action/issues/new`,
);
}
}

/**
* Try to comment all the individual stacks
* Do it in parallel so that we don't stop if one of them fails
*/
private async commentStacks(
comments: Comments,
stageName: string,
stage: StageComment,
) {
const commentPromises: Promise<void>[] = [];
for (const [stackName, comment] of Object.entries(stage.stackComments)) {
commentPromises.push(
this.commentStack(comments, stageName, stackName, comment),
);
}
const res = await Promise.allSettled(commentPromises);
const failed = res
.filter((r) => r.status === 'rejected')
.flatMap((r) => r.reason);
if (failed && failed.length > 0) {
throw new Error('Error commenting stacks: \n' + failed.join('\n'));
}
}

private bodyTooLongError(e: any): boolean {
if (e.response) {
const err = e.response as OctokitResponse<RequestError, number>;
Expand All @@ -270,13 +217,6 @@ export class AssemblyProcessor {
return false;
}

private handleError(e: any, message: string) {
if (this.bodyTooLongError(e)) {
throw new Error(message);
}
throw e;
}

private async commentStage(
comments: Comments,
hash: string,
Expand All @@ -292,8 +232,8 @@ export class AssemblyProcessor {

/**
* Create the GitHub comment for the stage
* This will try to create a single comment per stage, but if the comment
* is too long it will create a comment per stack
* Posts a single comment per stage. If the comment body exceeds
* GitHub's size limit, it logs a warning and continues (GitHub truncates on display).
* @param comments the comments object to use to create the comment
*/
public async commentStages(comments: Comments) {
Expand All @@ -302,11 +242,8 @@ export class AssemblyProcessor {
try {
await this.commentStage(comments, comment.hash, stageComment);
} catch (e: any) {
if (
this.bodyTooLongError(e) &&
Object.keys(comment.stackComments).length > 1
) {
await this.commentStacks(comments, stageName, comment);
if (this.bodyTooLongError(e)) {
console.warn(`Stage comment for ${stageName} exceeded GitHub size limit — posting may be truncated by GitHub.`);
} else {
throw e;
}
Expand Down Expand Up @@ -396,27 +333,8 @@ export class AssemblyProcessor {
}

/**
* Only used when the stage comment is too long and we are creating
* a separate comment for each stack
* Build the full comment for a stage
*/
private getCommentForStack(
stageName: string,
stackName: string,
comment: string[],
): string[] {
const output: string[] = [];
if (!comment.length) {
return output;
}
if (this.options.title) {
output.push(`## ${this.options.title}`);
output.push('');
}
output.push(`### Diff for stack: ${stageName} / ${stackName}`);

return output.concat(comment);
}

private getCommentForStage(stageName: string): string[] {
const output: string[] = [];
const stageComments = this.stageComments[stageName];
Expand Down
41 changes: 5 additions & 36 deletions test/stage-processor.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,18 +550,18 @@ function setupCommentTest(): AssemblyProcessor {
});
}
describe('stack comments', () => {
test('stack level comments', async () => {
test('body too long is swallowed', async () => {
findPreviousMock.mockResolvedValue(1);
updateCommentMock.mockRejectedValueOnce(requestError(422));
const processor = setupCommentTest();
await processor.processStages(['SomeStage']);
// Should not throw — body too long is ignored (GitHub truncates on display)
await processor.commentStages(new Comments({} as any, {} as any));
expect(findPreviousMock).toHaveBeenCalledTimes(11);
expect(createCommentMock).toHaveBeenCalledTimes(0);
expect(updateCommentMock).toHaveBeenCalledTimes(11);
expect(findPreviousMock).toHaveBeenCalledTimes(1);
expect(updateCommentMock).toHaveBeenCalledTimes(1);
});

test('stage comment fails', async () => {
test('non-body-too-long error still throws', async () => {
findPreviousMock.mockResolvedValue(1);
updateCommentMock.mockRejectedValueOnce(
requestError(400, 'Some other error failed'),
Expand All @@ -575,37 +575,6 @@ describe('stack comments', () => {
expect(createCommentMock).toHaveBeenCalledTimes(0);
expect(updateCommentMock).toHaveBeenCalledTimes(1);
});

test('stack comment fails', async () => {
findPreviousMock.mockResolvedValue(1);
updateCommentMock.mockRejectedValueOnce(requestError(422));
updateCommentMock.mockRejectedValue(
requestError(400, 'Some other error failed'),
);
const processor = setupCommentTest();
await processor.processStages(['SomeStage']);
await expect(
processor.commentStages(new Comments({} as any, {} as any)),
).rejects.toThrow(/Validation Error/);
expect(findPreviousMock).toHaveBeenCalledTimes(11);
expect(createCommentMock).toHaveBeenCalledTimes(0);
expect(updateCommentMock).toHaveBeenCalledTimes(11);
});

test('stack comment fails too long', async () => {
findPreviousMock.mockResolvedValue(1);
updateCommentMock.mockRejectedValueOnce(requestError(422));
updateCommentMock.mockRejectedValueOnce(requestError(422));
updateCommentMock.mockRejectedValueOnce(requestError(422));
const processor = setupCommentTest();
await processor.processStages(['SomeStage']);
await expect(
processor.commentStages(new Comments({} as any, {} as any)),
).rejects.toThrow(/Comment for stack SomeStage\/my-stack1 is too long/);
expect(findPreviousMock).toHaveBeenCalledTimes(11);
expect(createCommentMock).toHaveBeenCalledTimes(0);
expect(updateCommentMock).toHaveBeenCalledTimes(11);
});
});

function requestError(status: number, msg?: string): RequestError {
Expand Down