From 8ee4bf212d28612259a01ba767860d6cf458dd15 Mon Sep 17 00:00:00 2001 From: David Meister Date: Sat, 13 Jun 2026 10:45:10 +0000 Subject: [PATCH 1/4] fix(webapp): stop double-fetching custom registries in advanced mode loadRegistryUrl() called DotrainRegistry.validate(url) to validate a custom registry before persisting it and reloading the page. The post-reload +layout.ts then calls DotrainRegistry.new(url), which fetches and validates the exact same registry file plus all its settings files from GitHub again. Every custom-registry load therefore downloaded the rain documents twice, which trips GitHub's rate limiter ("Too many requests"). Remove the redundant validate() call from loadRegistryUrl(); it now just persists the URL and reloads. Validation is owned by page-load: +layout.ts already surfaces a DotrainRegistry.new() failure as an errorMessage on the error page, so an invalid registry URL is still reported to the user, just after the reload instead of before, without the duplicate fetch. Tests: rewrite loadRegistryUrl.test.ts to assert the function calls neither DotrainRegistry.validate nor DotrainRegistry.new (the regression guard), that it persists then reloads in that order, and that it never reloads when setRegistry throws. Confirmed the old (validate) implementation fails these. Fixes #2046 Co-Authored-By: Claude Opus 4.8 --- .../src/__tests__/loadRegistryUrl.test.ts | 66 ++++++++++++++----- .../src/lib/services/loadRegistryUrl.ts | 10 +-- 2 files changed, 53 insertions(+), 23 deletions(-) diff --git a/packages/ui-components/src/__tests__/loadRegistryUrl.test.ts b/packages/ui-components/src/__tests__/loadRegistryUrl.test.ts index 52f7586c5d..0fcbbefc2a 100644 --- a/packages/ui-components/src/__tests__/loadRegistryUrl.test.ts +++ b/packages/ui-components/src/__tests__/loadRegistryUrl.test.ts @@ -1,14 +1,17 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import type { Mock } from "vitest"; import { loadRegistryUrl } from "../lib/services/loadRegistryUrl"; import { RegistryManager } from "../lib/providers/registry/RegistryManager"; import { initialRegistry } from "../__fixtures__/RegistryManager"; import { DotrainRegistry } from "@rainlanguage/raindex"; -// Mock dependencies +// Mock dependencies. validate/new are spied so we can assert loadRegistryUrl +// performs NO registry fetch of its own (the page reload + +layout.ts owns +// fetching/validation). A regression that re-introduces a fetch here would +// double the HTTP requests to GitHub (issue #2046). vi.mock("@rainlanguage/raindex", () => ({ DotrainRegistry: { validate: vi.fn(), + new: vi.fn(), }, })); @@ -39,49 +42,76 @@ describe("loadRegistryUrl", () => { ).rejects.toThrow("Registry manager is required"); }); - it("should successfully load registry URL and reload the page", async () => { + it("should set the registry and reload the page", async () => { const testUrl = "https://example.com/registry"; const mockRegistryManager = initialRegistry as RegistryManager; - (DotrainRegistry.validate as Mock).mockResolvedValueOnce({ value: {} }); await loadRegistryUrl(testUrl, mockRegistryManager); - expect(DotrainRegistry.validate).toHaveBeenCalledWith(testUrl); + expect(mockRegistryManager.setRegistry).toHaveBeenCalledWith(testUrl); expect(window.location.reload).toHaveBeenCalled(); }); - it("should throw an error if fetching registry fails", async () => { + it("should NOT fetch/validate the registry itself (avoid double-fetch from GitHub)", async () => { const testUrl = "https://example.com/registry"; - const errorMessage = "Fetch failed"; + const mockRegistryManager = initialRegistry as RegistryManager; + + await loadRegistryUrl(testUrl, mockRegistryManager); + + // The whole point of issue #2046: loadRegistryUrl must not refetch the + // registry/settings, because the post-reload +layout.ts already does so via + // DotrainRegistry.new. Calling validate (or new) here would re-download the + // same files from GitHub and trigger rate limiting. + expect(DotrainRegistry.validate).not.toHaveBeenCalled(); + expect(DotrainRegistry.new).not.toHaveBeenCalled(); + }); + + it("should reload only after persisting the registry (ordering)", async () => { + const testUrl = "https://example.com/registry"; + const calls: string[] = []; const mockRegistryManager = { - setRegistry: vi.fn(), + setRegistry: vi.fn(() => { + calls.push("setRegistry"); + }), } as unknown as RegistryManager; - - (DotrainRegistry.validate as Mock).mockRejectedValueOnce( - new Error(errorMessage), + (window.location.reload as ReturnType).mockImplementation( + () => { + calls.push("reload"); + }, ); + await loadRegistryUrl(testUrl, mockRegistryManager); + + expect(calls).toEqual(["setRegistry", "reload"]); + }); + + it("should rethrow as an Error when setRegistry throws an Error", async () => { + const testUrl = "https://example.com/registry"; + const mockRegistryManager = { + setRegistry: vi.fn(() => { + throw new Error("Failed to save to localStorage"); + }), + } as unknown as RegistryManager; + await expect(loadRegistryUrl(testUrl, mockRegistryManager)).rejects.toThrow( - errorMessage, + "Failed to save to localStorage", ); - expect(mockRegistryManager.setRegistry).not.toHaveBeenCalled(); expect(window.location.reload).not.toHaveBeenCalled(); }); - it("should handle non-Error exception during registry fetch", async () => { + it("should map a non-Error throw to a default message", async () => { const testUrl = "https://example.com/registry"; const mockRegistryManager = { - setRegistry: vi.fn(), + setRegistry: vi.fn(() => { + throw "String error"; + }), } as unknown as RegistryManager; - (DotrainRegistry.validate as Mock).mockRejectedValueOnce("String error"); - await expect(loadRegistryUrl(testUrl, mockRegistryManager)).rejects.toThrow( "Failed to update registry URL", ); - expect(mockRegistryManager.setRegistry).not.toHaveBeenCalled(); expect(window.location.reload).not.toHaveBeenCalled(); }); }); diff --git a/packages/ui-components/src/lib/services/loadRegistryUrl.ts b/packages/ui-components/src/lib/services/loadRegistryUrl.ts index 5809968146..66d1a1b0e9 100644 --- a/packages/ui-components/src/lib/services/loadRegistryUrl.ts +++ b/packages/ui-components/src/lib/services/loadRegistryUrl.ts @@ -1,5 +1,4 @@ import { RegistryManager } from "../providers/registry/RegistryManager"; -import { DotrainRegistry } from "@rainlanguage/raindex"; export async function loadRegistryUrl( url: string, @@ -14,10 +13,11 @@ export async function loadRegistryUrl( } try { - const validationResult = await DotrainRegistry.validate(url); - if (validationResult.error) { - throw new Error(validationResult.error.readableMsg); - } + // Persist the new registry URL and reload. The page-load (+layout.ts) + // calls DotrainRegistry.new(url), which fetches and validates the + // registry. Validating here as well would refetch the same registry and + // settings files from GitHub, doubling the HTTP requests, so validation is + // left to page-load. registryManager.setRegistry(url); window.location.reload(); } catch (e) { From 0455250c5d1550fccf1b4553802e4dc246050b53 Mon Sep 17 00:00:00 2001 From: David Meister Date: Mon, 15 Jun 2026 14:17:28 +0000 Subject: [PATCH 2/4] docs(webapp): state registry-fetch ownership as present-tense fact in comments Rewrite the comments added in this PR so they describe current behavior only: loadRegistryUrl persists the URL and reloads, and fetching/validation is owned by the post-reload +layout.ts via DotrainRegistry.new. No code/logic change. Co-Authored-By: Claude Opus 4.8 --- .../src/__tests__/loadRegistryUrl.test.ts | 15 +++++++-------- .../src/lib/services/loadRegistryUrl.ts | 9 ++++----- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/ui-components/src/__tests__/loadRegistryUrl.test.ts b/packages/ui-components/src/__tests__/loadRegistryUrl.test.ts index 0fcbbefc2a..464dda1fb9 100644 --- a/packages/ui-components/src/__tests__/loadRegistryUrl.test.ts +++ b/packages/ui-components/src/__tests__/loadRegistryUrl.test.ts @@ -4,10 +4,10 @@ import { RegistryManager } from "../lib/providers/registry/RegistryManager"; import { initialRegistry } from "../__fixtures__/RegistryManager"; import { DotrainRegistry } from "@rainlanguage/raindex"; -// Mock dependencies. validate/new are spied so we can assert loadRegistryUrl -// performs NO registry fetch of its own (the page reload + +layout.ts owns -// fetching/validation). A regression that re-introduces a fetch here would -// double the HTTP requests to GitHub (issue #2046). +// Mock dependencies. validate/new are spied so the tests can assert that +// loadRegistryUrl performs no registry fetch of its own. Fetching and +// validation are owned by the post-reload +layout.ts (via DotrainRegistry.new); +// loadRegistryUrl only persists the URL and reloads. vi.mock("@rainlanguage/raindex", () => ({ DotrainRegistry: { validate: vi.fn(), @@ -58,10 +58,9 @@ describe("loadRegistryUrl", () => { await loadRegistryUrl(testUrl, mockRegistryManager); - // The whole point of issue #2046: loadRegistryUrl must not refetch the - // registry/settings, because the post-reload +layout.ts already does so via - // DotrainRegistry.new. Calling validate (or new) here would re-download the - // same files from GitHub and trigger rate limiting. + // loadRegistryUrl does not fetch or validate the registry/settings: that + // is owned by the post-reload +layout.ts via DotrainRegistry.new. So + // neither validate nor new is called from here. expect(DotrainRegistry.validate).not.toHaveBeenCalled(); expect(DotrainRegistry.new).not.toHaveBeenCalled(); }); diff --git a/packages/ui-components/src/lib/services/loadRegistryUrl.ts b/packages/ui-components/src/lib/services/loadRegistryUrl.ts index 66d1a1b0e9..0bd6a35b17 100644 --- a/packages/ui-components/src/lib/services/loadRegistryUrl.ts +++ b/packages/ui-components/src/lib/services/loadRegistryUrl.ts @@ -13,11 +13,10 @@ export async function loadRegistryUrl( } try { - // Persist the new registry URL and reload. The page-load (+layout.ts) - // calls DotrainRegistry.new(url), which fetches and validates the - // registry. Validating here as well would refetch the same registry and - // settings files from GitHub, doubling the HTTP requests, so validation is - // left to page-load. + // Persist the new registry URL and reload. Page-load (+layout.ts) calls + // DotrainRegistry.new(url), which owns fetching and validating the registry + // and settings files. This function persists and reloads only; it does not + // fetch or validate. registryManager.setRegistry(url); window.location.reload(); } catch (e) { From 5a937d6b369c5e671c5dcf213d176696d3bbec8a Mon Sep 17 00:00:00 2001 From: David Meister Date: Mon, 15 Jun 2026 16:28:15 +0000 Subject: [PATCH 3/4] fix(webapp): self-heal a failing custom registry instead of wedging the app When DotrainRegistry.new fails for a custom registry (from the ?registry= param or localStorage), +layout.ts clears the persisted custom registry and retries the default registry so the app still mounts, surfacing a non-fatal warning banner. A failing default registry remains a fatal ErrorPage. Co-Authored-By: Claude Opus 4.8 --- packages/webapp/src/lib/__mocks__/stores.ts | 3 +- packages/webapp/src/routes/+layout.svelte | 11 +- packages/webapp/src/routes/+layout.ts | 137 ++++++++++++++++++-- packages/webapp/src/routes/layout.test.ts | 25 ++++ 4 files changed, 166 insertions(+), 10 deletions(-) diff --git a/packages/webapp/src/lib/__mocks__/stores.ts b/packages/webapp/src/lib/__mocks__/stores.ts index b6288dec17..6f21fac6ff 100644 --- a/packages/webapp/src/lib/__mocks__/stores.ts +++ b/packages/webapp/src/lib/__mocks__/stores.ts @@ -8,7 +8,8 @@ export const initialPageState = { dotrain: 'some dotrain content', deployment: { key: 'deploy-key' }, orderDetail: {}, - errorMessage: '' + errorMessage: '', + registryWarning: '' }, url: new URL('http://localhost:3000/deploy'), params: {}, diff --git a/packages/webapp/src/routes/+layout.svelte b/packages/webapp/src/routes/+layout.svelte index 40ec5b91d7..3d80cc51a6 100644 --- a/packages/webapp/src/routes/+layout.svelte +++ b/packages/webapp/src/routes/+layout.svelte @@ -25,7 +25,7 @@ import { onMount } from 'svelte'; import type { RaindexClient } from '@rainlanguage/raindex'; - const { errorMessage, localDb, raindexClient, registry } = $page.data; + const { errorMessage, registryWarning, localDb, raindexClient, registry } = $page.data; const registryManager = new RegistryManager(REGISTRY_URL); const queryClient = new QueryClient({ @@ -63,6 +63,15 @@ {/if} +{#if registryWarning} +
+ {registryWarning} +
+{/if} + diff --git a/packages/webapp/src/routes/+layout.ts b/packages/webapp/src/routes/+layout.ts index 4247a5fc34..9acd446457 100644 --- a/packages/webapp/src/routes/+layout.ts +++ b/packages/webapp/src/routes/+layout.ts @@ -8,14 +8,55 @@ import type { LayoutLoad } from './$types'; export interface LayoutData { errorMessage?: string; + registryWarning?: string; stores: AppStoresInterface | null; raindexClient: RaindexClient | null; registry: DotrainRegistry | null; localDb: SQLiteWasmDatabase | null; } +/** Remove the persisted custom registry from localStorage and the URL param. */ +const clearCustomRegistry = (url: URL): void => { + if (typeof localStorage !== 'undefined') { + try { + localStorage.removeItem('registry'); + } catch { + // ignore removal failure + } + } + if (typeof window !== 'undefined') { + try { + const next = new URL(window.location.href); + next.searchParams.delete('registry'); + window.history.replaceState({}, '', next.toString()); + } catch { + // ignore URL update failure + } + } + url.searchParams.delete('registry'); +}; + +/** Build a DotrainRegistry, surfacing a readable message on failure. */ +const buildRegistry = async ( + registryUrl: string +): Promise<{ registry: DotrainRegistry | null; error?: string }> => { + try { + const registryResult = await DotrainRegistry.new(registryUrl); + if (registryResult.error) { + return { + registry: null, + error: 'Failed to load registry. ' + registryResult.error.readableMsg + }; + } + return { registry: registryResult.value }; + } catch (error: unknown) { + return { registry: null, error: 'Failed to load registry. ' + (error as Error).message }; + } +}; + export const load: LayoutLoad = async ({ url }) => { let errorMessage: string | undefined; + let registryWarning: string | undefined; const registryParam = url.searchParams.get('registry'); let registryUrl = REGISTRY_URL; @@ -40,16 +81,25 @@ export const load: LayoutLoad = async ({ url }) => { } let registry: DotrainRegistry | null = null; - if (!errorMessage) { - try { - const registryResult = await DotrainRegistry.new(registryUrl); - if (registryResult.error) { - errorMessage = 'Failed to load registry. ' + registryResult.error.readableMsg; + { + const result = await buildRegistry(registryUrl); + registry = result.registry; + if (result.error) { + if (registryUrl !== REGISTRY_URL) { + // A custom registry failed to load. Clear it so the next load uses the + // default, then retry the default in this load so the app still mounts. + clearCustomRegistry(url); + const fallback = await buildRegistry(REGISTRY_URL); + registry = fallback.registry; + if (fallback.error) { + errorMessage = fallback.error; + } else { + registryWarning = + 'The custom registry failed to load and has been reset to the default registry.'; + } } else { - registry = registryResult.value; + errorMessage = result.error; } - } catch (error: unknown) { - errorMessage = 'Failed to load registry. ' + (error as Error).message; } } @@ -97,6 +147,7 @@ export const load: LayoutLoad = async ({ url }) => { } return { + registryWarning, stores: { selectedChainIds: writable([]), showInactiveOrders: writable(false), @@ -227,5 +278,75 @@ if (import.meta.vitest) { expect(result.stores).not.toBeNull(); expect(result.registry).toEqual(mockRegistry); }); + + it('should reset a failing custom registry from localStorage, retry the default, and warn', async () => { + localStorage.setItem('registry', 'https://custom.example/registry'); + mockGetRaindexClient.mockResolvedValue({ value: { client: true } }); + const defaultRegistry = { getRaindexClient: mockGetRaindexClient }; + mockRegistryNew + .mockRejectedValueOnce(new Error('Network error')) + .mockResolvedValueOnce({ value: defaultRegistry }); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = await load({ url: new URL('http://localhost:3000') } as any); + + expect(mockRegistryNew).toHaveBeenNthCalledWith(1, 'https://custom.example/registry'); + expect(mockRegistryNew).toHaveBeenNthCalledWith(2, REGISTRY_URL); + expect(localStorage.getItem('registry')).toBeNull(); + expect(result.errorMessage).toBeUndefined(); + expect(result.registryWarning).toContain('custom registry'); + expect(result.stores).not.toBeNull(); + expect(result.registry).toEqual(defaultRegistry); + }); + + it('should reset a failing custom registry from the ?registry= param, retry the default, and warn', async () => { + mockGetRaindexClient.mockResolvedValue({ value: { client: true } }); + const defaultRegistry = { getRaindexClient: mockGetRaindexClient }; + mockRegistryNew + .mockRejectedValueOnce(new Error('Network error')) + .mockResolvedValueOnce({ value: defaultRegistry }); + + const result = await load({ + url: new URL('http://localhost:3000?registry=https://custom.example/registry') + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); + + expect(mockRegistryNew).toHaveBeenNthCalledWith(1, 'https://custom.example/registry'); + expect(mockRegistryNew).toHaveBeenNthCalledWith(2, REGISTRY_URL); + expect(localStorage.getItem('registry')).toBeNull(); + expect(result.errorMessage).toBeUndefined(); + expect(result.registryWarning).toContain('custom registry'); + expect(result.registry).toEqual(defaultRegistry); + }); + + it('should stay fatal when a failing custom registry resets but the default also fails', async () => { + localStorage.setItem('registry', 'https://custom.example/registry'); + mockRegistryNew + .mockRejectedValueOnce(new Error('Custom network error')) + .mockRejectedValueOnce(new Error('Default network error')); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = await load({ url: new URL('http://localhost:3000') } as any); + + expect(mockRegistryNew).toHaveBeenNthCalledWith(1, 'https://custom.example/registry'); + expect(mockRegistryNew).toHaveBeenNthCalledWith(2, REGISTRY_URL); + expect(localStorage.getItem('registry')).toBeNull(); + expect(result.registryWarning).toBeUndefined(); + expect(result).toHaveProperty('stores', null); + expect(result.errorMessage).toContain('Failed to load registry'); + }); + + it('should stay fatal without resetting when the default registry fails', async () => { + mockRegistryNew.mockRejectedValueOnce(new Error('Network error')); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = await load({ url: new URL('http://localhost:3000') } as any); + + expect(mockRegistryNew).toHaveBeenCalledTimes(1); + expect(mockRegistryNew).toHaveBeenCalledWith(REGISTRY_URL); + expect(result.registryWarning).toBeUndefined(); + expect(result).toHaveProperty('stores', null); + expect(result.errorMessage).toContain('Failed to load registry'); + }); }); } diff --git a/packages/webapp/src/routes/layout.test.ts b/packages/webapp/src/routes/layout.test.ts index e17d8a0510..eabc082fc9 100644 --- a/packages/webapp/src/routes/layout.test.ts +++ b/packages/webapp/src/routes/layout.test.ts @@ -147,4 +147,29 @@ describe('Layout component', () => { expect(screen.getByTestId('error-page')).toBeInTheDocument(); }); }); + + it('shows a non-fatal registry warning banner and still renders the app', async () => { + mockPageStore.mockSetSubscribeValue({ + ...initialPageState, + url: new URL('http://localhost/some-page'), + data: { + ...initialPageState.data, + registryWarning: 'The custom registry failed to load and has been reset to the default registry.' + } + }); + + render(Layout); + + await waitFor(() => { + expect(screen.getByTestId('registry-warning')).toBeInTheDocument(); + expect( + screen.getByText( + 'The custom registry failed to load and has been reset to the default registry.' + ) + ).toBeInTheDocument(); + // The warning is non-fatal: the app still mounts and the error page is absent. + expect(screen.getByTestId('layout-container')).toBeInTheDocument(); + expect(screen.queryByTestId('error-page')).not.toBeInTheDocument(); + }); + }); }); From fa448f83fcdb2b4c2b90377bdf1aee549036b1e3 Mon Sep 17 00:00:00 2001 From: David Meister Date: Fri, 19 Jun 2026 17:39:00 +0000 Subject: [PATCH 4/4] merge(main): resolve conflicts [merge-update]