Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions code/core/src/channels/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 & {
Expand Down
99 changes: 5 additions & 94 deletions code/core/src/channels/websocket/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 };
};
Expand All @@ -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', () => {
Expand All @@ -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();
});
});
44 changes: 1 addition & 43 deletions code/core/src/channels/websocket/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,9 @@ interface WebsocketTransportArgs extends Partial<Config> {
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 || {};
Expand All @@ -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' });
Expand All @@ -119,7 +78,6 @@ export class WebsocketTransport implements ChannelTransport {
from: page || 'preview',
});
this.isClosed = true;
clearTimeout(this.pingTimeout);
};
}

Expand Down
4 changes: 2 additions & 2 deletions code/core/src/cli/tools/sdk/node-channel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
2 changes: 0 additions & 2 deletions code/core/src/cli/tools/sdk/node-channel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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] });
Expand Down
1 change: 0 additions & 1 deletion code/core/src/manager/globals/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,6 @@ export default {
'storybook/internal/channels': [
'Channel',
'HEARTBEAT_INTERVAL',
'HEARTBEAT_MAX_LATENCY',
'PostMessageTransport',
'SERVER_CHANNEL_PATH',
'WebsocketTransport',
Expand Down
13 changes: 3 additions & 10 deletions code/core/src/manager/index.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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: <FailedIcon color={color.negative} />,
Expand Down Expand Up @@ -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 });
},
});

Expand Down
5 changes: 2 additions & 3 deletions code/core/src/manager/runtime.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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: <FailedIcon color={color.negative} />,
Expand Down
Loading