Skip puppeteer when a static fetch can extract the article#74
Conversation
Single-hop unshorten breaks chains like bit.ly to lihi to real URL,
and silently falls back to the original on servers that 405 HEAD.
- Loop up to 5 hops with cycle detection
- Retry HEAD with GET on 405/501
- Resolve relative Location against the current URL
- Send a CofactsBot User-Agent so shorteners gated by default-UA
filters return Location instead of 401/403
- Return { url, status } so callers can act on the final response status
When parseMeta returns incomplete and the upstream URL responds 5xx, 401, 403, 410, or 451, rendering it in puppeteer cannot recover the content. Surface the status to the caller and skip the scrap step. 404 still flows through because some sites serve soft-404s with valid OG metadata that puppeteer can extract from the rendered DOM.
For SSR-rendered pages a plain HTTP fetch already contains the article body; extracting it server-side via jsdom + Readability avoids booting puppeteer entirely. The new extractStatic step runs after parseMeta and before the puppeteer fallback. It throws ResolveError on the same network failures as unshorten so callers see consistent errors, and returns null on non-HTML responses or jsdom parse errors so the puppeteer fallback can still run when needed. Tests mock jsdom + @mozilla/readability so the test runner does not need to load jsdom's transitive ESM-only deps; real-world integration is exercised by bench/compare.js.
bench/compare.js runs extractStatic and scrap on a list of URLs and records per-call elapsed_ms, RSS delta, V8 heap delta, and (for puppeteer) the per-page JSHeapUsedSize from page.metrics(). Outputs a per-URL CSV plus a method-level summary on stdout. bench/README.md documents what each metric means, how to interpret the results, and the memory-measurement caveats (separate processes recommended for clean baselines; the persistent ~500 MB Chromium overhead is amortized across pages and not counted per URL). bench/results/ is gitignored so generated CSVs do not pollute the repo.
There was a problem hiding this comment.
Code Review
This pull request introduces a fast-path static extraction mechanism (extractStatic) using jsdom and @mozilla/readability to avoid booting Puppeteer for SSR-rendered pages, alongside a benchmarking suite to compare performance. It also refactors the unshorten utility to follow redirect chains up to a maximum number of hops with HEAD/GET fallback. The code review feedback highlights critical opportunities to prevent memory and socket leaks, specifically by explicitly closing the JSDOM instance, consuming unread response bodies in node-fetch early-exit paths, and wrapping the static extraction call in a try-catch block to ensure robust fallback to Puppeteer if a network error occurs.
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.
| let dom; | ||
| try { | ||
| dom = new JSDOM(html, { url: finalUrl }); | ||
| } catch (e) { | ||
| return null; | ||
| } | ||
|
|
||
| const document = dom.window.document; | ||
|
|
||
| const canonicalEl = document.querySelector('link[rel=canonical]'); | ||
| const canonical = | ||
| (canonicalEl && canonicalEl.href) || | ||
| pickMeta(document, 'meta[property="og:url"]') || | ||
| finalUrl; | ||
|
|
||
| let topImageUrl = ''; | ||
| const ogImage = pickMeta( | ||
| document, | ||
| 'meta[property="og:image"]', | ||
| 'meta[property="og:image:url"]', | ||
| 'meta[name="twitter:image"]' | ||
| ); | ||
| if (ogImage) { | ||
| try { | ||
| topImageUrl = new URL(ogImage, canonical).href; | ||
| } catch (e) { | ||
| topImageUrl = ''; | ||
| } | ||
| } | ||
|
|
||
| let article = null; | ||
| try { | ||
| article = new Readability(document).parse(); | ||
| } catch (e) { | ||
| article = null; | ||
| } | ||
|
|
||
| const title = | ||
| (article && article.title && article.title.trim()) || | ||
| pickMeta(document, 'meta[property="og:title"]') || | ||
| (document.title || '').trim(); | ||
|
|
||
| const summary = | ||
| (article && article.textContent && article.textContent.trim()) || | ||
| pickMeta( | ||
| document, | ||
| 'meta[property="og:description"]', | ||
| 'meta[name=description]' | ||
| ); | ||
|
|
||
| return new ScrapResult({ | ||
| canonical, | ||
| title: title || undefined, | ||
| summary: summary || undefined, | ||
| topImageUrl: topImageUrl || undefined, | ||
| html, | ||
| status, | ||
| }); | ||
| } |
There was a problem hiding this comment.
To prevent memory leaks in a long-running server process, the JSDOM instance should be explicitly closed using dom.window.close(). Additionally, if og:url is relative, resolving canonical against finalUrl ensures it is always an absolute URL, preventing errors when resolving topImageUrl.
let dom;
try {
dom = new JSDOM(html, { url: finalUrl });
} catch (e) {
return null;
}
try {
const document = dom.window.document;
const canonicalEl = document.querySelector('link[rel=canonical]');
let canonical =
(canonicalEl && canonicalEl.href) ||
pickMeta(document, 'meta[property="og:url"]') ||
finalUrl;
try {
canonical = new URL(canonical, finalUrl).href;
} catch (e) {
canonical = finalUrl;
}
let topImageUrl = '';
const ogImage = pickMeta(
document,
'meta[property="og:image"]',
'meta[property="og:image:url"]',
'meta[name="twitter:image"]'
);
if (ogImage) {
try {
topImageUrl = new URL(ogImage, canonical).href;
} catch (e) {
topImageUrl = '';
}
}
let article = null;
try {
article = new Readability(document).parse();
} catch (e) {
article = null;
}
const title =
(article && article.title && article.title.trim()) ||
pickMeta(document, 'meta[property="og:title"]') ||
(document.title || '').trim();
const summary =
(article && article.textContent && article.textContent.trim()) ||
pickMeta(
document,
'meta[property="og:description"]',
'meta[name=description]'
);
return new ScrapResult({
canonical,
title: title || undefined,
summary: summary || undefined,
topImageUrl: topImageUrl || undefined,
html,
status,
});
} finally {
dom.window.close();
}| if (!res.ok) { | ||
| return new ScrapResult({ canonical: finalUrl, status }); | ||
| } | ||
|
|
||
| const ct = (res.headers.get('content-type') || '').toLowerCase(); | ||
| if (!ct.startsWith('text/html') && !ct.startsWith('application/xhtml')) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
When returning early from extractStatic (either due to a non-OK status or a non-HTML content type), the response body stream must be resumed/consumed to release the underlying socket and prevent connection/memory leaks in node-fetch.
| if (!res.ok) { | |
| return new ScrapResult({ canonical: finalUrl, status }); | |
| } | |
| const ct = (res.headers.get('content-type') || '').toLowerCase(); | |
| if (!ct.startsWith('text/html') && !ct.startsWith('application/xhtml')) { | |
| return null; | |
| } | |
| if (!res.ok) { | |
| if (res.body) res.body.resume(); | |
| return new ScrapResult({ canonical: finalUrl, status }); | |
| } | |
| const ct = (res.headers.get('content-type') || '').toLowerCase(); | |
| if (!ct.startsWith('text/html') && !ct.startsWith('application/xhtml')) { | |
| if (res.body) res.body.resume(); | |
| return null; | |
| } |
| let res; | ||
| try { | ||
| res = await fetch(current, { method: 'HEAD', ...FETCH_OPTS }); | ||
| if (res.status === 405 || res.status === 501) { | ||
| res = await fetch(current, { method: 'GET', ...FETCH_OPTS }); | ||
| } | ||
| } catch (e) { |
There was a problem hiding this comment.
When falling back to a GET request in unshorten, the response body is never consumed. In node-fetch, leaving the response body unconsumed can leak sockets and memory. Resuming the body stream ensures resources are freed.
| let res; | |
| try { | |
| res = await fetch(current, { method: 'HEAD', ...FETCH_OPTS }); | |
| if (res.status === 405 || res.status === 501) { | |
| res = await fetch(current, { method: 'GET', ...FETCH_OPTS }); | |
| } | |
| } catch (e) { | |
| let res; | |
| try { | |
| res = await fetch(current, { method: 'HEAD', ...FETCH_OPTS }); | |
| if (res.status === 405 || res.status === 501) { | |
| res = await fetch(current, { method: 'GET', ...FETCH_OPTS }); | |
| } | |
| if (res.body) res.body.resume(); | |
| } catch (e) { | |
| if (fetchResult.isIncomplete) { | ||
| fetchResult.merge(await limit(() => scrap(unshortened))); | ||
| if (isTerminalHttpError(finalStatus)) { | ||
| fetchResult.status = finalStatus; | ||
| } else { | ||
| const staticResult = await extractStatic(unshortened); | ||
| if (staticResult) fetchResult.merge(staticResult); | ||
|
|
||
| if (fetchResult.isIncomplete) { | ||
| fetchResult.merge(await limit(() => scrap(unshortened))); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
If extractStatic throws a ResolveError (e.g., due to a timeout or temporary network issue), the entire URL resolution will fail immediately. Wrapping it in a try-catch block allows the system to gracefully fall back to the more robust scrap (puppeteer) layer.
if (fetchResult.isIncomplete) {
if (isTerminalHttpError(finalStatus)) {
fetchResult.status = finalStatus;
} else {
let staticResult = null;
try {
staticResult = await extractStatic(unshortened);
} catch (e) {
// eslint-disable-next-line no-console
console.error('[extractStatic]', unshortened, e);
}
if (staticResult) fetchResult.merge(staticResult);
if (fetchResult.isIncomplete) {
fetchResult.merge(await limit(() => scrap(unshortened)));
}
}
}
Base:
feat/cloudflare-browser-rendering— merge that PR first.Why
Booting puppeteer for every URL costs ~500 MB of Chromium overhead and 5–20× the wall-clock time of a plain HTTP fetch. For SSR-rendered pages,
parseMetaalready misses on rich content, but the article body is sitting right there in the HTML — we just need to parse it. Some URLs also can't possibly benefit from rendering:5xx,401,403,410,451won't return content no matter how long Chromium runs.This PR inserts a cheaper extraction layer between
parseMetaandscrap, and short-circuits puppeteer for cases where rendering can't recover.Changes
Pipeline (in order, cheapest first)
unshorten— now follows multi-hop redirect chainsparseMeta— unchangedextractStatic(new) —jsdom+@mozilla/readabilityserver-sidescrap— puppeteer fallback, skipped on terminal HTTP errorsEach layer either fills the result or falls through to the next.
extractStatic(src/lib/extractStatic.js)node-fetchHTML →jsdom→Readability.parse()ScrapResultwhen Readability succeedsnullon non-HTML responses or jsdom parse errors so the puppeteer fallback can still runResolveErroron network failures (same surface asunshorten)Adjacent fixes
unshortenmulti-hop — followed chains likebit.ly → lihi → real URLonly one hop. Now loops up to 5 hops with cycle detection, retriesHEADwithGETon405/501, resolves relativeLocationheaders, and sends aCofactsBotUA so shorteners gated by default-UA filters cooperate. Return shape is now{ url, status }so callers can act on the final response status.parseMetais incomplete and the upstream returned5xx/401/403/410/451, surface the status to the caller instead of paying for a render that can't recover.404still flows through because some sites serve soft-404s with valid OG metadata.Bench harness (
bench/)node --expose-gc bench/compare.jsruns both methods on a URL list and emits a per-URL CSV (status, elapsed_ms, RSS delta, V8 heap delta, puppeteer per-page JSHeapUsedSize) plus a method-level summary. The headline question it answers: what fraction of URLs are extractable by the static path alone? That fraction is the multiplier on how much puppeteer concurrency we can shrink in prod, and the ~500 MB Chromium overhead can scale down proportionally.Memory caveats and recommended methodology (separate processes for clean baselines) are documented in
bench/README.md.bench/results/is gitignored.New direct dependencies
package-lock.jsonwas regenerated withnpm install --before=2026-05-09so all transitive deps remain ≥30 days old, matching the supply-chain convention used by the upgrade-deps PR.Tests
npm test— 60/60 pass (was 35/35 on cloudflare base; +25 from newextractStatictests and extendedunshorten/resolveUrlstests)npx eslint index.js docker-test.js src/— cleanextractStatic.test.jsmocksjsdomand@mozilla/readabilitybecause jsdom has ESM-only transitive deps that don't load under jest 23. Real-network integration is exercised bybench/compare.js.Commits
fix(unshorten): follow multi-hop redirects and HEAD-reject serversperf(resolver): skip puppeteer for terminal HTTP errorsfeat(resolver): add server-side jsdom + Readability extractionchore(bench): compare jsdom + puppeteer extraction