Skip to content

fix: rate-limit comment API calls#1

Open
seth-martin-bold-penguin wants to merge 2 commits into
mainfrom
fix/rate-limit-comments
Open

fix: rate-limit comment API calls#1
seth-martin-bold-penguin wants to merge 2 commits into
mainfrom
fix/rate-limit-comments

Conversation

@seth-martin-bold-penguin

Copy link
Copy Markdown

Description

When CDK diff output is too large for a single PR comment, the action falls back to one comment per stack. It fires ALL comment API calls in parallel via Promise.allSettled, hitting GitHub's secondary rate limit with 100+ stacks.

Fix: Replace parallel Promise.allSettled with sequential execution + 200ms delay between API calls.

How did you test this?

Will validate via cdk-infra PR.

@szechyjs

szechyjs commented Jul 1, 2026

Copy link
Copy Markdown
Member

Can you instead update the source, and rebuild the dist part?

https://github.com/BoldPenguin/cdk-diff-action/blob/main/src/stage-processor.ts#L246

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread dist/index.js
failed.push(e);
}
// Rate limit: wait 200ms between API calls to avoid GitHub secondary rate limits
await new Promise(resolve => setTimeout(resolve, 200));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you think about not commenting per Stack at all, which is too many comments.

@diwakar-chand-bp diwakar-chand-bp Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be we can just truncate? and do a manual review. We have too many stacks. I'd add an attribute to the acton supressCommentPerStack -- we disable that, and the post 1 comment per stage, message too long review from local or something like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants