From 5d779a028942b4ada0e25d45ea7952447361a322 Mon Sep 17 00:00:00 2001 From: JJ Kasper <22380829+ijjk@users.noreply.github.com> Date: Fri, 15 Feb 2019 15:22:21 -0600 Subject: [PATCH] Add falling back to fetch based pinging for onDemandEntries (#6310) After discussion, I added falling back to fetch based pinging when the WebSocket fails to connect. I also added an example of how to proxy the onDemandEntries WebSocket when using a custom server. Fixes: #6296 --- ...on-demand-entries-websocket-unavailable.md | 29 +++++++ .../custom-server-proxy-websocket/README.md | 38 ++++++++ .../custom-server-proxy-websocket/genSSL.sh | 7 ++ .../next.config.js | 6 ++ .../package.json | 21 +++++ .../pages/another.js | 10 +++ .../pages/index.js | 10 +++ .../custom-server-proxy-websocket/server.js | 43 +++++++++ .../next/client/on-demand-entries-client.js | 51 +++++++++-- .../next/server/on-demand-entry-handler.js | 87 ++++++++++++------- test/integration/ondemand/test/index.test.js | 8 ++ 11 files changed, 270 insertions(+), 40 deletions(-) create mode 100644 errors/on-demand-entries-websocket-unavailable.md create mode 100644 examples/custom-server-proxy-websocket/README.md create mode 100755 examples/custom-server-proxy-websocket/genSSL.sh create mode 100644 examples/custom-server-proxy-websocket/next.config.js create mode 100644 examples/custom-server-proxy-websocket/package.json create mode 100644 examples/custom-server-proxy-websocket/pages/another.js create mode 100644 examples/custom-server-proxy-websocket/pages/index.js create mode 100644 examples/custom-server-proxy-websocket/server.js diff --git a/errors/on-demand-entries-websocket-unavailable.md b/errors/on-demand-entries-websocket-unavailable.md new file mode 100644 index 00000000..1b141561 --- /dev/null +++ b/errors/on-demand-entries-websocket-unavailable.md @@ -0,0 +1,29 @@ +# onDemandEntries WebSocket unavailable + +#### Why This Error Occurred + +By default Next.js uses a random port to create a WebSocket to receive pings from the client letting it know to keep pages active. For some reason when the client tried to connect to this WebSocket the connection fails. + +#### Possible Ways to Fix It + +If you don't mind the fetch requests in your network console then you don't have to do anything as the fallback to fetch works fine. If you do, then depending on your set up you might need configure settings using the below config options from `next.config.js`: + +```js +module.exports = { + onDemandEntries: { + // optionally configure a port for the onDemandEntries WebSocket, not needed by default + websocketPort: 3001, + // optionally configure a proxy path for the onDemandEntries WebSocket, not need by default + websocketProxyPath: '/hmr', + // optionally configure a proxy port for the onDemandEntries WebSocket, not need by default + websocketProxyPort: 7002, + }, +} +``` + +If you are using a custom server with SSL configured, you might want to take a look at [the example](https://github.com/zeit/next.js/tree/canary/examples/custom-server-proxy-websocket) showing how to proxy the WebSocket connection through your custom server + +### Useful Links + +- [onDemandEntries config](https://github.com/zeit/next.js#configuring-the-ondemandentries) +- [Custom server proxying example](https://github.com/zeit/next.js/tree/canary/examples/custom-server-proxy-websocket) \ No newline at end of file diff --git a/examples/custom-server-proxy-websocket/README.md b/examples/custom-server-proxy-websocket/README.md new file mode 100644 index 00000000..58d29fc6 --- /dev/null +++ b/examples/custom-server-proxy-websocket/README.md @@ -0,0 +1,38 @@ +# Custom server with Proxying onDemandEntries WebSocket + +## How to use + +### Using `create-next-app` + +Execute [`create-next-app`](https://github.com/segmentio/create-next-app) with [Yarn](https://yarnpkg.com/lang/en/docs/cli/create/) or [npx](https://github.com/zkat/npx#readme) to bootstrap the example: + +```bash +npx create-next-app --example custom-server-proxy-websocket custom-server-proxy-websocket +# or +yarn create next-app --example custom-server-proxy-websocket custom-server-proxy-websocket +``` + +### Download manually + +Download the example: + +```bash +curl https://codeload.github.com/zeit/next.js/tar.gz/canary | tar -xz --strip=2 next.js-canary/examples/custom-server-proxy-websocket +cd custom-server-proxy-websocket +``` + +Install it and run: + +```bash +npm install +npm run ssl +npm run dev +# or +yarn +yarn ssl +yarn dev +``` + +## The idea behind the example + +The example shows how you can use SSL with a custom server and still use onDemandEntries WebSocket from Next.js using [node-http-proxy](https://github.com/nodejitsu/node-http-proxy#readme) and [ExpressJS](https://github.com/expressjs/express). diff --git a/examples/custom-server-proxy-websocket/genSSL.sh b/examples/custom-server-proxy-websocket/genSSL.sh new file mode 100755 index 00000000..453aac5a --- /dev/null +++ b/examples/custom-server-proxy-websocket/genSSL.sh @@ -0,0 +1,7 @@ +#!/bin/sh + +# Generate self-signed certificate (only meant for testing don't use in production...) +# requires openssl be installed and in the $PATH + +openssl genrsa -out localhost.key 2048 +openssl req -new -x509 -key localhost.key -out localhost.cert -days 3650 -subj /CN=localhost diff --git a/examples/custom-server-proxy-websocket/next.config.js b/examples/custom-server-proxy-websocket/next.config.js new file mode 100644 index 00000000..6c128b13 --- /dev/null +++ b/examples/custom-server-proxy-websocket/next.config.js @@ -0,0 +1,6 @@ +module.exports = { + onDemandEntries: { + websocketPort: 3001, + websocketProxyPort: 3000 + } +} diff --git a/examples/custom-server-proxy-websocket/package.json b/examples/custom-server-proxy-websocket/package.json new file mode 100644 index 00000000..aa0e4805 --- /dev/null +++ b/examples/custom-server-proxy-websocket/package.json @@ -0,0 +1,21 @@ +{ + "name": "custom-server-proxy-websocket", + "version": "1.0.0", + "main": "server.js", + "license": "MIT", + "scripts": { + "dev": "node server.js", + "build": "next build", + "ssl": "./genSSL.sh", + "start": "NODE_ENV=production node server.js" + }, + "dependencies": { + "express": "4.16.4", + "next": "8.0.1", + "react": "16.8.2", + "react-dom": "16.8.2" + }, + "devDependencies": { + "http-proxy": "1.17.0" + } +} diff --git a/examples/custom-server-proxy-websocket/pages/another.js b/examples/custom-server-proxy-websocket/pages/another.js new file mode 100644 index 00000000..c5c0ffa4 --- /dev/null +++ b/examples/custom-server-proxy-websocket/pages/another.js @@ -0,0 +1,10 @@ +import Link from 'next/link' + +export default () => ( +
+

Another

+ + Index + +
+) diff --git a/examples/custom-server-proxy-websocket/pages/index.js b/examples/custom-server-proxy-websocket/pages/index.js new file mode 100644 index 00000000..17aa70c7 --- /dev/null +++ b/examples/custom-server-proxy-websocket/pages/index.js @@ -0,0 +1,10 @@ +import Link from 'next/link' + +export default () => ( +
+

Index

+ + Another + +
+) diff --git a/examples/custom-server-proxy-websocket/server.js b/examples/custom-server-proxy-websocket/server.js new file mode 100644 index 00000000..79b356d9 --- /dev/null +++ b/examples/custom-server-proxy-websocket/server.js @@ -0,0 +1,43 @@ +const express = require('express') +const Next = require('next') +const https = require('https') +const fs = require('fs') +const app = express() +const port = 3000 +const isDev = process.env.NODE_ENV !== 'production' +const next = Next({ dev: isDev }) + +// Set up next +next.prepare() + +// Set up next handler +app.use(next.getRequestHandler()) + +// Set up https.Server options with SSL +const options = { + key: fs.readFileSync('./localhost.key'), + cert: fs.readFileSync('./localhost.cert') +} + +// Create http server using express app as requestHandler +const server = https.createServer(options, app) + +// Set up proxying for Next's onDemandEntries WebSocket to allow +// using our SSL +if (isDev) { + const CreateProxyServer = require('http-proxy').createProxyServer + const proxy = CreateProxyServer({ + target: { + host: 'localhost', + port: 3001 + } + }) + + server.on('upgrade', (req, socket, head) => { + proxy.ws(req, socket, head) + }) +} + +server.listen(port, () => { + console.log(`Server listening at http://localhost:${port}`) +}) diff --git a/packages/next/client/on-demand-entries-client.js b/packages/next/client/on-demand-entries-client.js index 2f67e3df..eb56ce1f 100644 --- a/packages/next/client/on-demand-entries-client.js +++ b/packages/next/client/on-demand-entries-client.js @@ -8,25 +8,37 @@ const wsProtocol = protocol.includes('https') ? 'wss' : 'ws' const retryTime = 5000 let ws = null let lastHref = null +let wsConnectTries = 0 +let showedWarning = false export default async ({ assetPrefix }) => { Router.ready(() => { Router.events.on('routeChangeComplete', ping) }) - const setup = async (reconnect) => { + const setup = async () => { if (ws && ws.readyState === ws.OPEN) { return Promise.resolve() + } else if (wsConnectTries > 1) { + return } + wsConnectTries++ return new Promise(resolve => { ws = new WebSocket(`${wsProtocol}://${hostname}:${process.env.__NEXT_WS_PORT}${process.env.__NEXT_WS_PROXY_PATH}`) - ws.onopen = () => resolve() + ws.onopen = () => { + wsConnectTries = 0 + resolve() + } ws.onclose = () => { setTimeout(async () => { - // check if next restarted and we have to reload to get new port await fetch(`${assetPrefix}/_next/on-demand-entries-ping`) - .then(res => res.status === 200 && location.reload()) + .then(res => { + // Only reload if next was restarted and we have a new WebSocket port + if (res.status === 200 && res.headers.get('port') !== process.env.__NEXT_WS_PORT + '') { + location.reload() + } + }) .catch(() => {}) await setup(true) resolve() @@ -49,11 +61,36 @@ export default async ({ assetPrefix }) => { } }) } - await setup() + setup() async function ping () { - if (ws.readyState === ws.OPEN) { - ws.send(Router.pathname) + // Use WebSocket if available + if (ws && ws.readyState === ws.OPEN) { + return ws.send(Router.pathname) + } + if (!showedWarning) { + console.warn('onDemandEntries WebSocket failed to connect, falling back to fetch based pinging. https://err.sh/zeit/next.js/on-demand-entries-websocket-unavailable') + showedWarning = true + } + // If not, fallback to fetch based pinging + try { + const url = `${assetPrefix || ''}/_next/on-demand-entries-ping?page=${Router.pathname}` + const res = await fetch(url, { + credentials: 'same-origin' + }) + const payload = await res.json() + if (payload.invalid) { + // Payload can be invalid even if the page does not exist. + // So, we need to make sure it exists before reloading. + const pageRes = await fetch(location.href, { + credentials: 'same-origin' + }) + if (pageRes.status === 200) { + location.reload() + } + } + } catch (err) { + console.error(`Error with on-demand-entries-ping: ${err.message}`) } } diff --git a/packages/next/server/on-demand-entry-handler.js b/packages/next/server/on-demand-entry-handler.js index 5976bf28..97476f0d 100644 --- a/packages/next/server/on-demand-entry-handler.js +++ b/packages/next/server/on-demand-entry-handler.js @@ -1,6 +1,7 @@ import DynamicEntryPlugin from 'webpack/lib/DynamicEntryPlugin' import { EventEmitter } from 'events' import { join } from 'path' +import {parse} from 'url' import fs from 'fs' import promisify from '../lib/promisify' import globModule from 'glob' @@ -152,6 +153,40 @@ export default function onDemandEntryHandler (devMiddleware, multiCompiler, { reloadCallbacks = null } + function handlePing (pg, socket) { + const page = normalizePage(pg) + const entryInfo = entries[page] + + // If there's no entry. + // Then it seems like an weird issue. + if (!entryInfo) { + const message = `Client pings, but there's no entry for page: ${page}` + console.error(message) + return sendJson(socket, { invalid: true }) + } + + // 404 is an on demand entry but when a new page is added we have to refresh the page + if (page === '/_error') { + sendJson(socket, { invalid: true }) + } else { + sendJson(socket, { success: true }) + } + + // We don't need to maintain active state of anything other than BUILT entries + if (entryInfo.status !== BUILT) return + + // If there's an entryInfo + if (!lastAccessPages.includes(page)) { + lastAccessPages.unshift(page) + + // Maintain the buffer max length + if (lastAccessPages.length > pagesBufferLength) { + lastAccessPages.pop() + } + } + entryInfo.lastActiveTime = Date.now() + } + return { waitUntilReloaded () { if (!reloading) return Promise.resolve(true) @@ -225,37 +260,8 @@ export default function onDemandEntryHandler (devMiddleware, multiCompiler, { wsConnection (ws) { ws.onmessage = ({ data }) => { - const page = normalizePage(data) - const entryInfo = entries[page] - - // If there's no entry. - // Then it seems like an weird issue. - if (!entryInfo) { - const message = `Client pings, but there's no entry for page: ${page}` - console.error(message) - return sendJson(ws, { invalid: true }) - } - - // 404 is an on demand entry but when a new page is added we have to refresh the page - if (page === '/_error') { - sendJson(ws, { invalid: true }) - } else { - sendJson(ws, { success: true }) - } - - // We don't need to maintain active state of anything other than BUILT entries - if (entryInfo.status !== BUILT) return - - // If there's an entryInfo - if (!lastAccessPages.includes(page)) { - lastAccessPages.unshift(page) - - // Maintain the buffer max length - if (lastAccessPages.length > pagesBufferLength) { - lastAccessPages.pop() - } - } - entryInfo.lastActiveTime = Date.now() + // `data` should be the page here + handlePing(data, ws) } }, @@ -280,6 +286,12 @@ export default function onDemandEntryHandler (devMiddleware, multiCompiler, { } else { if (!/^\/_next\/on-demand-entries-ping/.test(req.url)) return next() + const { query } = parse(req.url, true) + + if (query.page) { + return handlePing(query.page, res) + } + res.statusCode = 200 res.setHeader('port', wsPort) res.end('200') @@ -328,8 +340,17 @@ export function normalizePage (page) { return unixPagePath.replace(/\/index$/, '') } -function sendJson (ws, data) { - ws.send(JSON.stringify(data)) +function sendJson (socket, data) { + data = JSON.stringify(data) + + // Handle fetch request + if (socket.setHeader) { + socket.setHeader('content-type', 'application/json') + socket.status = 200 + return socket.end(data) + } + // Should be WebSocket so just send + socket.send(data) } // Make sure only one invalidation happens at a time diff --git a/test/integration/ondemand/test/index.test.js b/test/integration/ondemand/test/index.test.js index bc48afe4..e078d7a5 100644 --- a/test/integration/ondemand/test/index.test.js +++ b/test/integration/ondemand/test/index.test.js @@ -103,4 +103,12 @@ describe('On Demand Entries', () => { } } }) + + it('should able to ping using fetch fallback', async () => { + const about = await renderViaHTTP(context.appPort, '/_next/on-demand-entries-ping', {page: '/about'}) + expect(JSON.parse(about)).toEqual({success: true}) + + const third = await renderViaHTTP(context.appPort, '/_next/on-demand-entries-ping', {page: '/third'}) + expect(JSON.parse(third)).toEqual({success: true}) + }) })