Merged
Conversation
This is the first step in attempting to solve the case when diff comments are too long. From testing it looks like the comment limit that the API returns (65535) is not the real limit, but it is hard to find any concrete information on what the real limit is. I was able to post some comments with a length > 150000. Instead of doing any validation on our end and failing early, this PR updates the logic to rely on the GH API failure message. We will always attempt to comment and catch any "Body too long" errors that occur. This should increase the size of the comment allowed and may fix the issue for some users. I will follow up this with additional PRs fixing the issue completely. re #34
There was a problem hiding this comment.
Pull Request Overview
This PR addresses the handling of overly long diff comments by removing the early length check and instead catching GitHub API “Body too long” errors.
- Updated comment logic to rely on the API error message rather than a fixed MAX_COMMENT_LENGTH.
- Refactored test code to accommodate the new error handling and adjusted expected mock call counts.
- Modified comment processing to run stack comment posting in parallel with improved error aggregation.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/stage-processor.test.ts | Updated tests with a new helper for setting up comment tests and adjusted mock call counts. |
| src/stage-processor.ts | Refactored comment methods to remove fixed length checks and implement error handling using API error responses. |
Comments suppressed due to low confidence (2)
src/stage-processor.ts:251
- [nitpick] Consider ensuring that the items in the 'failed' array are converted to strings (e.g. using error.message) before joining them, to avoid ambiguous error outputs if the rejection reasons are not plain strings.
const failed = res.filter((r) => r.status === 'rejected').flatMap((r) => r.reason);
test/stage-processor.test.ts:503
- [nitpick] The removal of the afterEach block that resets and restores mocks may lead to test leakage; consider reintroducing a cleanup step after each test to ensure test isolation.
- jest.resetAllMocks(); // clears mock state
rantoniuk
added a commit
to rantoniuk/corymhall-cdk-diff-action
that referenced
this pull request
Jun 2, 2025
* v2-beta: fix: process longer comments (corymhall#137) fix: upgrade toolkit-lib and use stack displayName (corymhall#134)
corymhall
added a commit
that referenced
this pull request
Jun 20, 2025
This is the first step in attempting to solve the case when diff comments are too long. From testing it looks like the comment limit that the API returns (65535) is not the real limit, but it is hard to find any concrete information on what the real limit is. I was able to post some comments with a length > 150000. Instead of doing any validation on our end and failing early, this PR updates the logic to rely on the GH API failure message. We will always attempt to comment and catch any "Body too long" errors that occur. This should increase the size of the comment allowed and may fix the issue for some users. I will follow up this with additional PRs fixing the issue completely. re #34
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the first step in attempting to solve the case when diff
comments are too long. From testing it looks like the comment limit that
the API returns (65535) is not the real limit, but it is hard to find
any concrete information on what the real limit is. I was able to post
some comments with a length > 150000.
Instead of doing any validation on our end and failing early, this PR
updates the logic to rely on the GH API failure message. We will always
attempt to comment and catch any "Body too long" errors that occur.
This should increase the size of the comment allowed and may fix the
issue for some users. I will follow up this with additional PRs fixing
the issue completely.
re #34