fix(sveltekit): Only instrument fetch if the client is valid

getsentry · Jun 22, 2023 · 0b79777 · 0b79777
1 parent da2487e
commit 0b79777
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 11 deletions.
diff --git a/packages/sveltekit/src/client/load.ts b/packages/sveltekit/src/client/load.ts
@@ -3,7 +3,7 @@ import type { BaseClient } from '@sentry/core';
 import { getCurrentHub, trace } from '@sentry/core';
 import type { Breadcrumbs, BrowserTracing } from '@sentry/svelte';
 import { captureException } from '@sentry/svelte';
-import type { ClientOptions, SanitizedRequestData } from '@sentry/types';
+import type { Client, ClientOptions, SanitizedRequestData } from '@sentry/types';
 import {
   addExceptionMechanism,
   addNonEnumerableProperty,
@@ -122,12 +122,14 @@ type SvelteKitFetch = LoadEvent['fetch'];
  * @returns a proxy of SvelteKit's fetch implementation
  */
 function instrumentSvelteKitFetch(originalFetch: SvelteKitFetch): SvelteKitFetch {
-  const client = getCurrentHub().getClient() as BaseClient<ClientOptions>;
+  const client = getCurrentHub().getClient();
 
-  const browserTracingIntegration =
-    client.getIntegrationById && (client.getIntegrationById('BrowserTracing') as BrowserTracing | undefined);
-  const breadcrumbsIntegration =
-    client.getIntegrationById && (client.getIntegrationById('Breadcrumbs') as Breadcrumbs | undefined);
+  if (!isValidClient(client)) {
+    return originalFetch;
+  }
+
+  const browserTracingIntegration = client.getIntegrationById('BrowserTracing') as BrowserTracing | undefined;
+  const breadcrumbsIntegration = client.getIntegrationById('Breadcrumbs') as Breadcrumbs | undefined;
 
   const browserTracingOptions = browserTracingIntegration && browserTracingIntegration.options;
 
@@ -270,3 +272,14 @@ function addFetchBreadcrumb(
     },
   );
 }
+
+type MaybeClientWithGetIntegrationsById =
+  | (Client & { getIntegrationById?: BaseClient<ClientOptions>['getIntegrationById'] })
+  | undefined;
+
+type ClientWithGetIntegrationById = Required<MaybeClientWithGetIntegrationsById> &
+  Exclude<MaybeClientWithGetIntegrationsById, undefined>;
+
+function isValidClient(client: MaybeClientWithGetIntegrationsById): client is ClientWithGetIntegrationById {
+  return !!client && typeof client.getIntegrationById === 'function';
+}
diff --git a/packages/sveltekit/test/client/load.test.ts b/packages/sveltekit/test/client/load.test.ts
@@ -1,3 +1,4 @@
+import * as sentryCore from '@sentry/core';
 import { addTracingExtensions, Scope } from '@sentry/svelte';
 import { baggageHeaderToDynamicSamplingContext } from '@sentry/utils';
 import type { Load } from '@sveltejs/kit';
@@ -52,6 +53,12 @@ const mockedGetIntegrationById = vi.fn(id => {
   return undefined;
 });
 
+const mockedGetClient = vi.fn(() => {
+  return {
+    getIntegrationById: mockedGetIntegrationById,
+  };
+});
+
 vi.mock('@sentry/core', async () => {
   const original = (await vi.importActual('@sentry/core')) as any;
   return {
@@ -62,11 +69,7 @@ vi.mock('@sentry/core', async () => {
     },
     getCurrentHub: () => {
       return {
-        getClient: () => {
-          return {
-            getIntegrationById: mockedGetIntegrationById,
-          };
-        },
+        getClient: mockedGetClient,
         getScope: () => {
           return {
             getSpan: () => {
@@ -427,6 +430,28 @@ describe('wrapLoadWithSentry', () => {
     });
   });
 
+  it.each([
+    ['is undefined', undefined],
+    ["doesn't have a `getClientById` method", {}],
+  ])("doesn't instrument fetch if the client %s", async (_, client) => {
+    // @ts-expect-error: we're mocking the client
+    mockedGetClient.mockImplementationOnce(() => client);
+
+    async function load(_event: Parameters<Load>[0]): Promise<ReturnType<Load>> {
+      return {
+        msg: 'hi',
+      };
+    }
+    const wrappedLoad = wrapLoadWithSentry(load);
+
+    const originalFetch = MOCK_LOAD_ARGS.fetch;
+    await wrappedLoad(MOCK_LOAD_ARGS);
+
+    expect(MOCK_LOAD_ARGS.fetch).toStrictEqual(originalFetch);
+
+    expect(mockTrace).toHaveBeenCalledTimes(1);
+  });
+
   it('adds an exception mechanism', async () => {
     const addEventProcessorSpy = vi.spyOn(mockScope, 'addEventProcessor').mockImplementationOnce(callback => {
       void callback({}, { event_id: 'fake-event-id' });