feat: Cloudflare Browser Rendering backend + bound puppeteer memory#72
Draft
nonumpa wants to merge 11 commits into
Draft
feat: Cloudflare Browser Rendering backend + bound puppeteer memory#72nonumpa wants to merge 11 commits into
nonumpa wants to merge 11 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request upgrades the project to Node 24, migrates from the deprecated grpc package to @grpc/grpc-js, and introduces support for Cloudflare Browser Rendering as an alternative browser backend. It also adds resource limits for scraping, such as blocking specific resource types and capping concurrency using p-limit. However, several critical issues must be addressed: the gRPC server will not accept requests because server.start() was omitted from the new asynchronous binding callback; unhandled promise rejections from browser launching and request interception could crash the Node 24 process; using new Function for evaluating Readability.js will fail on sites with strict Content Security Policies; and wrapping the entire resolution pipeline in limit unnecessarily throttles fast, non-Puppeteer operations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+283
to
+289
| resultArticle = await page.evaluate(readabilityJsStr => { | ||
| // eslint-disable-next-line no-new-func | ||
| const fn = new Function( | ||
| readabilityJsStr + ';return new Readability(document).parse();' | ||
| ); | ||
| return fn(); | ||
| }, readabilityJsStr); |
There was a problem hiding this comment.
Using new Function inside the browser context is subject to the page's Content Security Policy (CSP). On websites that restrict unsafe-eval (which is very common), this will throw an EvalError and fail to resolve the URL. Instead, pass the concatenated script string directly to page.evaluate(), which executes via CDP and completely bypasses CSP restrictions.
resultArticle = await page.evaluate(
`${readabilityJsStr};(() => new Readability(document).parse())()`
);
Comment on lines
+18
to
+56
| urls.map(url => | ||
| limit(async () => { | ||
| let fetchResult; | ||
| try { | ||
| // Normalize and unshorten URLs, update fetchResult | ||
| const normalized = normalize(url); | ||
| fetchResult = new ScrapResult({ canonical: normalized }); | ||
|
|
||
| const unshortened = await unshorten(normalized); | ||
| fetchResult = new ScrapResult({ canonical: unshortened }); | ||
| const unshortened = await unshorten(normalized); | ||
| fetchResult = new ScrapResult({ canonical: unshortened }); | ||
|
|
||
| // Fetch info from page | ||
| fetchResult = await parseMeta(unshortened); | ||
| // Fetch info from page | ||
| fetchResult = await parseMeta(unshortened); | ||
|
|
||
| if (fetchResult.isIncomplete) { | ||
| fetchResult.merge(await scrap(unshortened)); | ||
| } | ||
| if (fetchResult.isIncomplete) { | ||
| fetchResult.merge(await scrap(unshortened)); | ||
| } | ||
|
|
||
| call.write({ | ||
| ...fetchResult, | ||
| top_image_url: fetchResult.topImageUrl, | ||
| url, // Provide the most original url | ||
| successfully_resolved: true, | ||
| }); | ||
| } catch (e) { | ||
| // eslint-disable-next-line no-console | ||
| console.error('[resolvedUrls]', url, e); | ||
| let errMsg; | ||
| if (e instanceof ResolveError) { | ||
| errMsg = e.returnedError; | ||
| call.write({ | ||
| ...fetchResult, | ||
| top_image_url: fetchResult.topImageUrl, | ||
| url, // Provide the most original url | ||
| successfully_resolved: true, | ||
| }); | ||
| } catch (e) { | ||
| // eslint-disable-next-line no-console | ||
| console.error('[resolvedUrls]', url, e); | ||
| let errMsg; | ||
| if (e instanceof ResolveError) { | ||
| errMsg = e.returnedError; | ||
| } | ||
| call.write({ | ||
| ...fetchResult, // Still try return available fetch result | ||
| url, | ||
| error: errMsg, | ||
| }); | ||
| } | ||
| call.write({ | ||
| ...fetchResult, // Still try return available fetch result | ||
| url, | ||
| error: errMsg, | ||
| }); | ||
| } | ||
| }) | ||
| }) | ||
| ) |
There was a problem hiding this comment.
Wrapping the entire resolution block in limit caps the concurrency of all operations, including lightweight ones like normalize, unshorten, and parseMeta (which do not use Puppeteer). This introduces an unnecessary performance bottleneck for URLs that can be resolved quickly without scraping. Instead, only wrap the scrap call with limit so that the fast-path resolutions can run with full concurrency.
urls.map(async url => {
let fetchResult;
try {
// Normalize and unshorten URLs, update fetchResult
const normalized = normalize(url);
fetchResult = new ScrapResult({ canonical: normalized });
const unshortened = await unshorten(normalized);
fetchResult = new ScrapResult({ canonical: unshortened });
// Fetch info from page
fetchResult = await parseMeta(unshortened);
if (fetchResult.isIncomplete) {
fetchResult.merge(await limit(() => scrap(unshortened)));
}
call.write({
...fetchResult,
top_image_url: fetchResult.topImageUrl,
url,
successfully_resolved: true,
});
} catch (e) {
// eslint-disable-next-line no-console
console.error('[resolvedUrls]', url, e);
let errMsg;
if (e instanceof ResolveError) {
errMsg = e.returnedError;
}
call.write({
...fetchResult,
url,
error: errMsg,
});
}
})
A new launchOrConnect() helper picks between puppeteer.launch() (local chromium) and puppeteer-core.connect() (Cloudflare Browser Rendering WebSocket CDP) based on BROWSER_BACKEND env. Default stays local so existing deployments are unchanged. Cloudflare credentials live in CLOUDFLARE_ACCOUNT_ID + CLOUDFLARE_API_TOKEN and the required token scope is Browser Rendering: Edit. keep_alive defaults to the documented 600000 ms (10 min) ceiling and can be overridden via CLOUDFLARE_KEEP_ALIVE_MS. puppeteer-core is added as a dependency at the same major version as puppeteer to keep CDP wire protocols aligned.
scrap.js now obtains its browser via launchOrConnect(), so the same binary can run with local chromium (default) or Cloudflare Browser Rendering (BROWSER_BACKEND=cloudflare). All downstream code paths - targetcreated handler, disconnected reconnect, page-level operations - work identically against either backend because puppeteer.connect() returns the same Browser interface as puppeteer.launch().
Note Workers Paid plan requirement (Free tier 10 min/day is insufficient for production URL resolution) and indicative cost at 10k/100k URLs per day so operators can size their Cloudflare account before flipping the flag.
A five-URL request fans out into five concurrent puppeteer pages, each holding ~50 MB. Under bursty load this adds enough memory pressure to OOM the container; puppeteer auto-reconnects on disconnect without backoff, so the cycle repeats. Wrap each scrap call in a module-level p-limit semaphore so the cap holds across all in-flight gRPC streams, not per call. Default 3, override with SCRAP_MAX_CONCURRENCY.
Loading these resource types only inflates JS heap and network sockets; none of title, summary, canonical, or og:image-driven topImageUrl reads them. With image loading skipped, the topImageUrl fallback that walks <img> by rendered size returns the first image instead of the largest, so sites without og:image lose the "largest image" semantics. Sites with og:image (the common case) are unaffected. Default block list image,media,font; clear via SCRAP_BLOCK_RESOURCES= to disable.
Document the concurrency cap and resource block list with the topImageUrl fallback caveat so operators can tune or disable them per workload.
page.on('request', ...) handlers ignore the promises returned by req.abort()/req.continue(). If the page closes or the request was already resolved mid-flight (navigation cancellation, target closed), the rejection surfaces as an unhandled promise rejection on Node 24.
The semaphore exists to bound puppeteer page memory, but wrapping the entire per-url pipeline also serialised normalize/unshorten/parseMeta. A few slow metadata-only urls could then block unrelated fast results without ever opening Chromium.
nonumpa
force-pushed
the
feat/cloudflare-browser-rendering
branch
from
dfda2df to
ab9adce
Compare
The eager launchBrowser() at module load opens a remote browser session on process start even before any request arrives. The disconnected handler then immediately reconnects, so when Cloudflare auto-closes the session after the keep_alive inactivity window (max 10 min), we acquire a fresh session and the cycle repeats forever, burning quota and browser-time at idle. Split the lifecycle by backend: local stays eager (no billing, hot crash recovery wanted), cloudflare connects lazily on first scrap() and clears browserPromise on disconnect so the next scrap reconnects on demand. Also surface launchOrConnect rejections via a catch handler that resets browserPromise, so a transient acquire failure does not pin the resolver to a rejected promise.
nonumpa
changed the title
The previous 600000 ms (10 min) default was chosen when scrap eagerly relaunched on disconnect, so the keep_alive window only controlled churn frequency, not the bill — one session was always open either way. With lazy lifecycle (5c2dff8), the session only exists during/around traffic, and the keep_alive window is now a trade-off between scrap-to-scrap reuse and billed idle tail after the last request (CF bills until session timeout). Use CF's own 60s default; operators can widen for sparse traffic via CLOUDFLARE_KEEP_ALIVE_MS.
The repo has used scrap as the verb/noun for 'fetch + render + extract' since the first commit, but the correct English spelling is scrape. Rename module files (scrap.js, ScrapResult.js, mocks, tests), function and class identifiers, env var prefixes (SCRAPE_MAX_CONCURRENCY, SCRAPE_BLOCK_RESOURCES), the proto enum (UNKNOWN_SCRAPE_ERROR), README prose, and snapshot fixtures. Wire-format ResolveError numeric values are unchanged, so existing gRPC clients keep working without redeploy. Downstream code that imports the symbolic name UNKNOWN_SCRAP_ERROR will need to update on next proto re-sync.
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
1 participant
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.
Two related changes for url-resolver: an alternate browser backend (Cloudflare Browser Rendering) and memory bounds for the existing local Puppeteer path. They share the same audit of scrap.js and ship together because the memory bounds are what make the local path safe to keep running unattended.
Cloudflare Browser Rendering backend
Adds
local(default) andcloudflarebackends behind aBROWSER_BACKENDenv switch. Thecloudflarebackend connects via CDP over WebSocket to Cloudflare's hosted Chromium, returning aBrowserinstance compatible with the existingscrap.jscode path.Commits:
ca2deecfeat(browser): backend factorylaunchOrConnect.jswith tests for both branches0c95d06feat(scrap): scrap calls the factory instead ofpuppeteer.launchdirectly26f8756docs: README +.env.samplecover the new env vars5c2dff8fix(scrap): lazy connect for cloudflare backend — see belowBackend selection is explicit (no auto-detection from credential presence) so the same image can ship to both deployments without surprise.
Lazy lifecycle for Cloudflare
Cloudflare Browser Rendering sessions are billed by browser-time and auto-close after the
keep_aliveinactivity window (max 10 min). Eager acquisition at process start + eager relaunch on disconnect would acquire a fresh session every idle keep_alive cycle even with zero traffic, burning quota and silently breaking the cost model below. So:scrap()opens the session; on disconnect the resolver clears its handle so the nextscrap()reconnects on demand. Idle = no session = no spend.A transient acquire failure no longer pins the resolver to a rejected promise; the rejection is captured and the next call retries.
Configuration
Bound puppeteer memory (local backend)
A five-URL gRPC request fans out into five concurrent Puppeteer pages, each holding ~50 MB of Chromium memory plus image/font/media buffers. With
Promise.all(urls.map(scrap))and nosetRequestInterception, bursty traffic OOMs the container; Puppeteer auto-reconnects on disconnect with no backoff so the cycle repeats.Commits:
133db95feat(resolver): module-levelp-limitsemaphore caps concurrent scraps across all in-flight gRPC streams. Default 3, override viaSCRAP_MAX_CONCURRENCY.a32dccbfeat(scrap): abortsimage,media, andfontrequests during page load. None oftitle,summary,canonical, orog:image-driventopImageUrlreads them. Default block listimage,media,font, clear viaSCRAP_BLOCK_RESOURCES=to disable.12f75dfdocs: README +.env.samplecover the new env vars and thetopImageUrlfallback caveat.40654ccfix(scrap):req.abort()/req.continue()return promises in puppeteer 24 and reject if the page closes mid-flight. Captured to avoid Node 24 unhandled-rejection surfacing.ab9adcefix(resolver): semaphore wraps onlyscrap(), not the whole per-url pipeline.normalize/unshorten/parseMetarun with their natural concurrency since they don't open Chromium.Configuration
Trade-off
topImageUrlhas two paths inscrap.js:og:image/twitter:imagefrom<meta>— unaffected, this runs entirely off the parsed DOM.<img>and picks the largest by rendered area — affected. With image loading aborted, all<img>elements report 0×0, so the fallback returns the first<img>instead of the largest.Sites with
og:image(most major publishers, social sites, and CMS-rendered pages) are unaffected. Sites withoutog:imagelose the "largest image" semantics. Operators who need the original semantics can setSCRAP_BLOCK_RESOURCES=(empty) to disable interception.Verification
npm test: 7 suites / 35 tests / 83 snapshots, all passing. New tests:launchOrConnect.test.js: 4 tests covering local launch and Cloudflare CDP connect paths.scrap.test.js(new): 10 tests — 3 for resource interception (default block / custom override / empty disables), 1 for abort/continue rejection swallowing, 5 for backend lifecycle (eager local launch, no eager cloudflare launch, lazy cloudflare connect on first scrap, no auto-reconnect on cloudflare disconnect, eager reconnect on local disconnect, reconnect-on-demand after cloudflare disconnect), and the shared interception suite.resolveUrls.test.js: 1 test assertingscrap()concurrency is capped atSCRAP_MAX_CONCURRENCY=3with 5 URLs queued whileparseMetaconcurrency reachesurls.length.npm run lint: clean.title,canonical,summaryLen: 100% identical with vs. without resource blocking.topImageUrl: 8/8 sites identical. The one observed difference (BBC) was different breaking-news images between two consecutive runs, not a code-induced delta.status=200, without-block 3/4status=200. Blocking image/media/font letsnetworkidle0settle sooner and reduces flake.Backwards compatibility
BROWSER_BACKEND=localpreserves existing deployment behaviour (a local Chromium per scrap).SCRAP_MAX_CONCURRENCY=3andSCRAP_BLOCK_RESOURCES=image,media,fontchange runtime behaviour but not API contracts. SetSCRAP_BLOCK_RESOURCES=to fully restore prior network behaviour. RaiseSCRAP_MAX_CONCURRENCYto relax the cap.ScrapResultfields.