fix(browser): Avoid 4xx response for succesful diagnoseSdkConnectivity request (#16840)

Our browser SDKs expose a `diagnoseSdkConnectivity` function, whose
primary purpose is to be called by the example pages injected via the
wizard. As reported in #16226, even a successful request to Sentry (i.e.
no ad blockers or other parties blocked the request) gets logged to the
console because the previous URL included an invalid public key (as well
as org id). This did not cause a `fetch` error but just a 400 (bad
request) response. However, browsers log 4xx fetch requests to the
console and users mistook this console log for an incorrectly configured
SDK setup.

This patch now changes the key to a valid but deactivated public key in a
new project in the `sentry-sdks` org. All of the information in this URL
is public and as safe to share as any DSN. So from this perspective, I
don't see concerns (happy to reconsider if reviewers have concerns). It
seems like we return 200 even for deactivated keys, which means that
now, there's no console log anymore for successful requests.

closes #16226

Author: Lms24

packages/browser/src/diagnose-sdk.ts

@@ -25,9 +25,12 @@ export async function diagnoseSdkConnectivity(): Promise<
   try {
     // If fetch throws, there is likely an ad blocker active or there are other connective issues.
     await fetch(
-      // We want this to be as close as possible to an actual ingest URL so that ad blockers will actually block the request
-      // We are using the "sentry-sdks" org with id 447951 not to pollute any actual organizations.
-      'https://o447951.ingest.sentry.io/api/1337/envelope/?sentry_version=7&sentry_key=1337&sentry_client=sentry.javascript.browser%2F1.33.7',
+      // We are using the
+      // - "sentry-sdks" org with id 447951 not to pollute any actual organizations.
+      // - "diagnose-sdk-connectivity" project with id 4509632503087104
+      // - the public key of said org/project, which is disabled in the project settings
+      // => this DSN: https://c1dfb07d783ad5325c245c1fd3725390@o447951.ingest.us.sentry.io/4509632503087104 (i.e. disabled)
+      'https://o447951.ingest.sentry.io/api/4509632503087104/envelope/?sentry_version=7&sentry_key=c1dfb07d783ad5325c245c1fd3725390&sentry_client=sentry.javascript.browser%2F1.33.7',
       {
         body: '{}',
         method: 'POST',

packages/browser/test/diagnose-sdk.test.ts

@@ -0,0 +1,165 @@
+/**
+ * @vitest-environment jsdom
+ */
+
+import type { Client } from '@sentry/core';
+import * as sentryCore from '@sentry/core';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { diagnoseSdkConnectivity } from '../src/diagnose-sdk';
+
+// Mock the @sentry/core module
+vi.mock('@sentry/core', async requireActual => {
+  return {
+    ...((await requireActual()) as any),
+    getClient: vi.fn(),
+  };
+});
+
+// Mock global fetch
+const mockFetch = vi.fn();
+global.fetch = mockFetch;
+
+describe('diagnoseSdkConnectivity', () => {
+  const mockGetClient = sentryCore.getClient as any;
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  afterEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it('returns "no-client-active" when no client is active', async () => {
+    mockGetClient.mockReturnValue(undefined);
+
+    const result = await diagnoseSdkConnectivity();
+
+    expect(result).toBe('no-client-active');
+    expect(mockFetch).not.toHaveBeenCalled();
+  });
+
+  it('returns "no-dsn-configured" when client.getDsn() returns undefined', async () => {
+    const mockClient: Partial<Client> = {
+      getDsn: vi.fn().mockReturnValue(undefined),
+    };
+    mockGetClient.mockReturnValue(mockClient);
+
+    const result = await diagnoseSdkConnectivity();
+
+    expect(result).toBe('no-dsn-configured');
+    expect(mockClient.getDsn).toHaveBeenCalled();
+    expect(mockFetch).not.toHaveBeenCalled();
+  });
+
+  it('returns "sentry-unreachable" when fetch throws an error', async () => {
+    const mockClient: Partial<Client> = {
+      getDsn: vi.fn().mockReturnValue('https://test@example.com/123'),
+    };
+    mockGetClient.mockReturnValue(mockClient);
+    mockFetch.mockRejectedValue(new Error('Network error'));
+
+    const result = await diagnoseSdkConnectivity();
+
+    expect(result).toBe('sentry-unreachable');
+    expect(mockClient.getDsn).toHaveBeenCalled();
+    expect(mockFetch).toHaveBeenCalledWith(
+      'https://o447951.ingest.sentry.io/api/4509632503087104/envelope/?sentry_version=7&sentry_key=c1dfb07d783ad5325c245c1fd3725390&sentry_client=sentry.javascript.browser%2F1.33.7',
+      {
+        body: '{}',
+        method: 'POST',
+        mode: 'cors',
+        credentials: 'omit',
+      },
+    );
+  });
+
+  it('returns "sentry-unreachable" when fetch throws a TypeError (common for network issues)', async () => {
+    const mockClient: Partial<Client> = {
+      getDsn: vi.fn().mockReturnValue('https://test@example.com/123'),
+    };
+    mockGetClient.mockReturnValue(mockClient);
+    mockFetch.mockRejectedValue(new TypeError('Failed to fetch'));
+
+    const result = await diagnoseSdkConnectivity();
+
+    expect(result).toBe('sentry-unreachable');
+    expect(mockClient.getDsn).toHaveBeenCalled();
+    expect(mockFetch).toHaveBeenCalled();
+  });
+
+  it('returns undefined when connectivity check succeeds', async () => {
+    const mockClient: Partial<Client> = {
+      getDsn: vi.fn().mockReturnValue('https://test@example.com/123'),
+    };
+    mockGetClient.mockReturnValue(mockClient);
+    mockFetch.mockResolvedValue(new Response('{}', { status: 200 }));
+
+    const result = await diagnoseSdkConnectivity();
+
+    expect(result).toBeUndefined();
+    expect(mockClient.getDsn).toHaveBeenCalled();
+    expect(mockFetch).toHaveBeenCalledWith(
+      'https://o447951.ingest.sentry.io/api/4509632503087104/envelope/?sentry_version=7&sentry_key=c1dfb07d783ad5325c245c1fd3725390&sentry_client=sentry.javascript.browser%2F1.33.7',
+      {
+        body: '{}',
+        method: 'POST',
+        mode: 'cors',
+        credentials: 'omit',
+      },
+    );
+  });
+
+  it('returns undefined even when fetch returns an error status (4xx, 5xx)', async () => {
+    const mockClient: Partial<Client> = {
+      getDsn: vi.fn().mockReturnValue('https://test@example.com/123'),
+    };
+    mockGetClient.mockReturnValue(mockClient);
+    // Mock a 403 response (expected since the DSN is disabled)
+    mockFetch.mockResolvedValue(new Response('Forbidden', { status: 403 }));
+
+    const result = await diagnoseSdkConnectivity();
+
+    // The function only cares about fetch not throwing, not the response status
+    expect(result).toBeUndefined();
+    expect(mockClient.getDsn).toHaveBeenCalled();
+    expect(mockFetch).toHaveBeenCalled();
+  });
+
+  it('uses the correct test endpoint URL', async () => {
+    const mockClient: Partial<Client> = {
+      getDsn: vi.fn().mockReturnValue('https://test@example.com/123'),
+    };
+    mockGetClient.mockReturnValue(mockClient);
+    mockFetch.mockResolvedValue(new Response('{}', { status: 200 }));
+
+    await diagnoseSdkConnectivity();
+
+    expect(mockFetch).toHaveBeenCalledWith(
+      'https://o447951.ingest.sentry.io/api/4509632503087104/envelope/?sentry_version=7&sentry_key=c1dfb07d783ad5325c245c1fd3725390&sentry_client=sentry.javascript.browser%2F1.33.7',
+      expect.objectContaining({
+        body: '{}',
+        method: 'POST',
+        mode: 'cors',
+        credentials: 'omit',
+      }),
+    );
+  });
+
+  it('uses correct fetch options', async () => {
+    const mockClient: Partial<Client> = {
+      getDsn: vi.fn().mockReturnValue('https://test@example.com/123'),
+    };
+    mockGetClient.mockReturnValue(mockClient);
+    mockFetch.mockResolvedValue(new Response('{}', { status: 200 }));
+
+    await diagnoseSdkConnectivity();
+
+    expect(mockFetch).toHaveBeenCalledWith(expect.any(String), {
+      body: '{}',
+      method: 'POST',
+      mode: 'cors',
+      credentials: 'omit',
+    });
+  });
+});