Skip to content

Commit a25383c

Browse files
chargomechargome
authored
Merge branch 'develop' into cg-next-webpack-on-build-end
2 parents cee39bd + d039640 commit a25383c

File tree

31 files changed

+522
-279
lines changed

31 files changed

+522
-279
lines changed

‎dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/init.js‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import * as Sentry from '@sentry/browser';
2+
// eslint-disable-next-line import/no-duplicates
3+
import { thirdPartyErrorFilterIntegration } from '@sentry/browser';
4+
// eslint-disable-next-line import/no-duplicates
5+
import { captureConsoleIntegration } from '@sentry/browser';
6+
7+
// This is the code the bundler plugin would inject to mark the init bundle as a first party module:
8+
var _sentryModuleMetadataGlobal =
9+
typeof window !== 'undefined'
10+
? window
11+
: typeof global !== 'undefined'
12+
? global
13+
: typeof self !== 'undefined'
14+
? self
15+
: {};
16+
17+
_sentryModuleMetadataGlobal._sentryModuleMetadata = _sentryModuleMetadataGlobal._sentryModuleMetadata || {};
18+
19+
_sentryModuleMetadataGlobal._sentryModuleMetadata[new Error().stack] = Object.assign(
20+
{},
21+
_sentryModuleMetadataGlobal._sentryModuleMetadata[new Error().stack],
22+
{
23+
'_sentryBundlerPluginAppKey:my-app': true,
24+
},
25+
);
26+
27+
Sentry.init({
28+
dsn: 'https://[email protected]/1337',
29+
integrations: [
30+
thirdPartyErrorFilterIntegration({ behaviour: 'apply-tag-if-contains-third-party-frames', filterKeys: ['my-app'] }),
31+
captureConsoleIntegration({ levels: ['error'], handled: false }),
32+
],
33+
attachStacktrace: true,
34+
});

‎dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/subject.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// This is the code the bundler plugin would inject to mark the subject bundle as a first party module:
2+
var _sentryModuleMetadataGlobal =
3+
typeof window !== 'undefined'
4+
? window
5+
: typeof global !== 'undefined'
6+
? global
7+
: typeof self !== 'undefined'
8+
? self
9+
: {};
10+
11+
_sentryModuleMetadataGlobal._sentryModuleMetadata = _sentryModuleMetadataGlobal._sentryModuleMetadata || {};
12+
13+
_sentryModuleMetadataGlobal._sentryModuleMetadata[new Error().stack] = Object.assign(
14+
{},
15+
_sentryModuleMetadataGlobal._sentryModuleMetadata[new Error().stack],
16+
{
17+
'_sentryBundlerPluginAppKey:my-app': true,
18+
},
19+
);
20+
21+
const errorBtn = document.getElementById('errBtn');
22+
errorBtn.addEventListener('click', async () => {
23+
Promise.allSettled([Promise.reject('I am a first party Error')]).then(values =>
24+
values.forEach(value => {
25+
if (value.status === 'rejected') console.error(value.reason);
26+
}),
27+
);
28+
});

‎dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/template.html‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<!doctype html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<script src="thirdPartyScript.js"></script>
8+
<button id="errBtn">Throw 1st part yerror</button>
9+
</body>
10+
</html>

‎dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/test.ts‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import { readFileSync } from 'node:fs';
2+
import { join } from 'node:path';
3+
import { expect } from '@playwright/test';
4+
import { sentryTest } from '../../../utils/fixtures';
5+
import { envelopeRequestParser, waitForErrorRequest } from '../../../utils/helpers';
6+
7+
const bundle = process.env.PW_BUNDLE || '';
8+
// We only want to run this in non-CDN bundle mode because
9+
// thirdPartyErrorFilterIntegration is only available in the NPM package
10+
if (bundle.startsWith('bundle')) {
11+
sentryTest.skip();
12+
}
13+
14+
sentryTest('tags event if contains at least one third-party frame', async ({ getLocalTestUrl, page }) => {
15+
const url = await getLocalTestUrl({ testDir: __dirname });
16+
17+
const errorEventPromise = waitForErrorRequest(page, e => {
18+
return e.exception?.values?.[0]?.value === 'I am a third party Error';
19+
});
20+
21+
await page.route('**/thirdPartyScript.js', route =>
22+
route.fulfill({
23+
status: 200,
24+
body: readFileSync(join(__dirname, 'thirdPartyScript.js')),
25+
}),
26+
);
27+
28+
await page.goto(url);
29+
30+
const errorEvent = envelopeRequestParser(await errorEventPromise);
31+
expect(errorEvent.tags?.third_party_code).toBe(true);
32+
});
33+
34+
/**
35+
* This test seems a bit more complicated than necessary but this is intentional:
36+
* When using `captureConsoleIntegration` in combination with `thirdPartyErrorFilterIntegration`
37+
* and `attachStacktrace: true`, the stack trace includes native code stack frames which previously broke
38+
* the third party error filtering logic.
39+
*
40+
* see https://github.com/getsentry/sentry-javascript/issues/17674
41+
*/
42+
sentryTest(
43+
"doesn't tag event if doesn't contain third-party frames",
44+
async ({ getLocalTestUrl, page, browserName }) => {
45+
const url = await getLocalTestUrl({ testDir: __dirname });
46+
47+
const errorEventPromise = waitForErrorRequest(page, e => {
48+
return e.exception?.values?.[0]?.value === 'I am a first party Error';
49+
});
50+
51+
await page.route('**/thirdPartyScript.js', route =>
52+
route.fulfill({
53+
status: 200,
54+
body: readFileSync(join(__dirname, 'thirdPartyScript.js')),
55+
}),
56+
);
57+
58+
await page.goto(url);
59+
60+
await page.click('#errBtn');
61+
62+
const errorEvent = envelopeRequestParser(await errorEventPromise);
63+
64+
expect(errorEvent.tags?.third_party_code).toBeUndefined();
65+
66+
// ensure the stack trace includes native code stack frames which previously broke
67+
// the third party error filtering logic
68+
if (browserName === 'chromium') {
69+
expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames).toContainEqual({
70+
filename: '<anonymous>',
71+
function: 'Array.forEach',
72+
in_app: true,
73+
});
74+
} else if (browserName === 'webkit') {
75+
expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames).toContainEqual({
76+
filename: '[native code]',
77+
function: 'forEach',
78+
in_app: true,
79+
});
80+
}
81+
},
82+
);

‎dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/thirdPartyScript.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
setTimeout(() => {
2+
throw new Error('I am a third party Error');
3+
}, 100);

‎packages/browser/test/profiling/integration.test.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
* @vitest-environment jsdom
33
*/
44

5-
import type { BrowserClient } from '@sentry/browser';
65
import * as Sentry from '@sentry/browser';
76
import { describe, expect, it, vi } from 'vitest';
87
import type { JSSelfProfile } from '../../src/profiling/jsSelfProfiling';
@@ -36,7 +35,7 @@ describe('BrowserProfilingIntegration', () => {
3635

3736
const flush = vi.fn().mockImplementation(() => Promise.resolve(true));
3837
const send = vi.fn().mockImplementation(() => Promise.resolve());
39-
Sentry.init({
38+
const client = Sentry.init({
4039
tracesSampleRate: 1,
4140
profilesSampleRate: 1,
4241
environment: 'test-environment',
@@ -50,13 +49,11 @@ describe('BrowserProfilingIntegration', () => {
5049
integrations: [Sentry.browserTracingIntegration(), Sentry.browserProfilingIntegration()],
5150
});
5251

53-
const client = Sentry.getClient<BrowserClient>();
54-
5552
const currentTransaction = Sentry.getActiveSpan();
5653
expect(currentTransaction).toBeDefined();
5754
expect(Sentry.spanToJSON(currentTransaction!).op).toBe('pageload');
5855
currentTransaction?.end();
59-
await client?.flush(1000);
56+
await client!.flush(1000);
6057

6158
expect(send).toHaveBeenCalledTimes(1);
6259

‎packages/bun/src/transports/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { BaseTransportOptions, Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/core';
2-
import { createTransport, rejectedSyncPromise, suppressTracing } from '@sentry/core';
2+
import { createTransport, suppressTracing } from '@sentry/core';
33

44
export interface BunTransportOptions extends BaseTransportOptions {
55
/** Custom headers for the transport. Used by the XHRTransport and FetchTransport */
@@ -30,7 +30,7 @@ export function makeFetchTransport(options: BunTransportOptions): Transport {
3030
});
3131
});
3232
} catch (e) {
33-
return rejectedSyncPromise(e);
33+
return Promise.reject(e);
3434
}
3535
}
3636

‎packages/core/src/client.ts‎

Lines changed: 38 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ import { parseSampleRate } from './utils/parseSampleRate';
4444
import { prepareEvent } from './utils/prepareEvent';
4545
import { reparentChildSpans, shouldIgnoreSpan } from './utils/should-ignore-span';
4646
import { getActiveSpan, showSpanDropWarning, spanToTraceContext } from './utils/spanUtils';
47-
import { rejectedSyncPromise, resolvedSyncPromise, SyncPromise } from './utils/syncpromise';
47+
import { rejectedSyncPromise } from './utils/syncpromise';
4848
import { convertSpanJsonToTransactionEvent, convertTransactionEventToSpanJson } from './utils/transactionEvent';
4949

5050
const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been captured.";
@@ -316,16 +316,19 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
316316
* @returns A promise that will resolve with `true` if all events are sent before the timeout, or `false` if there are
317317
* still events in the queue when the timeout is reached.
318318
*/
319-
public flush(timeout?: number): PromiseLike<boolean> {
319+
// @ts-expect-error - PromiseLike is a subset of Promise
320+
public async flush(timeout?: number): PromiseLike<boolean> {
320321
const transport = this._transport;
321-
if (transport) {
322-
this.emit('flush');
323-
return this._isClientDoneProcessing(timeout).then(clientFinished => {
324-
return transport.flush(timeout).then(transportFlushed => clientFinished && transportFlushed);
325-
});
326-
} else {
327-
return resolvedSyncPromise(true);
322+
if (!transport) {
323+
return true;
328324
}
325+
326+
this.emit('flush');
327+
328+
const clientFinished = await this._isClientDoneProcessing(timeout);
329+
const transportFlushed = await transport.flush(timeout);
330+
331+
return clientFinished && transportFlushed;
329332
}
330333

331334
/**
@@ -336,12 +339,12 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
336339
* @returns {Promise<boolean>} A promise which resolves to `true` if the flush completes successfully before the timeout, or `false` if
337340
* it doesn't.
338341
*/
339-
public close(timeout?: number): PromiseLike<boolean> {
340-
return this.flush(timeout).then(result => {
341-
this.getOptions().enabled = false;
342-
this.emit('close');
343-
return result;
344-
});
342+
// @ts-expect-error - PromiseLike is a subset of Promise
343+
public async close(timeout?: number): PromiseLike<boolean> {
344+
const result = await this.flush(timeout);
345+
this.getOptions().enabled = false;
346+
this.emit('close');
347+
return result;
345348
}
346349

347350
/**
@@ -872,18 +875,21 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
872875
/**
873876
* Send an envelope to Sentry.
874877
*/
875-
public sendEnvelope(envelope: Envelope): PromiseLike<TransportMakeRequestResponse> {
878+
// @ts-expect-error - PromiseLike is a subset of Promise
879+
public async sendEnvelope(envelope: Envelope): PromiseLike<TransportMakeRequestResponse> {
876880
this.emit('beforeEnvelope', envelope);
877881

878882
if (this._isEnabled() && this._transport) {
879-
return this._transport.send(envelope).then(null, reason => {
883+
try {
884+
return await this._transport.send(envelope);
885+
} catch (reason) {
880886
DEBUG_BUILD && debug.error('Error while sending envelope:', reason);
881887
return {};
882-
});
888+
}
883889
}
884890

885891
DEBUG_BUILD && debug.error('Transport disabled');
886-
return resolvedSyncPromise({});
892+
return {};
887893
}
888894

889895
/* eslint-enable @typescript-eslint/unified-signatures */
@@ -938,24 +944,20 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
938944
* @returns A promise which will resolve to `true` if processing is already done or finishes before the timeout, and
939945
* `false` otherwise
940946
*/
941-
protected _isClientDoneProcessing(timeout?: number): PromiseLike<boolean> {
942-
return new SyncPromise(resolve => {
943-
let ticked: number = 0;
944-
const tick: number = 1;
947+
protected async _isClientDoneProcessing(timeout?: number): Promise<boolean> {
948+
let ticked = 0;
945949

946-
const interval = setInterval(() => {
947-
if (this._numProcessing == 0) {
948-
clearInterval(interval);
949-
resolve(true);
950-
} else {
951-
ticked += tick;
952-
if (timeout && ticked >= timeout) {
953-
clearInterval(interval);
954-
resolve(false);
955-
}
956-
}
957-
}, tick);
958-
});
950+
// if no timeout is provided, we wait "forever" until everything is processed
951+
while (!timeout || ticked < timeout) {
952+
await new Promise(resolve => setTimeout(resolve, 1));
953+
954+
if (!this._numProcessing) {
955+
return true;
956+
}
957+
ticked++;
958+
}
959+
960+
return false;
959961
}
960962

961963
/** Determines whether this SDK is enabled and a transport is present. */

‎packages/core/src/integrations/third-party-errors-filter.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,9 @@ function getBundleKeysForAllFramesWithFilenames(event: Event): string[][] | unde
108108

109109
return (
110110
frames
111-
// Exclude frames without a filename since these are likely native code or built-ins
112-
.filter(frame => !!frame.filename)
111+
// Exclude frames without a filename or without lineno and colno,
112+
// since these are likely native code or built-ins
113+
.filter(frame => !!frame.filename && (frame.lineno ?? frame.colno) != null)
113114
.map(frame => {
114115
if (frame.module_metadata) {
115116
return Object.keys(frame.module_metadata)

‎packages/core/src/transports/base.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import {
1616
} from '../utils/envelope';
1717
import { type PromiseBuffer, makePromiseBuffer, SENTRY_BUFFER_FULL_ERROR } from '../utils/promisebuffer';
1818
import { type RateLimits, isRateLimited, updateRateLimits } from '../utils/ratelimit';
19-
import { resolvedSyncPromise } from '../utils/syncpromise';
2019

2120
export const DEFAULT_TRANSPORT_BUFFER_SIZE = 64;
2221

@@ -51,7 +50,7 @@ export function createTransport(
5150

5251
// Skip sending if envelope is empty after filtering out rate limited events
5352
if (filteredEnvelopeItems.length === 0) {
54-
return resolvedSyncPromise({});
53+
return Promise.resolve({});
5554
}
5655

5756
const filteredEnvelope: Envelope = createEnvelope(envelope[0], filteredEnvelopeItems as (typeof envelope)[1]);
@@ -87,7 +86,7 @@ export function createTransport(
8786
if (error === SENTRY_BUFFER_FULL_ERROR) {
8887
DEBUG_BUILD && debug.error('Skipped sending event because buffer is full.');
8988
recordEnvelopeLoss('queue_overflow');
90-
return resolvedSyncPromise({});
89+
return Promise.resolve({});
9190
} else {
9291
throw error;
9392
}

0 commit comments

Comments
 (0)