fix(screenshot): keep the capture binary until it is returned - #938
Conversation
`encoding: 'base64'` is a documented page.screenshot option, but it made puppeteer hand back a string, and the blank-page check under `waitUntil: 'auto'` passed it to sharp as image bytes: "Input buffer contains unsupported image format". The overlay decodes with sharp too. Capture binary and encode once at the return, so every path (auto, a fixed waitUntil, fullPage, element, overlay) sees real bytes. `path` writes stay binary as puppeteer intends. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe screenshot API now accepts an ChangesScreenshot encoding
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What
encoding: 'base64'is one of the page.screenshot options this package promises to support, but it made puppeteer return a string instead of aUint8Array. Under the defaultwaitUntil: 'auto', the blank-page check then handed that string to sharp as image bytes:The overlay path decodes with sharp too, so it had the same problem.
encodingis now pulled out of the options given topage.screenshot, and applied once at the return. Every path (auto, an explicitwaitUntil,fullPage,element,overlay) sees real bytes, andpathwrites stay binary as puppeteer intends. The encode is zero-copy (Buffer.from(buffer, byteOffset, byteLength)), and it only touches byte arrays, sooverlay+path, which returns sharp'stoFileinfo object, passes through untouched.How it was tested
New regression test in
packages/screenshot/test/index.js, coveringauto, an explicitwaitUntil: 'load',fullPage,overlay, andencoding: 'binary':Input buffer contains unsupported image format.Full
packages/screenshotsuite: everything passes exceptgraphics features, which fails identically onmasterhere (it needs WebGL through ANGLE, which this macOS host lacks). Lint clean.Outcome
This was 14 events / 5 users over 14 days in the Microlink API's Sentry (
MICROLINK-API-QEV), where everyscreenshot.encoding=base64request was a 500. That metric should go to zero.🤖 Generated with Claude Code
Note
Low Risk
Localized screenshot return-path change with a focused regression test; no auth or data-handling impact.
Overview
Fixes
encoding: 'base64'breaking the defaultwaitUntil: 'auto'path (and overlay): Puppeteer was returning a string, which sharp used for blank-page detection treated as raw bytes and threw Input buffer contains unsupported image format.encodingis now stripped from options passed topage.screenshot, so internal steps always see binary.encodeScreenshotapplies base64 only on the final return (zero-copy from the byte buffer). Overlay is awaited before encoding.encoding: 'binary'and non-buffer returns (e.g.path) pass through unchanged.Adds a regression test covering auto, explicit load, fullPage, overlay, and binary encoding.
Reviewed by Cursor Bugbot for commit b9b30a1. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit