-
-
Notifications
You must be signed in to change notification settings - Fork 499
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.
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
Fix generation of large PDFs #779
Conversation
Thanks! It seems like the tests are failing, could you take a look at that? |
Bumps [stefanzweifel/git-auto-commit-action](https://github.com/stefanzweifel/git-auto-commit-action) from 4 to 5. - [Release notes](https://github.com/stefanzweifel/git-auto-commit-action/releases) - [Changelog](https://github.com/stefanzweifel/git-auto-commit-action/blob/master/CHANGELOG.md) - [Commits](stefanzweifel/git-auto-commit-action@v4...v5) --- updated-dependencies: - dependency-name: stefanzweifel/git-auto-commit-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v3...v4) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
The test "can evaluate page" was failing because it still needs to receive the encoded the output. So I moved the encoding condition inside the I also added |
I adapted my PR to support the new |
Will this be merged? Seems like a helpful addition. |
Could you rebase this with |
I rebased with main (v4). The tests are failing though but it's unrelated to this update. |
There are still some conflicts that prevent me from merging. Could you rebase again with |
Bumps [stefanzweifel/git-auto-commit-action](https://github.com/stefanzweifel/git-auto-commit-action) from 4 to 5. - [Release notes](https://github.com/stefanzweifel/git-auto-commit-action/releases) - [Changelog](https://github.com/stefanzweifel/git-auto-commit-action/blob/master/CHANGELOG.md) - [Commits](stefanzweifel/git-auto-commit-action@v4...v5) --- updated-dependencies: - dependency-name: stefanzweifel/git-auto-commit-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v3...v4) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
This reverts commit a582204.
This reverts commit cd32f37.
I rebased with main again. If this doesn't work, I'll open a new fresh PR. |
Best to open a new PR. Unfortunately, GitHub still detects conflicts, and won't let me merge this. |
Browsershot can not currently save PDFs that are over 512 MB.
The issue is described quite well here: microsoft/playwright#14675
The error
Cannot create a string longer than 0x1fffffe8 characters
occurs atoutput.toString('base64')
which can not handle very large output (see Stack Overflow thread).However, we don't need to convert the output to base64 when saving PDF files to disk. If we did, we should serialize the output to buffer but I don't think this is necessary.
Fixes #642 and #745.