mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 16:02:10 +00:00
stop websockets resurrecting a reclaimed dev server (#10676)
* fix: stop websockets resurrecting a reclaimed dev server * docs: condense the websocket invariant comment * test: stub fetch suite-wide so waking cannot hit a real dev server * fix: let websockets join an in-flight start
This commit is contained in:
@@ -57,6 +57,9 @@ function parseArgs(argv) {
|
||||
// Loopback by default: the supervised port fronts the /api proxy to a local backend
|
||||
// and serves source over /@fs, and `server.allowedHosts` does not stop a non-browser
|
||||
// client from sending whatever Host header it likes. Widening is opt-in.
|
||||
// Keep `--idle` above the app's 5-minute background poll. Below it, a tab that never
|
||||
// went dormant can have its server reclaimed with no way back: websockets cannot start
|
||||
// one, and the dormancy warm-up only fires on a dormant-to-awake transition.
|
||||
const opts = { targets: [], idleMs: parseDuration('15m'), bind: '127.0.0.1', stats: null }
|
||||
for (let i = 0; i < argv.length; i++) {
|
||||
const arg = argv[i]
|
||||
@@ -297,28 +300,27 @@ class Target {
|
||||
}
|
||||
|
||||
#dispatch(client, first) {
|
||||
// Three kinds of connection, told apart by the subprotocol vite gives its own
|
||||
// sockets. Vite's must not restart a reclaimed server, because its reconnect probe
|
||||
// (`vite-ping`) would resurrect every one we stop. The app's `/ws/*` language-server
|
||||
// and multiplayer sockets must, or a parked script editor loses its smart assistant
|
||||
// until reload. Neither counts as activity: a tab nobody is looking at still
|
||||
// heartbeats, and treating that as use is what pinned servers forever.
|
||||
// No websocket may start a stopped server, nor count as activity: every websocket
|
||||
// client here reconnects on an unconditional timer (y-websocket at a 2.5s ceiling),
|
||||
// so starting on one keeps a reclaimed server alive for as long as a tab is open.
|
||||
// `devPollingDormancy` warms it over HTTP on return instead, before they retry.
|
||||
const head = first.toString('latin1', 0, Math.min(first.length, MAX_HEAD_BYTES))
|
||||
const isWebsocket = /\r\nupgrade:\s*websocket/i.test(head)
|
||||
const isViteSocket =
|
||||
isWebsocket && /\r\nsec-websocket-protocol:[^\r\n]*vite-(hmr|ping)/i.test(head)
|
||||
if (isViteSocket && !this.child) {
|
||||
// `starting` too, not just `child`: a start is in flight before the child exists, and
|
||||
// the warm-up opens exactly that window for the sockets retrying alongside it. A
|
||||
// socket may join a start someone else asked for; it still never initiates one.
|
||||
if (isWebsocket && !this.child && !this.starting) {
|
||||
client.destroy()
|
||||
return
|
||||
}
|
||||
if (!isWebsocket) this.lastActivity = Date.now()
|
||||
if (!isViteSocket) {
|
||||
this.liveSockets.add(client)
|
||||
// Registered at insertion, not after the upstream connects: a client that gives
|
||||
// up during a cold start would otherwise stay in the set forever and silently
|
||||
// wedge the idle reaper for the life of the process.
|
||||
client.once('close', () => this.liveSockets.delete(client))
|
||||
if (!isWebsocket) {
|
||||
this.lastActivity = Date.now()
|
||||
}
|
||||
this.liveSockets.add(client)
|
||||
// Registered at insertion, not after the upstream connects: a client that gives up
|
||||
// during a cold start would otherwise stay in the set forever and silently wedge the
|
||||
// idle reaper for the life of the process.
|
||||
client.once('close', () => this.liveSockets.delete(client))
|
||||
|
||||
this.ensureStarted().then(
|
||||
(port) => {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
// Freshly imported per case: install is idempotent by design, so a shared module instance
|
||||
// would make the second case a no-op against a restored `window.setInterval`.
|
||||
@@ -12,6 +12,15 @@ async function install() {
|
||||
// only thing standing between a forgotten tab and a dev server that never gets reclaimed.
|
||||
describe('devPollingDormancy', () => {
|
||||
const original = { setInterval: window.setInterval, clearInterval: window.clearInterval }
|
||||
let fetchSpy: ReturnType<typeof vi.fn>
|
||||
|
||||
// Stubbed for every case, not just the one that asserts on it: waking issues a real
|
||||
// request otherwise, and jsdom's default origin is localhost:3000 — the frontend port
|
||||
// of the first worktree, whose dev server the suite would then resurrect.
|
||||
beforeEach(() => {
|
||||
fetchSpy = vi.fn(() => Promise.resolve(new Response()))
|
||||
vi.stubGlobal('fetch', fetchSpy)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
window.setInterval = original.setInterval
|
||||
@@ -20,6 +29,8 @@ describe('devPollingDormancy', () => {
|
||||
delete (window as unknown as Record<string, boolean>).__wmDevPollingDormancyInstalled
|
||||
vi.useRealTimers()
|
||||
vi.unstubAllEnvs()
|
||||
// restoreAllMocks does not undo stubGlobal, and the config sets no unstubGlobals.
|
||||
vi.unstubAllGlobals()
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
@@ -58,6 +69,25 @@ describe('devPollingDormancy', () => {
|
||||
expect(tick).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('warms the dev server on wake, since websockets cannot restart it', async () => {
|
||||
vi.useFakeTimers()
|
||||
vi.stubEnv('VITE_DEV_DORMANT_MS', '1000')
|
||||
vi.spyOn(document, 'hasFocus').mockReturnValue(true)
|
||||
setHidden(false)
|
||||
|
||||
await install()
|
||||
|
||||
setHidden(true)
|
||||
vi.advanceTimersByTime(1000)
|
||||
expect(fetchSpy).not.toHaveBeenCalled()
|
||||
|
||||
setHidden(false)
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(1)
|
||||
// The origin is the load-bearing half: it has to reach the supervised port.
|
||||
expect(fetchSpy.mock.calls[0][0]).toBe(`${location.origin}/`)
|
||||
expect(fetchSpy.mock.calls[0][1]).toMatchObject({ method: 'HEAD' })
|
||||
})
|
||||
|
||||
it('does not re-patch when the module itself is hot-replaced', async () => {
|
||||
vi.useFakeTimers()
|
||||
vi.spyOn(document, 'hasFocus').mockReturnValue(true)
|
||||
|
||||
@@ -92,6 +92,9 @@ export function installDevPollingDormancy(): void {
|
||||
}
|
||||
if (!dormant) return
|
||||
dormant = false
|
||||
// The dev supervisor may have reclaimed the server while we were quiet, and only HTTP
|
||||
// restarts it, so put it back up before the app's sockets retry into it.
|
||||
void fetch(`${location.origin}/`, { method: 'HEAD', cache: 'no-store' }).catch(() => {})
|
||||
for (const registration of registrations.values()) {
|
||||
registration.native = nativeSetInterval(
|
||||
registration.handler,
|
||||
|
||||
Reference in New Issue
Block a user