Skip to content

Commit

Permalink
Merge branch 'canary' into fix/upload-binary-artifact
Browse files Browse the repository at this point in the history
  • Loading branch information
kodiakhq[bot] committed Sep 13, 2021
2 parents b80c47a + 41484f6 commit 65a7351
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 1 deletion.
18 changes: 18 additions & 0 deletions errors/gssp-no-mutating-res
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Must not access ServerResponse after getServerSideProps() resolves

#### Why This Error Occurred

`getServerSideProps()` surfaces a `ServerResponse` object through the `res` property of its `context` arg. This object is not intended to be accessed or changed after `getServerSideProps()` resolves.

This is because the framework tries to optimize when items like headers or status codes are flushed to the browser. If they are changed after `getServerSideProps()` completes, we can't guarantee that the changes will work.

For this reason, accessing the object after this time is disallowed.


#### Possible Ways to Fix It

You can fix this error by moving any access of the `res` object into `getServerSideProps()` itself or any functions that run before `getServerSideProps()` returns.

### Useful Links

- [Data Fetching Docs](https://nextjs.org/docs/basic-features/data-fetching)
4 changes: 4 additions & 0 deletions errors/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@
"title": "gssp-mixed-not-found-redirect",
"path": "/errors/gssp-mixed-not-found-redirect.md"
},
{
"title": "gssp-no-mutating-res",
"path": "/errors/gssp-no-mutating-res.md"
},
{ "title": "head-build-id", "path": "/errors/head-build-id.md" },
{
"title": "href-interpolation-failed",
Expand Down
19 changes: 18 additions & 1 deletion packages/next/server/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -742,12 +742,28 @@ export async function renderToHTML(
if (getServerSideProps && !isFallback) {
let data: UnwrapPromise<ReturnType<GetServerSideProps>>

let canAccessRes = true
let resOrProxy = res
if (process.env.NODE_ENV !== 'production') {
resOrProxy = new Proxy<ServerResponse>(res, {
get: function (obj, prop, receiver) {
if (!canAccessRes) {
throw new Error(
`You should not access 'res' after getServerSideProps resolves.` +
`\nRead more: https://nextjs.org/docs/messages/gssp-no-mutating-res`
)
}
return Reflect.get(obj, prop, receiver)
},
})
}

try {
data = await getServerSideProps({
req: req as IncomingMessage & {
cookies: NextApiRequestCookies
},
res,
res: resOrProxy,
query,
resolvedUrl: renderOpts.resolvedUrl as string,
...(pageIsDynamic ? { params: params as ParsedUrlQuery } : undefined),
Expand All @@ -758,6 +774,7 @@ export async function renderToHTML(
locale: renderOpts.locale,
defaultLocale: renderOpts.defaultLocale,
})
canAccessRes = false
} catch (serverSidePropsError) {
// remove not found error code to prevent triggering legacy
// 404 rendering
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
export async function getServerSideProps(context) {
return {
props: (async function () {
// Mimic some async work, like getting data from an API
await new Promise((resolve) => setTimeout(resolve))
context.res.setHeader('test-header', 'this is a test header')
return {
text: 'res',
}
})(),
}
}

export default ({ text }) => {
return (
<>
<div>hello {text}</div>
</>
)
}
22 changes: 22 additions & 0 deletions test/integration/getserversideprops/pages/promise/mutate-res.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
let mutatedRes

export async function getServerSideProps(context) {
mutatedRes = context.res
return {
props: (async function () {
return {
text: 'res',
}
})(),
}
}

export default ({ text }) => {
mutatedRes.setHeader('test-header', 'this is a test header')

return (
<>
<div>hello {text}</div>
</>
)
}
33 changes: 33 additions & 0 deletions test/integration/getserversideprops/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,20 @@ const expectedManifestRoutes = () => [
),
page: '/promise',
},
{
dataRouteRegex: normalizeRegEx(
`^\\/_next\\/data\\/${escapeRegex(buildId)}\\/promise\\/mutate-res.json$`
),
page: '/promise/mutate-res',
},
{
dataRouteRegex: normalizeRegEx(
`^\\/_next\\/data\\/${escapeRegex(
buildId
)}\\/promise\\/mutate-res-props.json$`
),
page: '/promise/mutate-res-props',
},
{
dataRouteRegex: normalizeRegEx(
`^\\/_next\\/data\\/${escapeRegex(buildId)}\\/refresh.json$`
Expand Down Expand Up @@ -710,6 +724,20 @@ const runTests = (dev = false) => {
/Error serializing `.time` returned from `getServerSideProps`/
)
})

it('should show error for accessing res after gssp returns', async () => {
const html = await renderViaHTTP(appPort, '/promise/mutate-res')
expect(html).toContain(
`You should not access 'res' after getServerSideProps resolves`
)
})

it('should show error for accessing res through props promise after gssp returns', async () => {
const html = await renderViaHTTP(appPort, '/promise/mutate-res-props')
expect(html).toContain(
`You should not access 'res' after getServerSideProps resolves`
)
})
} else {
it('should not fetch data on mount', async () => {
const browser = await webdriver(appPort, '/blog/post-100')
Expand Down Expand Up @@ -767,6 +795,11 @@ const runTests = (dev = false) => {
await browser.elementByCss('#non-json').click()
await check(() => getBrowserBodyText(browser), /hello /)
})

it('should not show error for accessing res after gssp returns', async () => {
const html = await renderViaHTTP(appPort, '/promise/mutate-res')
expect(html).toMatch(/hello.*?res/)
})
}
}

Expand Down

0 comments on commit 65a7351

Please sign in to comment.