From 3b9587941063679f525551b8b809b600abb06aae Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 16 Jul 2024 11:25:24 +0200 Subject: [PATCH 1/5] Implement ChannelFetch util to relay fetch requests over the channel, and have URQL use it --- src/Panel.tsx | 8 ++- src/preset.ts | 4 ++ src/utils/ChannelFetch.ts | 48 +++++++++++++++ src/utils/graphQLClient.tsx | 109 +++++++++++++++++++---------------- src/utils/useChannelFetch.ts | 54 +++++++++++++++++ 5 files changed, 171 insertions(+), 52 deletions(-) create mode 100644 src/utils/ChannelFetch.ts create mode 100644 src/utils/useChannelFetch.ts diff --git a/src/Panel.tsx b/src/Panel.tsx index d972c750..3338a566 100644 --- a/src/Panel.tsx +++ b/src/Panel.tsx @@ -28,9 +28,10 @@ import { ControlsProvider } from "./screens/VisualTests/ControlsContext"; import { RunBuildProvider } from "./screens/VisualTests/RunBuildContext"; import { VisualTests } from "./screens/VisualTests/VisualTests"; import { APIInfoPayload, GitInfoPayload, LocalBuildProgress, UpdateStatusFunction } from "./types"; -import { client, Provider, useAccessToken } from "./utils/graphQLClient"; +import { createClient, GraphQLClientProvider, useAccessToken } from "./utils/graphQLClient"; import { TelemetryProvider } from "./utils/TelemetryContext"; import { useBuildEvents } from "./utils/useBuildEvents"; +import { useChannelFetch } from "./utils/useChannelFetch"; import { useProjectId } from "./utils/useProjectId"; import { clearSessionState, useSessionState } from "./utils/useSessionState"; import { useSharedState } from "./utils/useSharedState"; @@ -81,8 +82,9 @@ export const Panel = ({ active, api }: PanelProps) => { const trackEvent = useCallback((data: any) => emit(TELEMETRY, data), [emit]); const { isRunning, startBuild, stopBuild } = useBuildEvents({ localBuildProgress, accessToken }); + const fetch = useChannelFetch(); const withProviders = (children: React.ReactNode) => ( - + { - + ); if (!active) { diff --git a/src/preset.ts b/src/preset.ts index f74c8f3a..be042437 100644 --- a/src/preset.ts +++ b/src/preset.ts @@ -35,6 +35,7 @@ import { LocalBuildProgress, ProjectInfoPayload, } from "./types"; +import { ChannelFetch } from "./utils/ChannelFetch"; import { SharedState } from "./utils/SharedState"; import { updateChromaticConfig } from "./utils/updateChromaticConfig"; @@ -193,6 +194,9 @@ const watchConfigFile = async ( async function serverChannel(channel: Channel, options: Options & { configFile?: string }) { const { configFile, presets } = options; + // Handle relayed fetch requests from the client + ChannelFetch.subscribe(ADDON_ID, channel); + // Lazy load these APIs since we don't need them right away const apiPromise = presets.apply("experimental_serverAPI"); const corePromise = presets.apply("core"); diff --git a/src/utils/ChannelFetch.ts b/src/utils/ChannelFetch.ts new file mode 100644 index 00000000..353e6762 --- /dev/null +++ b/src/utils/ChannelFetch.ts @@ -0,0 +1,48 @@ +import type { Channel } from "@storybook/channels"; + +export const FETCH_ABORTED = "ChannelFetch/aborted"; +export const FETCH_REQUEST = "ChannelFetch/request"; +export const FETCH_RESPONSE = "ChannelFetch/response"; + +type ChannelLike = Pick; + +const instances = new Map(); + +export class ChannelFetch { + channel: ChannelLike; + + abortControllers: Map; + + constructor(channel: ChannelLike) { + this.channel = channel; + this.abortControllers = new Map(); + + this.channel.on(FETCH_ABORTED, ({ requestId }) => { + this.abortControllers.get(requestId)?.abort(); + this.abortControllers.delete(requestId); + }); + + this.channel.on(FETCH_REQUEST, async ({ requestId, input, init }) => { + const controller = new AbortController(); + this.abortControllers.set(requestId, controller); + + try { + const res = await fetch(input as RequestInfo, { ...init, signal: controller.signal }); + const body = await res.text(); + const headers = Array.from(res.headers as any); + const response = { body, headers, status: res.status, statusText: res.statusText }; + this.channel.emit(FETCH_RESPONSE, { requestId, response }); + } catch (error) { + this.channel.emit(FETCH_RESPONSE, { requestId, error }); + } finally { + this.abortControllers.delete(requestId); + } + }); + } + + static subscribe(key: string, channel: ChannelLike) { + const instance = instances.get(key) || new ChannelFetch(channel); + if (!instances.has(key)) instances.set(key, instance); + return instance; + } +} diff --git a/src/utils/graphQLClient.tsx b/src/utils/graphQLClient.tsx index 33248178..75b0c815 100644 --- a/src/utils/graphQLClient.tsx +++ b/src/utils/graphQLClient.tsx @@ -1,13 +1,11 @@ import { useAddonState } from "@storybook/manager-api"; import { authExchange } from "@urql/exchange-auth"; import React from "react"; -import { Client, fetchExchange, mapExchange, Provider } from "urql"; +import { Client, ClientOptions, fetchExchange, mapExchange, Provider } from "urql"; import { v4 as uuid } from "uuid"; import { ACCESS_TOKEN_KEY, ADDON_ID, CHROMATIC_API_URL } from "../constants"; -export { Provider }; - let currentToken: string | null; let currentTokenExpiration: number | null; const setCurrentToken = (token: string | null) => { @@ -56,56 +54,69 @@ export const getFetchOptions = (token?: string) => ({ }, }); -export const client = new Client({ - url: CHROMATIC_API_URL, - exchanges: [ - // We don't use cacheExchange, because it would inadvertently share data between stories. - mapExchange({ - onResult(result) { - // Not all queries contain the `viewer` field, in which case it will be `undefined`. - // When we do retrieve the field but the token is invalid, it will be `null`. - if (result.data?.viewer === null) setCurrentToken(null); - }, - }), - authExchange(async (utils) => { - return { - addAuthToOperation(operation) { - if (!currentToken) return operation; - return utils.appendHeaders(operation, { Authorization: `Bearer ${currentToken}` }); +export const createClient = (options?: Partial) => + new Client({ + url: CHROMATIC_API_URL, + exchanges: [ + // We don't use cacheExchange, because it would inadvertently share data between stories. + mapExchange({ + onResult(result) { + // Not all queries contain the `viewer` field, in which case it will be `undefined`. + // When we do retrieve the field but the token is invalid, it will be `null`. + if (result.data?.viewer === null) setCurrentToken(null); }, + }), + authExchange(async (utils) => { + return { + addAuthToOperation(operation) { + if (!currentToken) return operation; + return utils.appendHeaders(operation, { Authorization: `Bearer ${currentToken}` }); + }, - // Determine if the current error is an authentication error. - didAuthError: (error) => - error.response.status === 401 || - error.graphQLErrors.some((e) => e.message.includes("Must login")), + // Determine if the current error is an authentication error. + didAuthError: (error) => + error.response.status === 401 || + error.graphQLErrors.some((e) => e.message.includes("Must login")), - // If didAuthError returns true, clear the token. Ideally we should refresh the token here. - // The operation will be retried automatically. - async refreshAuth() { - setCurrentToken(null); - }, + // If didAuthError returns true, clear the token. Ideally we should refresh the token here. + // The operation will be retried automatically. + async refreshAuth() { + setCurrentToken(null); + }, - // Prevent making a request if we know the token is missing, invalid or expired. - // This handler is called repeatedly so we avoid parsing the token each time. - willAuthError() { - if (!currentToken) return true; - try { - if (!currentTokenExpiration) { - const { exp } = JSON.parse(atob(currentToken.split(".")[1])); - currentTokenExpiration = exp; + // Prevent making a request if we know the token is missing, invalid or expired. + // This handler is called repeatedly so we avoid parsing the token each time. + willAuthError() { + if (!currentToken) return true; + try { + if (!currentTokenExpiration) { + const { exp } = JSON.parse(atob(currentToken.split(".")[1])); + currentTokenExpiration = exp; + } + return Date.now() / 1000 > (currentTokenExpiration || 0); + } catch (e) { + return true; } - return Date.now() / 1000 > (currentTokenExpiration || 0); - } catch (e) { - return true; - } - }, - }; - }), - fetchExchange, - ], - fetchOptions: getFetchOptions(), // Auth header (token) is handled by authExchange -}); + }, + }; + }), + fetchExchange, + ], + fetchOptions: getFetchOptions(), // Auth header (token) is handled by authExchange + ...options, + }); -export const GraphQLClientProvider = ({ children }: { children: React.ReactNode }) => { - return {children}; +export const GraphQLClientProvider = ({ + children, + value = createClient(), + ...rest +}: { + children: React.ReactNode; + value?: Client; +}) => { + return ( + + {children} + + ); }; diff --git a/src/utils/useChannelFetch.ts b/src/utils/useChannelFetch.ts new file mode 100644 index 00000000..498715a9 --- /dev/null +++ b/src/utils/useChannelFetch.ts @@ -0,0 +1,54 @@ +import { useChannel } from "@storybook/manager-api"; + +const FETCH_ABORTED = "ChannelFetch/aborted"; +const FETCH_REQUEST = "ChannelFetch/request"; +const FETCH_RESPONSE = "ChannelFetch/response"; + +type SerializedResponse = { + status: number; + statusText: string; + headers: [string, string][]; + body: string; +}; + +const pendingRequests = new Map< + string, + { resolve: (value: Response) => void; reject: (reason?: any) => void } +>(); + +export const useChannelFetch: () => typeof fetch = () => { + const emit = useChannel({ + [FETCH_RESPONSE]: ( + data: + | { requestId: string; response: SerializedResponse } + | { requestId: string; error: string } + ) => { + const request = pendingRequests.get(data.requestId); + if (!request) return; + + pendingRequests.delete(data.requestId); + if ("error" in data) { + request.reject(new Error(data.error)); + } else { + const { body, headers, status, statusText } = data.response; + const res = new Response(body, { headers, status, statusText }); + request.resolve(res); + } + }, + }); + + return async (input: string | URL | Request, { signal, ...init }: RequestInit = {}) => { + const requestId = Math.random().toString(36).slice(2); + emit(FETCH_REQUEST, { requestId, input, init }); + + signal?.addEventListener("abort", () => emit(FETCH_ABORTED, { requestId })); + + return new Promise((resolve, reject) => { + pendingRequests.set(requestId, { resolve, reject }); + setTimeout(() => { + reject(new Error("Request timed out")); + pendingRequests.delete(requestId); + }, 30000); + }); + }; +}; From 2f1d25b8a616988a5c41c57713a1ea63b06db2e7 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 16 Jul 2024 13:56:05 +0200 Subject: [PATCH 2/5] Move event names to constants file --- src/constants.ts | 4 ++++ src/utils/ChannelFetch.ts | 4 +--- src/utils/useChannelFetch.ts | 4 +--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/constants.ts b/src/constants.ts index d24b86f6..d507f0bc 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -28,6 +28,10 @@ export const TELEMETRY = `${ADDON_ID}/telemetry`; export const ENABLE_FILTER = `${ADDON_ID}/enableFilter`; export const REMOVE_ADDON = `${ADDON_ID}/removeAddon`; +export const FETCH_ABORTED = `${ADDON_ID}/ChannelFetch/aborted`; +export const FETCH_REQUEST = `${ADDON_ID}ChannelFetch/request`; +export const FETCH_RESPONSE = `${ADDON_ID}ChannelFetch/response`; + export const CONFIG_OVERRIDES = { // Local changes should never be auto-accepted autoAcceptChanges: false, diff --git a/src/utils/ChannelFetch.ts b/src/utils/ChannelFetch.ts index 353e6762..c95a70c6 100644 --- a/src/utils/ChannelFetch.ts +++ b/src/utils/ChannelFetch.ts @@ -1,8 +1,6 @@ import type { Channel } from "@storybook/channels"; -export const FETCH_ABORTED = "ChannelFetch/aborted"; -export const FETCH_REQUEST = "ChannelFetch/request"; -export const FETCH_RESPONSE = "ChannelFetch/response"; +import { FETCH_ABORTED, FETCH_REQUEST, FETCH_RESPONSE } from "../constants"; type ChannelLike = Pick; diff --git a/src/utils/useChannelFetch.ts b/src/utils/useChannelFetch.ts index 498715a9..9b56025c 100644 --- a/src/utils/useChannelFetch.ts +++ b/src/utils/useChannelFetch.ts @@ -1,8 +1,6 @@ import { useChannel } from "@storybook/manager-api"; -const FETCH_ABORTED = "ChannelFetch/aborted"; -const FETCH_REQUEST = "ChannelFetch/request"; -const FETCH_RESPONSE = "ChannelFetch/response"; +import { FETCH_ABORTED, FETCH_REQUEST, FETCH_RESPONSE } from "../constants"; type SerializedResponse = { status: number; From d5af903ea88148c734c6be87d6abfcdcee13c7ec Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 16 Jul 2024 15:47:38 +0200 Subject: [PATCH 3/5] Improve abortSignal handling --- src/utils/useChannelFetch.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/utils/useChannelFetch.ts b/src/utils/useChannelFetch.ts index 9b56025c..8eca1d1e 100644 --- a/src/utils/useChannelFetch.ts +++ b/src/utils/useChannelFetch.ts @@ -36,11 +36,18 @@ export const useChannelFetch: () => typeof fetch = () => { }); return async (input: string | URL | Request, { signal, ...init }: RequestInit = {}) => { + if (signal?.aborted) { + return Promise.reject(signal.reason); + } + const requestId = Math.random().toString(36).slice(2); + signal?.addEventListener("abort", () => { + emit(FETCH_ABORTED, { requestId }); + pendingRequests.get(requestId)?.reject(signal.reason); + pendingRequests.delete(requestId); + }); emit(FETCH_REQUEST, { requestId, input, init }); - signal?.addEventListener("abort", () => emit(FETCH_ABORTED, { requestId })); - return new Promise((resolve, reject) => { pendingRequests.set(requestId, { resolve, reject }); setTimeout(() => { From b7066517503f2c4ca626bb16ba80b71f712ec9b4 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 16 Jul 2024 20:32:14 +0200 Subject: [PATCH 4/5] Add tests for ChannelFetch --- src/utils/ChannelFetch.test.ts | 96 ++++++++++++++++++++++++++++++++++ src/utils/ChannelFetch.ts | 11 ++-- src/utils/MockChannel.ts | 16 ++++++ src/utils/SharedState.test.ts | 18 +------ 4 files changed, 119 insertions(+), 22 deletions(-) create mode 100644 src/utils/ChannelFetch.test.ts create mode 100644 src/utils/MockChannel.ts diff --git a/src/utils/ChannelFetch.test.ts b/src/utils/ChannelFetch.test.ts new file mode 100644 index 00000000..62bf9a46 --- /dev/null +++ b/src/utils/ChannelFetch.test.ts @@ -0,0 +1,96 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { FETCH_ABORTED, FETCH_REQUEST, FETCH_RESPONSE } from "../constants"; +import { ChannelFetch } from "./ChannelFetch"; +import { MockChannel } from "./MockChannel"; + +const resolveAfter = (ms: number, value: any) => + new Promise((resolve) => setTimeout(resolve, ms, value)); + +const rejectAfter = (ms: number, reason: any) => + new Promise((_, reject) => setTimeout(reject, ms, reason)); + +describe("ChannelFetch", () => { + let channel: MockChannel; + + beforeEach(() => { + channel = new MockChannel(); + }); + + it("should handle fetch requests", async () => { + const fetch = vi.fn(() => resolveAfter(100, { headers: [], text: async () => "data" })); + ChannelFetch.subscribe("req", channel, fetch as any); + + channel.emit(FETCH_REQUEST, { + requestId: "req", + input: "https://example.com", + init: { headers: { foo: "bar" } }, + }); + + await vi.waitFor(() => { + expect(fetch).toHaveBeenCalledWith("https://example.com", { + headers: { foo: "bar" }, + signal: expect.any(AbortSignal), + }); + }); + }); + + it("should send fetch responses", async () => { + const fetch = vi.fn(() => resolveAfter(100, { headers: [], text: async () => "data" })); + const instance = ChannelFetch.subscribe("res", channel, fetch as any); + + const promise = new Promise((resolve) => { + channel.on(FETCH_RESPONSE, ({ response, error }) => { + expect(response.body).toBe("data"); + expect(error).toBeUndefined(); + resolve(); + }); + }); + + channel.emit(FETCH_REQUEST, { requestId: "res", input: "https://example.com" }); + await vi.waitFor(() => { + expect(instance.abortControllers.size).toBe(1); + }); + + await promise; + + expect(instance.abortControllers.size).toBe(0); + }); + + it("should send fetch error responses", async () => { + const fetch = vi.fn(() => rejectAfter(100, new Error("oops"))); + const instance = ChannelFetch.subscribe("err", channel, fetch as any); + + const promise = new Promise((resolve) => { + channel.on(FETCH_RESPONSE, ({ response, error }) => { + expect(response).toBeUndefined(); + expect(error).toMatch(/oops/); + resolve(); + }); + }); + + channel.emit(FETCH_REQUEST, { requestId: "err", input: "https://example.com" }); + await vi.waitFor(() => { + expect(instance.abortControllers.size).toBe(1); + }); + + await promise; + expect(instance.abortControllers.size).toBe(0); + }); + + it("should abort fetch requests", async () => { + const fetch = vi.fn((input, init) => new Promise(() => {})); + const instance = ChannelFetch.subscribe("abort", channel, fetch); + + channel.emit(FETCH_REQUEST, { requestId: "abort", input: "https://example.com" }); + await vi.waitFor(() => { + expect(instance.abortControllers.size).toBe(1); + }); + + channel.emit(FETCH_ABORTED, { requestId: "abort" }); + await vi.waitFor(() => { + expect(fetch.mock.lastCall?.[1].signal.aborted).toBe(true); + expect(instance.abortControllers.size).toBe(0); + }); + }); +}); diff --git a/src/utils/ChannelFetch.ts b/src/utils/ChannelFetch.ts index c95a70c6..d6fb1371 100644 --- a/src/utils/ChannelFetch.ts +++ b/src/utils/ChannelFetch.ts @@ -11,7 +11,7 @@ export class ChannelFetch { abortControllers: Map; - constructor(channel: ChannelLike) { + constructor(channel: ChannelLike, _fetch = fetch) { this.channel = channel; this.abortControllers = new Map(); @@ -25,12 +25,13 @@ export class ChannelFetch { this.abortControllers.set(requestId, controller); try { - const res = await fetch(input as RequestInfo, { ...init, signal: controller.signal }); + const res = await _fetch(input as RequestInfo, { ...init, signal: controller.signal }); const body = await res.text(); const headers = Array.from(res.headers as any); const response = { body, headers, status: res.status, statusText: res.statusText }; this.channel.emit(FETCH_RESPONSE, { requestId, response }); - } catch (error) { + } catch (err) { + const error = err instanceof Error ? err.message : String(err); this.channel.emit(FETCH_RESPONSE, { requestId, error }); } finally { this.abortControllers.delete(requestId); @@ -38,8 +39,8 @@ export class ChannelFetch { }); } - static subscribe(key: string, channel: ChannelLike) { - const instance = instances.get(key) || new ChannelFetch(channel); + static subscribe(key: string, channel: ChannelLike, _fetch = fetch) { + const instance = instances.get(key) || new ChannelFetch(channel, _fetch); if (!instances.has(key)) instances.set(key, instance); return instance; } diff --git a/src/utils/MockChannel.ts b/src/utils/MockChannel.ts new file mode 100644 index 00000000..209e4f39 --- /dev/null +++ b/src/utils/MockChannel.ts @@ -0,0 +1,16 @@ +export class MockChannel { + private listeners: Record void)[]> = {}; + + on(event: string, listener: (...args: any[]) => void) { + this.listeners[event] = [...(this.listeners[event] ?? []), listener]; + } + + off(event: string, listener: (...args: any[]) => void) { + this.listeners[event] = (this.listeners[event] ?? []).filter((l) => l !== listener); + } + + emit(event: string, ...args: any[]) { + // setTimeout is used to simulate the asynchronous nature of the real channel + (this.listeners[event] || []).forEach((listener) => setTimeout(() => listener(...args))); + } +} diff --git a/src/utils/SharedState.test.ts b/src/utils/SharedState.test.ts index 7b2d1ecb..ca98b216 100644 --- a/src/utils/SharedState.test.ts +++ b/src/utils/SharedState.test.ts @@ -1,24 +1,8 @@ import { beforeEach, describe, expect, it } from "vitest"; +import { MockChannel } from "./MockChannel"; import { SharedState } from "./SharedState"; -class MockChannel { - private listeners: Record void)[]> = {}; - - on(event: string, listener: (...args: any[]) => void) { - this.listeners[event] = [...(this.listeners[event] ?? []), listener]; - } - - off(event: string, listener: (...args: any[]) => void) { - this.listeners[event] = (this.listeners[event] ?? []).filter((l) => l !== listener); - } - - emit(event: string, ...args: any[]) { - // setTimeout is used to simulate the asynchronous nature of the real channel - (this.listeners[event] || []).forEach((listener) => setTimeout(() => listener(...args))); - } -} - const tick = () => new Promise((resolve) => setTimeout(resolve, 0)); describe("SharedState", () => { From d094e1f7edeb5aee626035b598b79f49d512beb9 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Fri, 19 Jul 2024 13:14:05 +0200 Subject: [PATCH 5/5] Remove unnecessary key prop --- src/Panel.tsx | 4 +--- src/utils/graphQLClient.tsx | 9 +-------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/Panel.tsx b/src/Panel.tsx index b73101eb..faa85e9c 100644 --- a/src/Panel.tsx +++ b/src/Panel.tsx @@ -11,7 +11,6 @@ import { IS_OFFLINE, IS_OUTDATED, LOCAL_BUILD_PROGRESS, - PANEL_ID, REMOVE_ADDON, TELEMETRY, } from "./constants"; @@ -96,7 +95,7 @@ export const Panel = ({ active, api }: PanelProps) => { const fetch = useChannelFetch(); const withProviders = (children: React.ReactNode) => ( - + { if (!accessToken) { return withProviders( ) => export const GraphQLClientProvider = ({ children, value = createClient(), - ...rest }: { children: React.ReactNode; value?: Client; -}) => { - return ( - - {children} - - ); -}; +}) => {children};