Skip to content

Commit

Permalink
Ensure timed out prefetches are cleaned up correctly (#28899)
Browse files Browse the repository at this point in the history
This applies the fix from the awesome investigation done in #28797 by @jayphelps and adds a test to ensure this is working as expected. It seems that the `route-loader` has a race condition while prefetching and if a script is executed before we have created a current "future" entry to resolve the entry stays in a pending state causing routes to hang so this handles the condition by ensuring pending/errored entries do not stay around. 

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have helpful link attached, see `contributing.md`

Fixes: #28797
Fixes: #27783
  • Loading branch information
ijjk committed Sep 8, 2021
1 parent 4f8d883 commit b71df19
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 18 deletions.
9 changes: 7 additions & 2 deletions packages/next/client/route-loader.ts
Expand Up @@ -60,8 +60,13 @@ function withFuture<T>(
})
map.set(key, (entry = { resolve: resolver!, future: prom }))
return generator
? // eslint-disable-next-line no-sequences
generator().then((value) => (resolver(value), value))
? generator()
// eslint-disable-next-line no-sequences
.then((value) => (resolver(value), value))
.catch((err) => {
map.delete(key)
throw err
})
: prom
}

Expand Down
2 changes: 1 addition & 1 deletion test/integration/preload-viewport/pages/another.js
@@ -1 +1 @@
export default () => <p>Hello world</p>
export default () => <p id="another">Hello world</p>
69 changes: 55 additions & 14 deletions test/integration/preload-viewport/test/index.test.js
@@ -1,37 +1,77 @@
/* eslint-env jest */

import {
nextServer,
runNextCommand,
startApp,
stopApp,
findPort,
killApp,
nextBuild,
nextStart,
waitFor,
} from 'next-test-utils'
import http from 'http'
import httpProxy from 'http-proxy'
import webdriver from 'next-webdriver'
import { join } from 'path'
import { readFile } from 'fs-extra'

jest.setTimeout(1000 * 60 * 5)

const appDir = join(__dirname, '../')
let appPort
let server
let app
let appPort
let stallJs
let proxyServer

describe('Prefetching Links in viewport', () => {
beforeAll(async () => {
await runNextCommand(['build', appDir])
await nextBuild(appDir)
const port = await findPort()
app = await nextStart(appDir, port)
appPort = await findPort()

const proxy = httpProxy.createProxyServer({
target: `http://localhost:${port}`,
})

proxyServer = http.createServer(async (req, res) => {
if (stallJs && req.url.includes('chunks/pages/another')) {
console.log('stalling request for', req.url)
await new Promise((resolve) => setTimeout(resolve, 5 * 1000))
}
proxy.web(req, res)
})

proxy.on('error', (err) => {
console.warn('Failed to proxy', err)
})

app = nextServer({
dir: join(__dirname, '../'),
dev: false,
quiet: true,
await new Promise((resolve) => {
proxyServer.listen(appPort, () => resolve())
})
})
afterAll(async () => {
await killApp(app)
proxyServer.close()
})

server = await startApp(app)
appPort = server.address().port
it('should handle timed out prefetch correctly', async () => {
try {
stallJs = true
const browser = await webdriver(appPort, '/')

await browser.elementByCss('#scroll-to-another').click()
// wait for preload to timeout
await waitFor(6 * 1000)

await browser
.elementByCss('#link-another')
.click()
.waitForElementByCss('#another')

expect(await browser.elementByCss('#another').text()).toBe('Hello world')
} finally {
stallJs = false
}
})
afterAll(() => stopApp(server))

it('should prefetch with link in viewport onload', async () => {
let browser
Expand Down Expand Up @@ -280,6 +320,7 @@ describe('Prefetching Links in viewport', () => {

expect(await browser.eval('window.hadUnhandledReject')).toBeFalsy()

await browser.waitForElementByCss('#invalid-link')
await browser.elementByCss('#invalid-link').moveTo()
expect(await browser.eval('window.hadUnhandledReject')).toBeFalsy()
})
Expand Down
1 change: 0 additions & 1 deletion test/integration/route-load-cancel-css/test/index.test.js
Expand Up @@ -20,7 +20,6 @@ const appDir = join(__dirname, '../')
function runTests() {
it('should cancel slow page loads on re-navigation', async () => {
const browser = await webdriver(appPort, '/')
await waitFor(5000)

await browser.elementByCss('#link-1').click()
await waitFor(3000)
Expand Down

0 comments on commit b71df19

Please sign in to comment.