feat: Iterate on ApiProxy to keep login/logout message off the Messag…

…eChannel (#142)

Related to: getsentry/sentry#81942

Changes:
- The `request-login` and `request-logout` messages are going over
`iframe.contentWindow.postMessage()` now. The MessagePort is reserved
for when the iframe is logged in and properly configured.
- When state _changes_ we dispose of any ports (would only been
connected if we were previously logged in) and reload the iframe. This
ensures that we get a new state told to us from the html file
- We've dropped the `sentryApiPath` config value, because the config
endpoint (domain, region if needed, and this prefix) is set inside the
iframe js scripts. The `fetch` command of the proxy can only make fetch
requests to the sentry api now, not to other domains.

.storybook/preview.tsx

@@ -5,7 +5,6 @@ import localStorage from 'toolbar/utils/localStorage';
 
 const baseConfig = {
   sentryOrigin: 'http://localhost:8080',
-  sentryApiPath: '/api/0',
 
   // FeatureFlagsConfig
   featureFlags: undefined,

docs/conditional-script.md

@@ -119,7 +119,6 @@ function MyReactApp() {
 
       // ConnectionConfig
       sentryOrigin: 'https://sentry.sentry.io',
-      sentryApiPath: '/api/0',
 
       // FeatureFlagsConfig
       featureFlags: undefined,

src/env/demo/App.tsx

@@ -12,7 +12,6 @@ export default function App() {
 
       // ConnectionConfig -> See .env.example for defaults
       sentryOrigin: import.meta.env.VITE_SENTRY_ORIGIN ?? 'http://localhost:8080',
-      sentryApiPath: import.meta.env.VITE_SENTRY_API_PATH ?? '/region/us/api/0',
 
       // FeatureFlagsConfig
       featureFlags: MockFeatureFlagIntegration(),

src/lib/components/Navigation.tsx

@@ -85,7 +85,7 @@ export default function Navigation() {
 
         <hr className={menuSeparator} />
 
-        <MenuItem label="logout" onClick={() => apiProxy.exec(new AbortController().signal, 'clear-authn', [])}>
+        <MenuItem label="logout" onClick={() => apiProxy.logout()}>
           <div className={iconItemClass}>
             <IconLock size="sm" isLocked={false} />
             Logout

src/lib/components/unauth/Login.tsx

@@ -11,6 +11,8 @@ const buttonClass = cx('rounded-full p-1 hover:bg-gray-500 hover:underline');
 
 export default function Login() {
   const {debug} = useContext(ConfigContext);
+  const debugLoginSuccess = debug?.includes(DebugTarget.LOGIN_SUCCESS);
+
   const apiProxy = useApiProxyInstance();
 
   const [isLoggingIn, setIsLoggingIn] = useState(false);
@@ -29,9 +31,7 @@ export default function Login() {
   const openPopup = useCallback(() => {
     setIsLoggingIn(true);
 
-    const signal = new AbortController().signal;
-
-    apiProxy.exec(signal, 'request-authn', debug?.includes(DebugTarget.LOGIN_SUCCESS) ? [] : [3000]);
+    apiProxy.login(debugLoginSuccess ? undefined : 3000);
 
     // start timer, after a sec ask about popups
     if (timeoutRef.current) {
@@ -40,7 +40,7 @@ export default function Login() {
     timeoutRef.current = window.setTimeout(() => {
       setShowPopupBlockerMessage(true);
     }, POPUP_MESSAGE_DELAY_MS);
-  }, [apiProxy, debug]);
+  }, [apiProxy, debugLoginSuccess]);
 
   return (
     <UnauthPill>

src/lib/context/ApiProxyContext.tsx

@@ -3,12 +3,13 @@ import {createContext, useCallback, useContext, useEffect, useRef, useState} fro
 import type {ReactNode} from 'react';
 import ConfigContext from 'toolbar/context/ConfigContext';
 import defaultConfig from 'toolbar/context/defaultConfig';
+import usePrevious from 'toolbar/hooks/usePrevious';
 import {getSentryIFrameOrigin} from 'toolbar/sentryApi/urls';
 import {DebugTarget} from 'toolbar/types/config';
 import ApiProxy, {type ProxyState} from 'toolbar/utils/ApiProxy';
 
 const ApiProxyStateContext = createContext<ProxyState>('connecting');
-const ApiProxyContext = createContext<ApiProxy>(new ApiProxy(defaultConfig));
+const ApiProxyContext = createContext<ApiProxy>(new ApiProxy(defaultConfig, {current: null}));
 
 interface Props {
   children: ReactNode;
@@ -17,18 +18,22 @@ interface Props {
 export function ApiProxyContextProvider({children}: Props) {
   const config = useContext(ConfigContext);
   const {debug, organizationSlug, projectIdOrSlug} = config;
+  const enableLogging = debug?.includes(DebugTarget.LOGGING);
+
+  const iframeRef = useRef<HTMLIFrameElement | null>(null);
 
   const log = useCallback(
     (...args: unknown[]) => {
-      if (debug?.includes(DebugTarget.LOGGING)) {
+      if (enableLogging) {
         console.log('<ApiProxyContextProvider>', ...args);
       }
     },
-    [debug]
+    [enableLogging]
   );
 
-  const proxyRef = useRef<ApiProxy>(new ApiProxy(config));
+  const proxyRef = useRef<ApiProxy>(ApiProxy.singleton(config, iframeRef));
   const [proxyState, setProxyState] = useState<ProxyState>('connecting');
+  const lastProxyState = usePrevious(proxyState);
 
   useEffect(() => {
     proxyRef.current.listen();
@@ -41,12 +46,27 @@ export function ApiProxyContextProvider({children}: Props) {
     };
   }, [config, log]);
 
-  const frameSrc = `${getSentryIFrameOrigin(config)}/toolbar/${organizationSlug}/${projectIdOrSlug}/iframe/?logging=${debug ? 1 : 0}`;
+  useEffect(() => {
+    if (proxyState !== lastProxyState) {
+      proxyRef.current.disposePort();
+    }
+  });
+
+  const frameSrc = `${getSentryIFrameOrigin(config)}/toolbar/${organizationSlug}/${projectIdOrSlug}/iframe/?logging=${enableLogging ? 1 : 0}`;
 
+  log('Render with state', {proxyState});
   return (
     <ApiProxyContext.Provider value={proxyRef.current}>
       <ApiProxyStateContext.Provider value={proxyState}>
-        <iframe referrerPolicy="origin" height="0" width="0" src={frameSrc} className="hidden" />
+        <iframe
+          key={proxyState}
+          referrerPolicy="origin"
+          height="0"
+          width="0"
+          src={frameSrc}
+          className="hidden"
+          ref={iframeRef}
+        />
         {children}
       </ApiProxyStateContext.Provider>
     </ApiProxyContext.Provider>

src/lib/context/__mocks__/defaultConfig.ts

@@ -3,7 +3,6 @@ import type {Configuration} from 'toolbar/types/config';
 const defaultConfig: Configuration = {
   // ConnectionConfig
   sentryOrigin: 'https://sentry.io',
-  sentryApiPath: '/api/0',
 
   // FeatureFlagsConfig
   featureFlags: undefined,

src/lib/context/defaultConfig.ts

@@ -3,7 +3,6 @@ import type {Configuration} from 'toolbar/types/config';
 const defaultConfig: Configuration = {
   // ConnectionConfig
   sentryOrigin: 'https://test.sentry.io',
-  sentryApiPath: '/api/0',
 
   // FeatureFlagsConfig
   featureFlags: undefined,

src/lib/hooks/fetch/useSentryApi.ts

@@ -1,8 +1,6 @@
 import qs from 'query-string';
-import {useContext, useMemo} from 'react';
+import {useMemo} from 'react';
 import {useApiProxyInstance} from 'toolbar/context/ApiProxyContext';
-import ConfigContext from 'toolbar/context/ConfigContext';
-import {getSentryApiBaseUrl} from 'toolbar/sentryApi/urls';
 import type {ApiEndpointQueryKey, ApiResult} from 'toolbar/types/api';
 import parseLinkHeader from 'toolbar/utils/parseLinkHeader';
 import type {ParsedHeader} from 'toolbar/utils/parseLinkHeader';
@@ -29,14 +27,11 @@ interface InfiniteFetchParams extends FetchParams {
 
 export default function useSentryApi<Data>() {
   const apiProxy = useApiProxyInstance();
-  const config = useContext(ConfigContext);
-
-  const apiBaseUrl = getSentryApiBaseUrl(config);
 
   const fetchFn = useMemo(
     () =>
       async ({/* signal, */ queryKey: [endpoint, options]}: FetchParams): Promise<ApiResult<Data>> => {
-        const url = qs.stringifyUrl({url: apiBaseUrl + endpoint, query: options?.query});
+        const path = qs.stringifyUrl({url: endpoint, query: options?.query});
         const contentType = options?.payload ? {'Content-Type': 'application/json'} : {};
         const init = {
           body: options?.payload ? JSON.stringify(options?.payload) : undefined,
@@ -49,7 +44,7 @@ export default function useSentryApi<Data>() {
         };
 
         const signal = new AbortController().signal; // TODO: nothing is cancellable with this signal
-        const response = (await apiProxy.exec(signal, 'fetch', [url, init])) as Omit<ApiResult, 'json'>;
+        const response = (await apiProxy.exec(signal, 'fetch', [path, init])) as Omit<ApiResult, 'json'>;
         const apiResult = {
           ...response,
           json: tryJsonParse(response.text),
@@ -66,7 +61,7 @@ export default function useSentryApi<Data>() {
         }
         return apiResult;
       },
-    [apiBaseUrl, apiProxy]
+    [apiProxy]
   );
 
   const fetchInfiniteFn = useMemo(

src/lib/hooks/usePrevious.spec.ts

@@ -0,0 +1,30 @@
+import {renderHook} from '@testing-library/react';
+import usePrevious from 'toolbar/hooks/usePrevious';
+
+describe('usePrevious', () => {
+  it('stores initial value', () => {
+    const {result} = renderHook(usePrevious, {initialProps: 'Initial Value'});
+    expect(result.current).toBe('Initial Value');
+  });
+
+  it('provides initial value', () => {
+    const {result} = renderHook(usePrevious, {
+      initialProps: 'Initial Value',
+    });
+
+    expect(result.current).toBe('Initial Value');
+  });
+
+  it('provides previous value', () => {
+    const {result, rerender} = renderHook(usePrevious<string | undefined>, {
+      initialProps: undefined,
+    });
+
+    rerender('New Value');
+    // We did not pass anything under initialProps
+    expect(result.current).toBe(undefined);
+    rerender('New New Value');
+    // Result should point to old value
+    expect(result.current).toBe('New Value');
+  });
+});

src/lib/hooks/usePrevious.ts

@@ -0,0 +1,23 @@
+import {useEffect, useRef} from 'react';
+
+/**
+ * Provides previous prop or state inside of function components.
+ * It’s possible that in the future React will provide a usePrevious Hook out of the box since it’s a relatively common use case.
+ * @see {@link https://reactjs.org/docs/hooks-faq.html#how-to-get-the-previous-props-or-state}
+ *
+ * @returns 'ref.current' and therefore should not be used as a dependency of useEffect.
+ * Mutable values like 'ref.current' are not valid dependencies of useEffect because changing them does not re-render the component.
+ */
+function usePrevious<T>(value: T): T {
+  // The ref object is a generic container whose current property is mutable ...
+  // ... and can hold any value, similar to an instance property on a class
+  const ref = useRef<T>(value);
+  // Store current value in ref
+  useEffect(() => {
+    ref.current = value;
+  }, [value]); // Only re-run if value changes
+  // Return previous value (happens before update in useEffect above)
+  return ref.current;
+}
+
+export default usePrevious;

src/lib/sentryApi/urls.spec.ts

@@ -1,22 +1,16 @@
 import defaultConfig from 'toolbar/context/defaultConfig';
-import {getSentryApiBaseUrl, getSentryWebOrigin, getSentryIFrameOrigin} from 'toolbar/sentryApi/urls';
+import {getSentryWebOrigin, getSentryIFrameOrigin} from 'toolbar/sentryApi/urls';
 import type {Configuration} from 'toolbar/types/config';
 
-type TestCase = [
-  string,
-  Partial<Configuration>,
-  {getSentryApiBaseUrl: string; getSentryWebOrigin: string; getSentryIFrameOrigin: string},
-];
+type TestCase = [string, Partial<Configuration>, {getSentryWebOrigin: string; getSentryIFrameOrigin: string}];
 const testCases: TestCase[] = [
   [
     'SaaS config: root',
     {
       sentryOrigin: 'https://sentry.io',
-      sentryApiPath: '/api/0/',
       organizationSlug: 'acme',
     },
     {
-      getSentryApiBaseUrl: 'https://acme.sentry.io/api/0/',
       getSentryWebOrigin: 'https://acme.sentry.io',
       getSentryIFrameOrigin: 'https://acme.sentry.io',
     },
@@ -25,11 +19,9 @@ const testCases: TestCase[] = [
     'SaaS config: subdomain',
     {
       sentryOrigin: 'https://acme.sentry.io',
-      sentryApiPath: '/api/0/',
       organizationSlug: 'acme',
     },
     {
-      getSentryApiBaseUrl: 'https://acme.sentry.io/api/0/',
       getSentryWebOrigin: 'https://acme.sentry.io',
       getSentryIFrameOrigin: 'https://acme.sentry.io',
     },
@@ -38,11 +30,9 @@ const testCases: TestCase[] = [
     'sentry devserver config',
     {
       sentryOrigin: 'https://dev.getsentry.net:8000',
-      sentryApiPath: '/api/0',
       organizationSlug: 'acme',
     },
     {
-      getSentryApiBaseUrl: 'https://dev.getsentry.net:8000/api/0',
       getSentryWebOrigin: 'https://dev.getsentry.net:8000',
       getSentryIFrameOrigin: 'https://dev.getsentry.net:8000',
     },
@@ -51,23 +41,15 @@ const testCases: TestCase[] = [
     'yarn dev:standalone config',
     {
       sentryOrigin: 'http://localhost:8080',
-      sentryApiPath: '/api/0',
       organizationSlug: 'acme',
     },
     {
-      getSentryApiBaseUrl: 'http://localhost:8080/api/0',
       getSentryWebOrigin: 'http://localhost:8080',
       getSentryIFrameOrigin: 'http://localhost:8080',
     },
   ],
 ];
 
-describe('getSentryApiBaseUrl', () => {
-  it.each<TestCase>(testCases)('should get the correct url for api requests: %s', (_title, config, expected) => {
-    expect(getSentryApiBaseUrl({...defaultConfig, ...config})).toBe(expected.getSentryApiBaseUrl);
-  });
-});
-
 describe('getSentryWebOrigin', () => {
   it.each<TestCase>(testCases)('should get the correct url for web requests: %s', (_title, config, expected) => {
     expect(getSentryWebOrigin({...defaultConfig, ...config})).toBe(expected.getSentryWebOrigin);

src/lib/sentryApi/urls.ts

@@ -16,42 +16,6 @@ function isSaasOrigin(hostname: string) {
   return isSaasOriginWithSubdomain(hostname) || hostname === 'sentry.io';
 }
 
-/**
- * Given a configuration, we want to return the base URL for API calls
- */
-export function getSentryApiBaseUrl(config: Configuration) {
-  const {organizationSlug, sentryOrigin, sentryApiPath} = config;
-
-  const origin = getOrigin(sentryOrigin);
-  if (!origin) {
-    return sentryOrigin;
-  }
-
-  const parts = [origin.protocol, '//'];
-
-  if (isSaasOrigin(origin.hostname)) {
-    // Ignore `${region}.sentry.io` for now, because we don't have support in the iframe.
-    // Instead we'll use the org slug as the subdomain.
-    // What could happen is we read the region on the server-side, and then the
-    // iframe.html template could set the cookie on the correct place, and return
-    // the iframe api-url to the toolbar directly.
-    // The org api response calls it `links.regionUrl`
-    parts.push(`${organizationSlug}.sentry.io`);
-  } else {
-    parts.push(origin.hostname);
-  }
-
-  if (origin.port) {
-    parts.push(`:${origin.port}`);
-  }
-  if (origin.pathname) {
-    parts.push(origin.pathname.replace(/\/$/, ''));
-  }
-
-  parts.push(sentryApiPath ?? '');
-  return parts.join('');
-}
-
 /**
  * Returns an origin with organization subdomain included, if SaaS is detected.
  */

src/lib/types/config.ts

@@ -11,18 +11,6 @@ interface ConnectionConfig {
    * Must not have a trailing backslash.
    */
   sentryOrigin: string;
-
-  /**
-   * The path prefix for the api endpoints.
-   *
-   * Example: `/api/0`
-   *
-   * Must not have a trailing backslash.
-   *
-   * You could write API_PATH=/region/us/api/0, and leave REGION blank but
-   * that's discouraged because it's not as clear as using the individual vars.
-   */
-  sentryApiPath: string | undefined;
 }
 
 interface FeatureFlagsConfig {