fix: github PR body max length by emmenko · Pull Request #157 · changesets/action

emmenko · 2022-03-04T10:20:22Z

I had an issue today where the PR body exceeded a size limit.

This prevents the PR to be created.

To fix that we can truncate the message if we detect it's too long.

Let me know what you think and if I need to add a test for this use case.

This is how the truncated message in the PR looks like:

changeset-bot · 2022-03-04T10:20:24Z

🦋 Changeset detected

Latest commit: 8d5da5c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@changesets/action	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Andarist · 2022-03-04T10:36:43Z

src/run.ts

  let searchResult = await searchResultPromise;
  console.log(JSON.stringify(searchResult.data, null, 2));
+
+  let messageBody = await prBodyPromise;


Since we have prBodyPromise, this variable probably should be called prBody to make the connection between them obvious.

Andarist · 2022-03-04T10:39:20Z

src/run.ts

+    messageBody = [
+      messageBody.substring(0, MAX_CHARACTERS_PER_MESSAGE),
+      "[Too long, message truncated]",
+    ].join("\n\n");


This has the potential of breaking some formatting stuff or breaking links etc (which has probably happened on the attached screenshot). I wonder if on such an occasion, we shouldn't just replace the whole "changelogs" part with a placeholder message explaining the problem and advising to read the git diff, we could even construct a link to the "File Changed" tab there. WDYT?

Yes, formatting might be broken. I don't have a strong opinion to be honest.
The important part is that the PR is opened without errors, as otherwise the release is blocked.

Up to you to decide.

I think it's safer to start with the suggested way.

We'd have to just move this part to compute a separate object:

action/src/run.ts

Line 245 in 7e613b3

await Promise.all(

and construct the final body later with the algorithm like:

prepare the message without considering the limit

if the limit has been exceeded then construct a fallback body that would have those things in it:

"this preface"

information that the limit has been exceeded

list of releases (names + versions, similar to what we already print in the full body, just without the changelog part

@Andarist I pushed some changes following your suggestions. It would be nice if you can take a look when you have time.
Thanks

emmenko · 2022-03-07T20:27:01Z

src/run.ts

+  preState?: PreState;
+};
+
+export async function getChangesetsMessage({


Extracted the logic to construct the message into a separate function.

emmenko · 2022-03-07T20:27:36Z

src/run.ts

  prTitle = "Version Packages",
  commitMessage = "Version Packages",
  hasPublishScript = false,
+  maxCharactersPerMessage = MAX_CHARACTERS_PER_MESSAGE,


Useful for testing

emmenko · 2022-03-07T20:28:08Z

src/run.ts

+          content: entry.content,
+          header: `## ${pkg.packageJson.name}@${pkg.packageJson.version}`,


These information have been split into two fields now.

Andarist · 2022-04-15T09:44:56Z

src/run.ts

+  // Check (again) that the message is within the size limit.
+  // If not, omit all release content this time.
+  if (fullMessage.length > maxCharactersPerMessage) {


heh, this is probably very, very unlikely :P but I appreciate thinking about this edge case 👍

Andarist · 2022-04-15T09:45:26Z

src/run.ts

+⚠️⚠️⚠️⚠️⚠️⚠️
+`
+    : "";
+  let messageReleases = `# Releases`;


nit:

Suggested change

let messageReleases = `# Releases`;

let messageReleasesHeading = `# Releases`;

Andarist · 2022-04-15T09:46:22Z

src/run.ts

  prTitle?: string;
  commitMessage?: string;
  hasPublishScript?: boolean;
+  maxCharactersPerMessage?: number;


nit:

Suggested change

maxCharactersPerMessage?: number;

prBodyMaxCharacters?: number;

Andarist · 2022-04-15T09:48:08Z

src/run.ts

+  preState?: PreState;
+};
+
+export async function getChangesetsMessage({


nit:

Suggested change

export async function getChangesetsMessage({

export async function getVersionPrBody({

Andarist · 2022-04-15T09:49:42Z

src/run.ts

  });
  let changedPackages = await getChangedPackages(cwd, versionsByDirectory);
+  let changedPackagesInfo = (
+    await Promise.all(


nit: previously this was parallelized, we could maintain this with:

Suggested change

await Promise.all(

Promise.all(

src/run.ts

… limit

emmenko · 2022-04-20T08:37:25Z

@Andarist Hi 👋, I applied your suggestions and rebased the branch.

I think this should be good to go now, unless you have other suggestions. Thanks

emmenko · 2022-04-20T10:15:47Z

Btw, here's a real example of how this looks when the content is reduced.

commercetools/ui-kit#2178

emmenko · 2022-05-10T08:21:12Z

@Andarist Friendly reminder in case you haven't had a chance to look at the final changes. Thanks

src/run.test.ts

src/run.ts

emmenko · 2022-05-11T13:21:22Z

@Andarist Awesome, thanks!

tedraykov · 2022-10-06T10:57:38Z

This solution only fixes the generated Version PR body exceeding the max body length. The problem still exists for creating GitHub releases:

Error: Validation Failed: {"resource":"Release","code":"custom","field":"body","message":"body is too long (maximum is 125000 characters)"}

The failing action run: https://github.com/tedraykov/reaction/actions/runs/3196539005/jobs/5218616158#step:7:163

Andarist · 2022-10-06T21:14:35Z

My educated guess here is that we could detect the format of this CHANGELOG correctly and thus the whole thing was passed as the github release body in the payload. This is a problem on its own. Exceeding the character limit with legitimate requests is possible but very unlikely.

…60k characters by omitting some of the changelog information (changesets#157) * fix: body max length * docs: improve logs * docs: changeset * refactor: changeset message to omit some info if message exceeds size limit * refactor: minor adjustments based on feedback * Apply nits from code review * Fixed tests Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>

Andarist reviewed Mar 4, 2022

View reviewed changes

emmenko force-pushed the nm-fix-body-max-length branch from 61ae721 to a01e812 Compare March 7, 2022 20:26

emmenko commented Mar 7, 2022

View reviewed changes

Andarist mentioned this pull request Apr 15, 2022

body is too long (maximum is 65536 characters) #174

Closed

Andarist linked an issue Apr 15, 2022 that may be closed by this pull request

body is too long (maximum is 65536 characters) #174

Closed

Andarist reviewed Apr 15, 2022

View reviewed changes

src/run.ts Show resolved Hide resolved

emmenko added 5 commits April 20, 2022 10:27

fix: body max length

55e5630

docs: improve logs

71fc24b

docs: changeset

97e5569

refactor: changeset message to omit some info if message exceeds size…

e351993

… limit

refactor: minor adjustments based on feedback

6220154

emmenko force-pushed the nm-fix-body-max-length branch from a01e812 to 6220154 Compare April 20, 2022 08:36

emmenko requested a review from Andarist April 20, 2022 08:36

Andarist reviewed May 10, 2022

View reviewed changes

src/run.test.ts Outdated Show resolved Hide resolved

Andarist reviewed May 10, 2022

View reviewed changes

src/run.test.ts Outdated Show resolved Hide resolved

Andarist reviewed May 10, 2022

View reviewed changes

src/run.ts Outdated Show resolved Hide resolved

Andarist reviewed May 10, 2022

View reviewed changes

src/run.ts Outdated Show resolved Hide resolved

Apply nits from code review

b083449

Andarist added 2 commits May 10, 2022 10:40

Merge branch 'main' into nm-fix-body-max-length

680ac61

Fixed tests

8d5da5c

Andarist merged commit 521c27b into changesets:main May 10, 2022

github-actions bot mentioned this pull request May 10, 2022

Version Packages #169

Merged

emmenko deleted the nm-fix-body-max-length branch May 11, 2022 13:21

github-actions bot mentioned this pull request Sep 8, 2022

Version Packages pedronauck/changesets-action#1

Closed

emmenko mentioned this pull request Sep 8, 2022

Support "aggregate" mode in createGithubReleases config flag #193

Open

davenicoll mentioned this pull request Oct 18, 2022

working dir fixes htaic/.github#7

Merged

9 tasks

sethidden mentioned this pull request Jun 23, 2023

createGithubReleases: true -> body is too long (max is 125000 characters) #304

Closed

		content: entry.content,
		header: `## ${pkg.packageJson.name}@${pkg.packageJson.version}`,

	let messageReleases = `# Releases`;
	let messageReleasesHeading = `# Releases`;

	maxCharactersPerMessage?: number;
	prBodyMaxCharacters?: number;

	export async function getChangesetsMessage({
	export async function getVersionPrBody({

Conversation

emmenko commented Mar 4, 2022

Uh oh!

changeset-bot bot commented Mar 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andarist Mar 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

emmenko commented Apr 20, 2022

Uh oh!

emmenko commented Apr 20, 2022

Uh oh!

emmenko commented May 10, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emmenko commented May 11, 2022

Uh oh!

tedraykov commented Oct 6, 2022

Uh oh!

Andarist commented Oct 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

changeset-bot bot commented Mar 4, 2022 •

edited

Loading

Andarist Mar 4, 2022 •

edited

Loading