-
Notifications
You must be signed in to change notification settings - Fork 12
Open
Description
Detail Bug Report
Summary
- Context: The
prerenderfunction insrc/index.jsattempts to render a webpage using a headless browser (puppeteer/browserless), while simultaneously fetching the page with a simple HTTP request as a fallback mechanism. - Bug: When both prerender and fetch operations fail, the returned response object is missing the
statusCodeandredirectsfields that are present in all other code paths. - Actual vs. expected: The function returns
{ headers, html, url, mode }instead of{ headers, html, url, mode, statusCode, redirects }, creating an inconsistent API surface. - Impact: Consumer code that expects
statusCodeto always be present (e.g., for error handling likeif (result.statusCode >= 400)) will receiveundefined, potentially causing runtime errors or incorrect behavior.
Code with Bug
return isFetchResRejected
? {
headers: data.headers || {},
html: '',
url,
mode: 'prerender'
// <-- BUG 🔴 missing `statusCode` and `redirects` fields in this failure path
}
: dataExplanation
In prerender, browserless rendering and a parallel fetch fallback run together. If browserless fails, the function falls back to the fetch result. However, when the fetch result is also rejected (reflect: true case), the fallback return constructs a partial object and omits statusCode and redirects.
This is inconsistent with:
fetcherror returns, which includestatusCodeandredirects- success returns from both
fetchandprerender, which includestatusCode(andredirectsin newer code)
Codebase Inconsistency
fetch error handling (non-reflect) always includes these fields:
return reflect
? { isRejected: true, error }
: {
url,
html: '',
mode: 'fetch',
headers: error.response ? error.response.headers : {},
statusCode: error.response ? error.response.statusCode : undefined,
redirects
}Existing tests also assume statusCode is always present:
t.is(prerenderDisabled.statusCode, prerenderEnabled.statusCode)
t.is(prerenderDisabled.statusCode, 200)Recommended Fix
Include statusCode and redirects in the isFetchResRejected fallback return so the return shape is consistent:
return isFetchResRejected
? {
headers: data.headers || {},
html: '',
url,
mode: 'prerender',
statusCode: data.error?.response?.statusCode,
redirects: []
}
: dataHistory
This bug was introduced in commit b66c1e0. The bug was later compounded in commit 54cfe61 when the redirects field was added to all paths except this same overlooked error fallback.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels