From 7f9f8c6851d1eb49a72dcb6c134873148a2e81eb Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Fri, 25 Oct 2024 13:54:11 +0900 Subject: [PATCH] fix: use websocket to test server liveness before client reload (#17891) Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com> --- packages/vite/src/client/client.ts | 40 +++++----- packages/vite/src/node/server/ws.ts | 39 +++++++++- .../__tests__/client-reload.spec.ts | 75 +++++++++++++++++++ playground/client-reload/__tests__/serve.ts | 6 ++ playground/client-reload/index.html | 4 + playground/client-reload/package.json | 12 +++ playground/client-reload/vite.config.ts | 5 ++ playground/js-sourcemap/test-ssr-dev.js | 1 + playground/ssr-html/test-network-imports.js | 1 + .../ssr-html/test-stacktrace-runtime.js | 1 + playground/ssr-html/test-stacktrace.js | 1 + playground/test-utils.ts | 5 ++ pnpm-lock.yaml | 2 + 13 files changed, 170 insertions(+), 22 deletions(-) create mode 100644 playground/client-reload/__tests__/client-reload.spec.ts create mode 100644 playground/client-reload/__tests__/serve.ts create mode 100644 playground/client-reload/index.html create mode 100644 playground/client-reload/package.json create mode 100644 playground/client-reload/vite.config.ts diff --git a/packages/vite/src/client/client.ts b/packages/vite/src/client/client.ts index 3563454f7..d51dd73c6 100644 --- a/packages/vite/src/client/client.ts +++ b/packages/vite/src/client/client.ts @@ -331,24 +331,28 @@ async function waitForSuccessfulPing( hostAndPath: string, ms = 1000, ) { - const pingHostProtocol = socketProtocol === 'wss' ? 'https' : 'http' - - const ping = async () => { - // A fetch on a websocket URL will return a successful promise with status 400, - // but will reject a networking error. - // When running on middleware mode, it returns status 426, and a cors error happens if mode is not no-cors - try { - await fetch(`${pingHostProtocol}://${hostAndPath}`, { - mode: 'no-cors', - headers: { - // Custom headers won't be included in a request with no-cors so (ab)use one of the - // safelisted headers to identify the ping request - Accept: 'text/x-vite-ping', - }, - }) - return true - } catch {} - return false + async function ping() { + const socket = new WebSocket( + `${socketProtocol}://${hostAndPath}`, + 'vite-ping', + ) + return new Promise((resolve) => { + function onOpen() { + resolve(true) + close() + } + function onError() { + resolve(false) + close() + } + function close() { + socket.removeEventListener('open', onOpen) + socket.removeEventListener('error', onError) + socket.close() + } + socket.addEventListener('open', onOpen) + socket.addEventListener('error', onError) + }) } if (await ping()) { diff --git a/packages/vite/src/node/server/ws.ts b/packages/vite/src/node/server/ws.ts index e54e061b5..c68ba6ce3 100644 --- a/packages/vite/src/node/server/ws.ts +++ b/packages/vite/src/node/server/ws.ts @@ -133,7 +133,9 @@ export function createWebSocketServer( wss = new WebSocketServerRaw({ noServer: true }) hmrServerWsListener = (req, socket, head) => { if ( - req.headers['sec-websocket-protocol'] === HMR_HEADER && + [HMR_HEADER, 'vite-ping'].includes( + req.headers['sec-websocket-protocol']!, + ) && req.url === hmrBase ) { wss.handleUpgrade(req, socket as Socket, head, (ws) => { @@ -157,17 +159,46 @@ export function createWebSocketServer( }) res.end(body) }) as Parameters[1] + // vite dev server in middleware mode + // need to call ws listen manually if (httpsOptions) { wsHttpServer = createHttpsServer(httpsOptions, route) } else { wsHttpServer = createHttpServer(route) } - // vite dev server in middleware mode - // need to call ws listen manually - wss = new WebSocketServerRaw({ server: wsHttpServer }) + wss = new WebSocketServerRaw({ noServer: true }) + wsHttpServer.on('upgrade', (req, socket, head) => { + const protocol = req.headers['sec-websocket-protocol']! + if (protocol === 'vite-ping' && server && !server.listening) { + // reject connection to tell the vite/client that the server is not ready + // if the http server is not listening + // because the ws server listens before the http server listens + req.destroy() + return + } + wss.handleUpgrade(req, socket as Socket, head, (ws) => { + wss.emit('connection', ws, req) + }) + }) + wsHttpServer.on('error', (e: Error & { code: string }) => { + if (e.code === 'EADDRINUSE') { + config.logger.error( + colors.red(`WebSocket server error: Port is already in use`), + { error: e }, + ) + } else { + config.logger.error( + colors.red(`WebSocket server error:\n${e.stack || e.message}`), + { error: e }, + ) + } + }) } wss.on('connection', (socket) => { + if (socket.protocol === 'vite-ping') { + return + } socket.on('message', (raw) => { if (!customListeners.size) return let parsed: any diff --git a/playground/client-reload/__tests__/client-reload.spec.ts b/playground/client-reload/__tests__/client-reload.spec.ts new file mode 100644 index 000000000..0872a2e17 --- /dev/null +++ b/playground/client-reload/__tests__/client-reload.spec.ts @@ -0,0 +1,75 @@ +import path from 'node:path' +import { type ServerOptions, type ViteDevServer, createServer } from 'vite' +import { afterEach, describe, expect, test } from 'vitest' +import { hmrPorts, isServe, page, ports } from '~utils' + +let server: ViteDevServer + +afterEach(async () => { + await server?.close() +}) + +async function testClientReload(serverOptions: ServerOptions) { + // start server + server = await createServer({ + root: path.resolve(import.meta.dirname, '..'), + logLevel: 'silent', + server: { + strictPort: true, + ...serverOptions, + }, + }) + + await server.listen() + const serverUrl = server.resolvedUrls.local[0] + + // open page and wait for connection + const connectedPromise = page.waitForEvent('console', { + predicate: (message) => message.text().includes('[vite] connected.'), + timeout: 5000, + }) + await page.goto(serverUrl) + await connectedPromise + + // input state + await page.locator('input').fill('hello') + + // restart and wait for reconnection after reload + const reConnectedPromise = page.waitForEvent('console', { + predicate: (message) => message.text().includes('[vite] connected.'), + timeout: 5000, + }) + await server.restart() + await reConnectedPromise + expect(await page.textContent('input')).toBe('') +} + +describe.runIf(isServe)('client-reload', () => { + test('default', async () => { + await testClientReload({ + port: ports['client-reload'], + }) + }) + + test('custom hmr port', async () => { + await testClientReload({ + port: ports['client-reload/hmr-port'], + hmr: { + port: hmrPorts['client-reload/hmr-port'], + }, + }) + }) + + test('custom hmr port and cross origin isolation', async () => { + await testClientReload({ + port: ports['client-reload/cross-origin'], + hmr: { + port: hmrPorts['client-reload/cross-origin'], + }, + headers: { + 'Cross-Origin-Embedder-Policy': 'require-corp', + 'Cross-Origin-Opener-Policy': 'same-origin', + }, + }) + }) +}) diff --git a/playground/client-reload/__tests__/serve.ts b/playground/client-reload/__tests__/serve.ts new file mode 100644 index 000000000..1d33d8064 --- /dev/null +++ b/playground/client-reload/__tests__/serve.ts @@ -0,0 +1,6 @@ +// do nothing here since server is managed inside spec +export async function serve(): Promise<{ close(): Promise }> { + return { + close: () => Promise.resolve(), + } +} diff --git a/playground/client-reload/index.html b/playground/client-reload/index.html new file mode 100644 index 000000000..7e78f23e2 --- /dev/null +++ b/playground/client-reload/index.html @@ -0,0 +1,4 @@ + +

Test Client Reload

+ + diff --git a/playground/client-reload/package.json b/playground/client-reload/package.json new file mode 100644 index 000000000..a6fa570c6 --- /dev/null +++ b/playground/client-reload/package.json @@ -0,0 +1,12 @@ +{ + "name": "@vitejs/test-client-reload", + "private": true, + "version": "0.0.0", + "type": "module", + "scripts": { + "debug": "node --inspect-brk ../../packages/vite/bin/vite", + "dev": "vite", + "build": "vite build", + "preview": "vite preview" + } +} diff --git a/playground/client-reload/vite.config.ts b/playground/client-reload/vite.config.ts new file mode 100644 index 000000000..4c9c4be6b --- /dev/null +++ b/playground/client-reload/vite.config.ts @@ -0,0 +1,5 @@ +import { defineConfig } from 'vite' + +export default defineConfig({ + server: {}, +}) diff --git a/playground/js-sourcemap/test-ssr-dev.js b/playground/js-sourcemap/test-ssr-dev.js index c414f0585..0e21a0ae8 100644 --- a/playground/js-sourcemap/test-ssr-dev.js +++ b/playground/js-sourcemap/test-ssr-dev.js @@ -12,6 +12,7 @@ async function runTest() { server: { middlewareMode: true, hmr: false, + ws: false, }, define: { __testDefineObject: '{ "hello": "test" }', diff --git a/playground/ssr-html/test-network-imports.js b/playground/ssr-html/test-network-imports.js index 6e6a87d93..d205acb12 100644 --- a/playground/ssr-html/test-network-imports.js +++ b/playground/ssr-html/test-network-imports.js @@ -8,6 +8,7 @@ async function runTest(userRunner) { root: fileURLToPath(new URL('.', import.meta.url)), server: { middlewareMode: true, + ws: false, }, }) let mod diff --git a/playground/ssr-html/test-stacktrace-runtime.js b/playground/ssr-html/test-stacktrace-runtime.js index 0f4914dcb..a53b7fbcd 100644 --- a/playground/ssr-html/test-stacktrace-runtime.js +++ b/playground/ssr-html/test-stacktrace-runtime.js @@ -10,6 +10,7 @@ const server = await createServer({ root: fileURLToPath(new URL('.', import.meta.url)), server: { middlewareMode: true, + ws: false, }, }) diff --git a/playground/ssr-html/test-stacktrace.js b/playground/ssr-html/test-stacktrace.js index 327a8b4c4..85f48a822 100644 --- a/playground/ssr-html/test-stacktrace.js +++ b/playground/ssr-html/test-stacktrace.js @@ -29,6 +29,7 @@ const vite = await createServer({ logLevel: isTest ? 'error' : 'info', server: { middlewareMode: true, + ws: false, }, appType: 'custom', }) diff --git a/playground/test-utils.ts b/playground/test-utils.ts index 544bc31f8..b0b43544c 100644 --- a/playground/test-utils.ts +++ b/playground/test-utils.ts @@ -47,6 +47,9 @@ export const ports = { 'css/dynamic-import': 5007, 'css/lightningcss-proxy': 5008, 'backend-integration': 5009, + 'client-reload': 5010, + 'client-reload/hmr-port': 5011, + 'client-reload/cross-origin': 5012, } export const hmrPorts = { 'optimize-missing-deps': 24680, @@ -58,6 +61,8 @@ export const hmrPorts = { 'css/lightningcss-proxy': 24686, json: 24687, 'ssr-conditions': 24688, + 'client-reload/hmr-port': 24689, + 'client-reload/cross-origin': 24690, } const hexToNameMap: Record = {} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0365cbc56..4321578f1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -530,6 +530,8 @@ importers: specifier: ^0.11.4 version: 0.11.4 + playground/client-reload: {} + playground/config/packages/entry: dependencies: '@vite/test-config-plugin-module-condition':