Skip to content

Commit 8f4d56f

Browse files
Lms24andreiborza
andauthored
fix(browser): Ensure request from diagnoseSdkConnectivity doesn't create span (#17280)
This patch ensures that we suppress tracing (and breadcrumbs) for `diagnoseSdkConnectivity`-made requests. The `suppressTracing` approach works well here. We could also get an native fetch implementation here but I'd say we only try this if for some reason `suppressTracing` doesn't work. --------- Co-authored-by: Andrei <[email protected]>
1 parent aa4547c commit 8f4d56f

File tree

5 files changed

+90
-15
lines changed

5 files changed

+90
-15
lines changed
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://[email protected]/1337',
7+
integrations: [Sentry.browserTracingIntegration({ idleTimeout: 3000, childSpanTimeout: 3000 })],
8+
tracesSampleRate: 1,
9+
});
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Sentry.diagnoseSdkConnectivity().then(res => console.log('SDK connectivity:', res));
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { expect } from '@playwright/test';
2+
import { sentryTest } from '../../../utils/fixtures';
3+
import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../utils/helpers';
4+
5+
sentryTest('makes a call to sentry.io to diagnose SDK connectivity', async ({ getLocalTestUrl, page }) => {
6+
const bundle = process.env.PW_BUNDLE as string | undefined;
7+
if (shouldSkipTracingTest() || !!bundle) {
8+
// the CDN bundle doesn't export diagnoseSdkConnectivity. So skipping the test for bundles.
9+
sentryTest.skip();
10+
}
11+
12+
const pageloadRequestPromise = waitForTransactionRequest(page, e => e.contexts?.trace?.op === 'pageload');
13+
14+
// mock sdk connectivity url to avoid making actual request to sentry.io
15+
page.route('**/api/4509632503087104/envelope/**/*', route => {
16+
return route.fulfill({
17+
status: 200,
18+
body: '{}',
19+
});
20+
});
21+
22+
const diagnoseMessagePromise = new Promise<string>(resolve => {
23+
page.on('console', msg => {
24+
if (msg.text().includes('SDK connectivity:')) {
25+
resolve(msg.text());
26+
}
27+
});
28+
});
29+
30+
const url = await getLocalTestUrl({ testDir: __dirname });
31+
await page.goto(url);
32+
33+
const pageLoadEvent = envelopeRequestParser(await pageloadRequestPromise);
34+
35+
// undefined is expected and means the request was successful
36+
expect(await diagnoseMessagePromise).toEqual('SDK connectivity: undefined');
37+
38+
// the request to sentry.io should not be traced, hence no http.client span should be sent.
39+
const httpClientSpans = pageLoadEvent.spans?.filter(s => s.op === 'http.client');
40+
expect(httpClientSpans).toHaveLength(0);
41+
42+
// no fetch breadcrumb should be sent (only breadcrumb for the console log)
43+
expect(pageLoadEvent.breadcrumbs).toEqual([
44+
expect.objectContaining({
45+
category: 'console',
46+
message: 'SDK connectivity: undefined',
47+
}),
48+
]);
49+
});

packages/browser/src/diagnose-sdk.ts

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getClient } from '@sentry/core';
1+
import { getClient, suppressTracing } from '@sentry/core';
22

33
/**
44
* A function to diagnose why the SDK might not be successfully sending data.
@@ -23,20 +23,22 @@ export async function diagnoseSdkConnectivity(): Promise<
2323
}
2424

2525
try {
26-
// If fetch throws, there is likely an ad blocker active or there are other connective issues.
27-
await fetch(
28-
// We are using the
29-
// - "sentry-sdks" org with id 447951 not to pollute any actual organizations.
30-
// - "diagnose-sdk-connectivity" project with id 4509632503087104
31-
// - the public key of said org/project, which is disabled in the project settings
32-
// => this DSN: https://[email protected]/4509632503087104 (i.e. disabled)
33-
'https://o447951.ingest.sentry.io/api/4509632503087104/envelope/?sentry_version=7&sentry_key=c1dfb07d783ad5325c245c1fd3725390&sentry_client=sentry.javascript.browser%2F1.33.7',
34-
{
35-
body: '{}',
36-
method: 'POST',
37-
mode: 'cors',
38-
credentials: 'omit',
39-
},
26+
await suppressTracing(() =>
27+
// If fetch throws, there is likely an ad blocker active or there are other connective issues.
28+
fetch(
29+
// We are using the
30+
// - "sentry-sdks" org with id 447951 not to pollute any actual organizations.
31+
// - "diagnose-sdk-connectivity" project with id 4509632503087104
32+
// - the public key of said org/project, which is disabled in the project settings
33+
// => this DSN: https://[email protected]/4509632503087104 (i.e. disabled)
34+
'https://o447951.ingest.sentry.io/api/4509632503087104/envelope/?sentry_version=7&sentry_key=c1dfb07d783ad5325c245c1fd3725390&sentry_client=sentry.javascript.browser%2F1.33.7',
35+
{
36+
body: '{}',
37+
method: 'POST',
38+
mode: 'cors',
39+
credentials: 'omit',
40+
},
41+
),
4042
);
4143
} catch {
4244
return 'sentry-unreachable';

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,4 +162,18 @@ describe('diagnoseSdkConnectivity', () => {
162162
credentials: 'omit',
163163
});
164164
});
165+
166+
it('calls suppressTracing to avoid tracing the fetch call to sentry', async () => {
167+
const suppressTracingSpy = vi.spyOn(sentryCore, 'suppressTracing');
168+
169+
const mockClient: Partial<Client> = {
170+
getDsn: vi.fn().mockReturnValue('https://[email protected]/123'),
171+
};
172+
mockGetClient.mockReturnValue(mockClient);
173+
mockFetch.mockResolvedValue(new Response('{}', { status: 200 }));
174+
175+
await diagnoseSdkConnectivity();
176+
177+
expect(suppressTracingSpy).toHaveBeenCalledTimes(1);
178+
});
165179
});

0 commit comments

Comments
 (0)