diff --git a/site/src/contexts/ProxyContext.test.tsx b/site/src/contexts/ProxyContext.test.tsx index 8e16e868627e3..03f2662037733 100644 --- a/site/src/contexts/ProxyContext.test.tsx +++ b/site/src/contexts/ProxyContext.test.tsx @@ -26,7 +26,11 @@ import type * as ProxyLatency from "./useProxyLatency"; // here and not inside a unit test. jest.mock("contexts/useProxyLatency", () => ({ useProxyLatency: () => { - return { proxyLatencies: hardCodedLatencies, refetch: jest.fn() }; + return { + proxyLatencies: hardCodedLatencies, + refetch: jest.fn(), + loaded: true, + }; }, })); @@ -115,7 +119,7 @@ describe("ProxyContextGetURLs", () => { preferredPathAppURL, preferredWildcardHostname, ) => { - const preferred = getPreferredProxy(regions, selected, latencies); + const preferred = getPreferredProxy(regions, selected, latencies, true); expect(preferred.preferredPathAppURL).toBe(preferredPathAppURL); expect(preferred.preferredWildcardHostname).toBe( preferredWildcardHostname, @@ -138,10 +142,22 @@ const TestingComponent = () => { // TestingScreen just mounts some components that we can check in the unit test. const TestingScreen = () => { - const { proxy, userProxy, isFetched, isLoading, clearProxy, setProxy } = - useProxy(); + const { + proxy, + userProxy, + isFetched, + isLoading, + latenciesLoaded, + clearProxy, + setProxy, + } = useProxy(); + return ( <> +
@@ -206,7 +222,6 @@ describe("ProxyContextSelection", () => { }; it.each([ - // Not latency behavior [ "empty", { @@ -220,6 +235,7 @@ describe("ProxyContextSelection", () => { "regions_no_selection", { expProxyID: MockPrimaryWorkspaceProxy.id, + expUserProxyID: MockPrimaryWorkspaceProxy.id, regions: MockWorkspaceProxies, storageProxy: undefined, }, @@ -261,11 +277,12 @@ describe("ProxyContextSelection", () => { expUserProxyID: MockHealthyWildWorkspaceProxy.id, }, ], - // Latency behavior is disabled, so the primary should be selected. + // First page load defers to the proxy by latency [ "regions_default_low_latency", { - expProxyID: MockPrimaryWorkspaceProxy.id, + expProxyID: MockHealthyWildWorkspaceProxy.id, + expUserProxyID: MockHealthyWildWorkspaceProxy.id, regions: MockWorkspaceProxies, storageProxy: undefined, latencies: { @@ -362,6 +379,10 @@ describe("ProxyContextSelection", () => { TestingComponent(); await waitForLoaderToBeRemoved(); + await screen.findByTestId("latenciesLoaded").then((x) => { + expect(x.title).toBe("true"); + }); + if (afterLoad) { await afterLoad(); } diff --git a/site/src/contexts/ProxyContext.tsx b/site/src/contexts/ProxyContext.tsx index 55637e32a3069..c162c2c4952ff 100644 --- a/site/src/contexts/ProxyContext.tsx +++ b/site/src/contexts/ProxyContext.tsx @@ -54,6 +54,9 @@ export interface ProxyContextValue { // then the latency has not been fetched yet. Calculations happen async for each proxy in the list. // Refer to the returned report for a given proxy for more information. proxyLatencies: ProxyLatencies; + // latenciesLoaded is true when the latencies have been initially loaded. + // Once set to true, it will not be set to false again. + latenciesLoaded: boolean; // refetchProxyLatencies will trigger refreshing of the proxy latencies. By default the latencies // are loaded once. refetchProxyLatencies: () => Date; @@ -122,8 +125,11 @@ export const ProxyProvider: FC = ({ children }) => { // Every time we get a new proxiesResponse, update the latency check // to each workspace proxy. - const { proxyLatencies, refetch: refetchProxyLatencies } = - useProxyLatency(proxiesResp); + const { + proxyLatencies, + refetch: refetchProxyLatencies, + loaded: latenciesLoaded, + } = useProxyLatency(proxiesResp); // updateProxy is a helper function that when called will // update the proxy being used. @@ -136,7 +142,8 @@ export const ProxyProvider: FC = ({ children }) => { loadUserSelectedProxy(), proxyLatencies, // Do not auto select based on latencies, as inconsistent latencies can cause this - // to behave poorly. + // to change on each call. updateProxy should be stable when selecting a proxy to + // prevent flickering. false, ), ); @@ -149,6 +156,34 @@ export const ProxyProvider: FC = ({ children }) => { updateProxy(); }, [proxiesResp, proxyLatencies]); + // This useEffect will auto select the best proxy if the user has not selected one. + // It must wait until all latencies are loaded to select based on latency. This does mean + // the first time a user loads the page, the proxy will "flicker" to the best proxy. + // + // Once the page is loaded, or the user selects a proxy, this will not run again. + // biome-ignore lint/correctness/useExhaustiveDependencies: Only update if the source data changes + useEffect(() => { + if (loadUserSelectedProxy() !== undefined) { + return; // User has selected a proxy, do not auto select. + } + if (!latenciesLoaded) { + // Wait until the latencies are loaded first. + return; + } + + const best = getPreferredProxy( + proxiesResp ?? [], + loadUserSelectedProxy(), + proxyLatencies, + true, + ); + + if (best?.proxy) { + saveUserSelectedProxy(best.proxy); + updateProxy(); + } + }, [latenciesLoaded, proxiesResp, proxyLatencies]); + return ( = ({ children }) => { userProxy: userSavedProxy, proxy: proxy, proxies: proxiesResp, + latenciesLoaded: latenciesLoaded, isLoading: proxiesLoading, isFetched: proxiesFetched, error: proxiesError, @@ -214,12 +250,12 @@ export const getPreferredProxy = ( // If no proxy is selected, or the selected proxy is unhealthy default to the primary proxy. if (!selectedProxy || !selectedProxy.healthy) { - // By default, use the primary proxy. + // Default to the primary proxy selectedProxy = proxies.find((proxy) => proxy.name === "primary"); // If we have latencies, then attempt to use the best proxy by latency instead. const best = selectByLatency(proxies, latencies); - if (autoSelectBasedOnLatency && best) { + if (autoSelectBasedOnLatency && best !== undefined) { selectedProxy = best; } } diff --git a/site/src/contexts/useProxyLatency.ts b/site/src/contexts/useProxyLatency.ts index ff8be8cd66135..f5f3d2acb415c 100644 --- a/site/src/contexts/useProxyLatency.ts +++ b/site/src/contexts/useProxyLatency.ts @@ -48,6 +48,11 @@ export const useProxyLatency = ( // Until the new values are loaded, the old values will still be used. refetch: () => Date; proxyLatencies: Record; + // loaded signals all latency requests have completed. Once set to true, this will not change. + // Latencies at this point should be loaded from local storage, and updated asynchronously as needed. + // If local storage has updated latencies, then this will be set to true with 0 actual network requests. + // The loaded latencies will all be from the cache. + loaded: boolean; } => { // maxStoredLatencies is the maximum number of latencies to store per proxy in local storage. let maxStoredLatencies = 1; @@ -73,6 +78,8 @@ export const useProxyLatency = ( new Date(new Date().getTime() - proxyIntervalSeconds * 1000).toISOString(), ); + const [loaded, setLoaded] = useState(false); + // Refetch will always set the latestFetchRequest to the current time, making all the cached latencies // stale and triggering a refetch of all proxies in the list. const refetch = () => { @@ -231,6 +238,7 @@ export const useProxyLatency = ( // Local storage cleanup garbageCollectStoredLatencies(proxies, maxStoredLatencies); + setLoaded(true); }); return () => { @@ -241,6 +249,7 @@ export const useProxyLatency = ( return { proxyLatencies, refetch, + loaded, }; }; diff --git a/site/src/modules/dashboard/Navbar/MobileMenu.stories.tsx b/site/src/modules/dashboard/Navbar/MobileMenu.stories.tsx index 058c8799c95e0..cb186dcb973b0 100644 --- a/site/src/modules/dashboard/Navbar/MobileMenu.stories.tsx +++ b/site/src/modules/dashboard/Navbar/MobileMenu.stories.tsx @@ -23,6 +23,7 @@ const meta: Meta = { component: MobileMenu, args: { proxyContextValue: { + latenciesLoaded: true, proxy: { preferredPathAppURL: "", preferredWildcardHostname: "", diff --git a/site/src/modules/dashboard/Navbar/NavbarView.test.tsx b/site/src/modules/dashboard/Navbar/NavbarView.test.tsx index 6739f666c2b17..358b717b492a4 100644 --- a/site/src/modules/dashboard/Navbar/NavbarView.test.tsx +++ b/site/src/modules/dashboard/Navbar/NavbarView.test.tsx @@ -6,6 +6,7 @@ import { renderWithAuth } from "testHelpers/renderHelpers"; import { NavbarView } from "./NavbarView"; const proxyContextValue: ProxyContextValue = { + latenciesLoaded: true, proxy: { preferredPathAppURL: "", preferredWildcardHostname: "", diff --git a/site/src/modules/dashboard/Navbar/ProxyMenu.stories.tsx b/site/src/modules/dashboard/Navbar/ProxyMenu.stories.tsx index 6df47684173fe..15dbb18471c3f 100644 --- a/site/src/modules/dashboard/Navbar/ProxyMenu.stories.tsx +++ b/site/src/modules/dashboard/Navbar/ProxyMenu.stories.tsx @@ -15,6 +15,7 @@ import { withDesktopViewport } from "testHelpers/storybook"; import { ProxyMenu } from "./ProxyMenu"; const defaultProxyContextValue = { + latenciesLoaded: true, proxyLatencies: MockProxyLatencies, proxy: getPreferredProxy(MockWorkspaceProxies, undefined), proxies: MockWorkspaceProxies, diff --git a/site/src/testHelpers/storybook.tsx b/site/src/testHelpers/storybook.tsx index ed64c10958a0b..4b2ba94bd2577 100644 --- a/site/src/testHelpers/storybook.tsx +++ b/site/src/testHelpers/storybook.tsx @@ -167,6 +167,7 @@ export const withProxyProvider = return (