diff --git a/code/core/src/channels/index.ts b/code/core/src/channels/index.ts index 5f2307151bdd..8985051afc13 100644 --- a/code/core/src/channels/index.ts +++ b/code/core/src/channels/index.ts @@ -19,12 +19,7 @@ export { export default Channel; export { PostMessageTransport } from './postmessage/index.ts'; -export { - WebsocketTransport, - HEARTBEAT_INTERVAL, - HEARTBEAT_MAX_LATENCY, - SERVER_CHANNEL_PATH, -} from './websocket/index.ts'; +export { WebsocketTransport, HEARTBEAT_INTERVAL, SERVER_CHANNEL_PATH } from './websocket/index.ts'; export type { ChannelWebSocket } from './websocket/index.ts'; type Options = Config & { diff --git a/code/core/src/channels/websocket/index.test.ts b/code/core/src/channels/websocket/index.test.ts index 96267db31150..d270aeb1a0de 100644 --- a/code/core/src/channels/websocket/index.test.ts +++ b/code/core/src/channels/websocket/index.test.ts @@ -2,16 +2,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { parse, stringify } from 'telejson'; -import { HEARTBEAT_INTERVAL, HEARTBEAT_MAX_LATENCY, WebsocketTransport } from './index.ts'; - -const TIMEOUT = HEARTBEAT_INTERVAL + HEARTBEAT_MAX_LATENCY; +import { HEARTBEAT_INTERVAL, WebsocketTransport } from './index.ts'; const socketRef = { current: undefined as unknown as MockWebSocket }; -/** - * Minimal WebSocket stub that keeps outgoing `send` calls separate from incoming messages, so tests - * can assert heartbeat behavior without a pong echoing straight back into the message handler. - */ class MockWebSocket { static OPEN = 1; @@ -56,7 +50,6 @@ const createConnectedTransport = () => { onError, }); transport.setHandler(handler); - // onopen marks the transport ready and starts the heartbeat watchdog socketRef.current.onopen?.(); return { transport, handler, socket: socketRef.current }; }; @@ -75,78 +68,20 @@ describe('WebsocketTransport heartbeat', () => { vi.unstubAllGlobals(); }); - it('closes with code 3008 when no message is received within the timeout window', () => { + it('does not close the socket when no message arrives', () => { const { socket } = createConnectedTransport(); - vi.advanceTimersByTime(TIMEOUT - 1); - expect(socket.closed).toBeUndefined(); - - vi.advanceTimersByTime(1); - expect(socket.closed).toEqual({ code: 3008, reason: 'timeout' }); - }); - - it('does not close while the heartbeat is paused, then restarts the watchdog on resume', () => { - const { transport, socket } = createConnectedTransport(); - - transport.pauseHeartbeat(); - vi.advanceTimersByTime(TIMEOUT * 3); + vi.advanceTimersByTime(HEARTBEAT_INTERVAL * 4); expect(socket.closed).toBeUndefined(); - - transport.resumeHeartbeat(); - vi.advanceTimersByTime(TIMEOUT - 1); - expect(socket.closed).toBeUndefined(); - - vi.advanceTimersByTime(1); - expect(socket.closed).toEqual({ code: 3008, reason: 'timeout' }); }); - it('does not start the receive watchdog when enableHeartbeat is false', () => { - const transport = new WebsocketTransport({ - url: 'ws://localhost:6006', - page: 'manager', - onError: vi.fn(), - enableHeartbeat: false, - }); - transport.setHandler(vi.fn()); - socketRef.current.onopen?.(); - - vi.advanceTimersByTime(TIMEOUT * 3); - expect(socketRef.current.closed).toBeUndefined(); - - transport.resumeHeartbeat(); - vi.advanceTimersByTime(TIMEOUT * 3); - expect(socketRef.current.closed).toBeUndefined(); - }); - - it('resets the heartbeat and replies with pong when a ping is received', () => { + it('replies with pong when a ping is received', () => { const { socket } = createConnectedTransport(); - vi.advanceTimersByTime(TIMEOUT - 1); socket.receive({ type: 'ping' }); expect(sentEvents(socket)).toContainEqual({ type: 'pong' }); - - // The heartbeat was reset, so another near-full window can elapse without closing - vi.advanceTimersByTime(TIMEOUT - 1); expect(socket.closed).toBeUndefined(); - - vi.advanceTimersByTime(1); - expect(socket.closed).toEqual({ code: 3008, reason: 'timeout' }); - }); - - it('resets the heartbeat when a non-ping event is received', () => { - const { socket } = createConnectedTransport(); - - vi.advanceTimersByTime(TIMEOUT - 1); - socket.receive({ type: 'test', args: [], from: 'preview' }); - - // Any message is evidence the connection is alive, not just a literal ping - so this arrival - // must reset the watchdog too, even though it's not the `ping` type. - vi.advanceTimersByTime(TIMEOUT - 1); - expect(socket.closed).toBeUndefined(); - - vi.advanceTimersByTime(1); - expect(socket.closed).toEqual({ code: 3008, reason: 'timeout' }); }); it('does not forward ping events to the channel handler', () => { @@ -165,38 +100,14 @@ describe('WebsocketTransport heartbeat', () => { expect(handler).toHaveBeenCalledWith(expect.objectContaining({ type: 'test' })); }); - it('resets the heartbeat for pings even when the channel handler throws', () => { + it('replies with pong even when the channel handler throws', () => { const { transport, socket } = createConnectedTransport(); - // Simulate an overloaded/failing channel handler. Heartbeat processing must not depend on it, - // otherwise a busy channel could starve the reset and close a healthy connection with code 3008. transport.setHandler(() => { throw new Error('handler boom'); }); - vi.advanceTimersByTime(TIMEOUT - 1); expect(() => socket.receive({ type: 'ping' })).not.toThrow(); expect(sentEvents(socket)).toContainEqual({ type: 'pong' }); - - vi.advanceTimersByTime(TIMEOUT - 1); - expect(socket.closed).toBeUndefined(); - }); - - it('resets the heartbeat for non-ping events even when the channel handler throws', () => { - const { transport, socket } = createConnectedTransport(); - - transport.setHandler(() => { - throw new Error('handler boom'); - }); - - vi.advanceTimersByTime(TIMEOUT - 1); - // The handler throwing is expected to propagate to the caller (the socket's own onmessage - // dispatch), but the heartbeat reset must already have happened before that point. - expect(() => socket.receive({ type: 'test', args: [], from: 'preview' })).toThrow( - 'handler boom' - ); - - vi.advanceTimersByTime(TIMEOUT - 1); - expect(socket.closed).toBeUndefined(); }); }); diff --git a/code/core/src/channels/websocket/index.ts b/code/core/src/channels/websocket/index.ts index f33b40a00ac7..e394d86d19ba 100644 --- a/code/core/src/channels/websocket/index.ts +++ b/code/core/src/channels/websocket/index.ts @@ -28,11 +28,9 @@ interface WebsocketTransportArgs extends Partial { url: string; onError: OnError; createSocket?: (url: string) => ChannelWebSocket; - enableHeartbeat?: boolean; } export const HEARTBEAT_INTERVAL = 15000; -export const HEARTBEAT_MAX_LATENCY = 5000; export const SERVER_CHANNEL_PATH = '/storybook-server-channel'; const CHANNEL_OPTIONS = globalThis.CHANNEL_OPTIONS || {}; @@ -48,56 +46,17 @@ export class WebsocketTransport implements ChannelTransport { private isClosed = false; - private pingTimeout: number | NodeJS.Timeout = 0; - - private heartbeatPaused = false; - - private enableHeartbeat = true; - - private heartbeat() { - clearTimeout(this.pingTimeout); - if (!this.enableHeartbeat || this.heartbeatPaused || this.isClosed) { - return; - } - - this.pingTimeout = setTimeout(() => { - this.socket.close(3008, 'timeout'); - }, HEARTBEAT_INTERVAL + HEARTBEAT_MAX_LATENCY); - } - - pauseHeartbeat() { - this.heartbeatPaused = true; - clearTimeout(this.pingTimeout); - } - - resumeHeartbeat() { - this.heartbeatPaused = false; - if (this.isReady) { - this.heartbeat(); - } - } - - constructor({ - url, - onError, - page, - createSocket, - enableHeartbeat = true, - }: WebsocketTransportArgs) { - this.enableHeartbeat = enableHeartbeat; + constructor({ url, onError, page, createSocket }: WebsocketTransportArgs) { // eslint-disable-next-line compat/compat this.socket = createSocket ? createSocket(url) : new WebSocket(url); this.socket.onopen = () => { this.isReady = true; - this.heartbeat(); this.flush(); }; this.socket.onmessage = ({ data }: { data: any }) => { const event = typeof data === 'string' && isJSON(data) ? parse(data) : data; invariant(this.handler, 'WebsocketTransport handler should be set'); - this.heartbeat(); - if (event.type === 'ping') { // Pings are internal to the transport and have no channel listeners. this.send({ type: 'pong' }); @@ -119,7 +78,6 @@ export class WebsocketTransport implements ChannelTransport { from: page || 'preview', }); this.isClosed = true; - clearTimeout(this.pingTimeout); }; } diff --git a/code/core/src/cli/tools/sdk/node-channel.test.ts b/code/core/src/cli/tools/sdk/node-channel.test.ts index ea2b29c09711..e7b33c63853b 100644 --- a/code/core/src/cli/tools/sdk/node-channel.test.ts +++ b/code/core/src/cli/tools/sdk/node-channel.test.ts @@ -152,10 +152,10 @@ describe('createNodeChannel', () => { channel.on(CHANNEL_WS_DISCONNECT, (payload) => disconnects.push(payload)); const connection = await firstConnection(); - connection.close(3008, 'timeout'); + connection.close(1000); await expect(disconnected).rejects.toThrow('Storybook dev server disconnected'); - expect(disconnects).toEqual([{ code: 3008, reason: 'timeout' }]); + expect(disconnects).toEqual([{ code: 1000, reason: '' }]); }); it('rejects with a dev server disconnected error when the server was never reachable', async () => { diff --git a/code/core/src/cli/tools/sdk/node-channel.ts b/code/core/src/cli/tools/sdk/node-channel.ts index fe66cb6fb906..c7272453aac2 100644 --- a/code/core/src/cli/tools/sdk/node-channel.ts +++ b/code/core/src/cli/tools/sdk/node-channel.ts @@ -86,8 +86,6 @@ export function createNodeChannel({ url, token }: NodeChannelOptions): NodeChann url: socketUrl.href, onError: () => {}, createSocket: () => socket, - // Config load and tool calls occupy this event loop longer than the 20s receive watchdog. - enableHeartbeat: false, }); const channel = new Channel({ transports: [transport] }); diff --git a/code/core/src/manager/globals/exports.ts b/code/core/src/manager/globals/exports.ts index 76dcaccbe576..44de094b36ef 100644 --- a/code/core/src/manager/globals/exports.ts +++ b/code/core/src/manager/globals/exports.ts @@ -488,7 +488,6 @@ export default { 'storybook/internal/channels': [ 'Channel', 'HEARTBEAT_INTERVAL', - 'HEARTBEAT_MAX_LATENCY', 'PostMessageTransport', 'SERVER_CHANNEL_PATH', 'WebsocketTransport', diff --git a/code/core/src/manager/index.stories.tsx b/code/core/src/manager/index.stories.tsx index 46771ae56d8a..74b45b0fe69a 100644 --- a/code/core/src/manager/index.stories.tsx +++ b/code/core/src/manager/index.stories.tsx @@ -80,14 +80,13 @@ class ReactProvider extends Provider { api.selectStory('example-button--primary', undefined, { viewMode: 'story' }); }); - this.channel.on(CHANNEL_WS_DISCONNECT, (ev) => { - const TIMEOUT_CODE = 3008; + this.channel.on(CHANNEL_WS_DISCONNECT, () => { this.wsDisconnected = true; api.addNotification({ id: WS_DISCONNECTED_NOTIFICATION_ID, content: { - headline: ev.code === TIMEOUT_CODE ? 'Server timed out' : 'Connection lost', + headline: 'Connection lost', subHeadline: 'Please restart your Storybook server and reload the page', }, icon: , @@ -180,13 +179,7 @@ export const FullScreen = meta.story({ export const ConnectionLost = meta.story({ play: async () => { - channel.emit(CHANNEL_WS_DISCONNECT, { code: 3007 }); - }, -}); - -export const ServerTimedOut = meta.story({ - play: async () => { - channel.emit(CHANNEL_WS_DISCONNECT, { code: 3008 }); + channel.emit(CHANNEL_WS_DISCONNECT, { code: 1006 }); }, }); diff --git a/code/core/src/manager/runtime.tsx b/code/core/src/manager/runtime.tsx index 7c2568aec217..1f6896f9e092 100644 --- a/code/core/src/manager/runtime.tsx +++ b/code/core/src/manager/runtime.tsx @@ -54,14 +54,13 @@ class ReactProvider extends Provider { handleAPI(api: API) { this.addons.loadAddons(api); - this.channel.on(CHANNEL_WS_DISCONNECT, (ev) => { - const TIMEOUT_CODE = 3008; + this.channel.on(CHANNEL_WS_DISCONNECT, () => { this.wsDisconnected = true; api.addNotification({ id: WS_DISCONNECTED_NOTIFICATION_ID, content: { - headline: ev.code === TIMEOUT_CODE ? 'Server timed out' : 'Connection lost', + headline: 'Connection lost', subHeadline: 'Please restart your Storybook server and reload the page', }, icon: ,