Skip to content

Commit 3415c80

Browse files
BeltranBulbarellaDDrgaignault
BeltranBulbarellaDD
and
rgaignault
authored
🐛[RUM-11247] Pass down error stack to isAllowedTrackingOrigins (#3824)
Moved error stack to the init function fix formatter Merge branch 'main' into beltran.bulbarella/RUM-11247_2 Add extra check for extension detection Revert "Add extra check for extension detection" This reverts commit f9dc26c. Add parse stack function Add tests constants Moved error stack to the first frame. Consisntency accross tests Merge branch 'main' into beltran.bulbarella/RUM-11247_2 Update schema fix stack trace check remove guard in errorStack, generate schemas Revert "remove guard in errorStack, generate schemas" This reverts commit 51cfd95. generate schemas Merge branch 'main' into beltran.bulbarella/RUM-11247_2 fix typecheck fix unit Moved error stacks to the test folder, fixed firefox error stack fix undefined guard Merge branch 'main' into beltran.bulbarella/RUM-11247_2 rollback unrelated config changes remove comment remove unneded comment Fix comment fix one liner Improve frameLines function Co-authored-by: rgaignault <roman.gaignault@datadoghq.com>
1 parent b335a77 commit 3415c80

File tree

19 files changed

+152
-121
lines changed

19 files changed

+152
-121
lines changed

‎packages/core/src/domain/allowedTrackingOrigins.spec.ts‎

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { STACK_WITH_INIT_IN_EXTENSION, STACK_WITH_INIT_IN_PAGE } from '../../test'
12
import { display } from '../tools/display'
23
import {
34
isAllowedTrackingOrigins,
@@ -21,7 +22,7 @@ describe('checkForAllowedTrackingOrigins', () => {
2122
})
2223

2324
it('should not warn if not in extension environment', () => {
24-
const result = isAllowedTrackingOrigins(DEFAULT_CONFIG, 'https://app.example.com')
25+
const result = isAllowedTrackingOrigins(DEFAULT_CONFIG, STACK_WITH_INIT_IN_PAGE, 'https://app.example.com')
2526
expect(displayWarnSpy).not.toHaveBeenCalled()
2627
expect(displayErrorSpy).not.toHaveBeenCalled()
2728
expect(result).toBe(true)
@@ -34,6 +35,7 @@ describe('checkForAllowedTrackingOrigins', () => {
3435
...DEFAULT_CONFIG,
3536
allowedTrackingOrigins: ['https://app.example.com'],
3637
},
38+
STACK_WITH_INIT_IN_PAGE,
3739
'https://app.example.com'
3840
)
3941
expect(displayWarnSpy).not.toHaveBeenCalled()
@@ -47,6 +49,7 @@ describe('checkForAllowedTrackingOrigins', () => {
4749
...DEFAULT_CONFIG,
4850
allowedTrackingOrigins: [/^https:\/\/.*\.example\.com$/],
4951
},
52+
STACK_WITH_INIT_IN_PAGE,
5053
'https://app.example.com'
5154
)
5255
expect(displayWarnSpy).not.toHaveBeenCalled()
@@ -60,6 +63,7 @@ describe('checkForAllowedTrackingOrigins', () => {
6063
...DEFAULT_CONFIG,
6164
allowedTrackingOrigins: [(origin: string) => origin.includes('example.com')],
6265
},
66+
STACK_WITH_INIT_IN_PAGE,
6367
'https://app.example.com'
6468
)
6569
expect(displayWarnSpy).not.toHaveBeenCalled()
@@ -77,6 +81,7 @@ describe('checkForAllowedTrackingOrigins', () => {
7781
(origin: string) => origin.startsWith('https://app.'),
7882
],
7983
},
84+
STACK_WITH_INIT_IN_PAGE,
8085
'https://app.example.com'
8186
)
8287
expect(displayWarnSpy).not.toHaveBeenCalled()
@@ -92,8 +97,8 @@ describe('checkForAllowedTrackingOrigins', () => {
9297
...DEFAULT_CONFIG,
9398
allowedTrackingOrigins: ['https://different.com'],
9499
},
95-
'https://example.com',
96-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
100+
STACK_WITH_INIT_IN_EXTENSION,
101+
'https://example.com'
97102
)
98103
expect(displayErrorSpy).toHaveBeenCalledWith(ERROR_NOT_ALLOWED_TRACKING_ORIGIN)
99104
expect(result).toBe(false)
@@ -105,8 +110,8 @@ describe('checkForAllowedTrackingOrigins', () => {
105110
...DEFAULT_CONFIG,
106111
allowedTrackingOrigins: [/^https:\/\/specific-[a-z]+\.com$/],
107112
},
108-
'https://example.com',
109-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
113+
STACK_WITH_INIT_IN_EXTENSION,
114+
'https://example.com'
110115
)
111116
expect(displayErrorSpy).toHaveBeenCalledWith(ERROR_NOT_ALLOWED_TRACKING_ORIGIN)
112117
expect(result).toBe(false)
@@ -118,8 +123,8 @@ describe('checkForAllowedTrackingOrigins', () => {
118123
...DEFAULT_CONFIG,
119124
allowedTrackingOrigins: [(origin: string) => origin.includes('specific-id')],
120125
},
121-
'https://example.com',
122-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
126+
STACK_WITH_INIT_IN_EXTENSION,
127+
'https://example.com'
123128
)
124129
expect(displayErrorSpy).toHaveBeenCalledWith(ERROR_NOT_ALLOWED_TRACKING_ORIGIN)
125130
expect(result).toBe(false)
@@ -135,8 +140,8 @@ describe('checkForAllowedTrackingOrigins', () => {
135140
(origin: string) => origin.includes('specific-id'),
136141
],
137142
},
138-
'https://example.com',
139-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
143+
STACK_WITH_INIT_IN_EXTENSION,
144+
'https://example.com'
140145
)
141146
expect(displayErrorSpy).toHaveBeenCalledWith(ERROR_NOT_ALLOWED_TRACKING_ORIGIN)
142147
expect(result).toBe(false)
@@ -148,8 +153,8 @@ describe('checkForAllowedTrackingOrigins', () => {
148153
...DEFAULT_CONFIG,
149154
allowedTrackingOrigins: ['https://example.com'],
150155
},
151-
'https://example.com.extra.com',
152-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
156+
STACK_WITH_INIT_IN_EXTENSION,
157+
'https://example.com.extra.com'
153158
)
154159
expect(displayErrorSpy).toHaveBeenCalledWith(ERROR_NOT_ALLOWED_TRACKING_ORIGIN)
155160
expect(result).toBe(false)
@@ -161,8 +166,8 @@ describe('checkForAllowedTrackingOrigins', () => {
161166
...DEFAULT_CONFIG,
162167
allowedTrackingOrigins: [/^chrome-extension:\/\//],
163168
},
164-
'chrome-extension://abcdefghijklmno',
165-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
169+
STACK_WITH_INIT_IN_EXTENSION,
170+
'chrome-extension://abcdefghijklmno'
166171
)
167172
expect(displayErrorSpy).not.toHaveBeenCalled()
168173
expect(result).toBe(true)
@@ -176,8 +181,8 @@ describe('checkForAllowedTrackingOrigins', () => {
176181
...DEFAULT_CONFIG,
177182
allowedTrackingOrigins: undefined,
178183
},
179-
'https://example.com',
180-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
184+
STACK_WITH_INIT_IN_EXTENSION,
185+
'https://example.com'
181186
)
182187
expect(displayWarnSpy).toHaveBeenCalledWith(WARN_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN)
183188
expect(result).toBe(true)
@@ -189,8 +194,8 @@ describe('checkForAllowedTrackingOrigins', () => {
189194
...DEFAULT_CONFIG,
190195
allowedTrackingOrigins: [],
191196
},
192-
'https://example.com',
193-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
197+
STACK_WITH_INIT_IN_EXTENSION,
198+
'https://example.com'
194199
)
195200
expect(displayErrorSpy).toHaveBeenCalledWith(ERROR_NOT_ALLOWED_TRACKING_ORIGIN)
196201
expect(result).toBe(false)
@@ -202,8 +207,8 @@ describe('checkForAllowedTrackingOrigins', () => {
202207
...DEFAULT_CONFIG,
203208
allowedTrackingOrigins: undefined,
204209
},
205-
'https://example.com',
206-
'Error: at https://example.com/script.js:10:15'
210+
STACK_WITH_INIT_IN_PAGE,
211+
'https://example.com'
207212
)
208213
expect(displayWarnSpy).not.toHaveBeenCalled()
209214
expect(displayErrorSpy).not.toHaveBeenCalled()

‎packages/core/src/domain/allowedTrackingOrigins.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ export const ERROR_NOT_ALLOWED_TRACKING_ORIGIN = 'SDK initialized on a non-allow
1010

1111
export function isAllowedTrackingOrigins(
1212
configuration: InitConfiguration,
13-
windowOrigin = typeof location !== 'undefined' ? location.origin : '',
14-
errorStack = new Error().stack
13+
errorStack: string,
14+
windowOrigin = typeof location !== 'undefined' ? location.origin : ''
1515
): boolean {
1616
const allowedTrackingOrigins = configuration.allowedTrackingOrigins
1717
if (!allowedTrackingOrigins) {

‎packages/core/src/domain/configuration/configuration.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,10 @@ export function isSampleRate(sampleRate: unknown, name: string) {
358358
return true
359359
}
360360

361-
export function validateAndBuildConfiguration(initConfiguration: InitConfiguration): Configuration | undefined {
361+
export function validateAndBuildConfiguration(
362+
initConfiguration: InitConfiguration,
363+
errorStack?: string
364+
): Configuration | undefined {
362365
if (!initConfiguration || !initConfiguration.clientToken) {
363366
display.error('Client Token is not configured, we will not send any data.')
364367
return
@@ -381,7 +384,7 @@ export function validateAndBuildConfiguration(initConfiguration: InitConfigurati
381384
!isString(initConfiguration.version, 'Version') ||
382385
!isString(initConfiguration.env, 'Env') ||
383386
!isString(initConfiguration.service, 'Service') ||
384-
!isAllowedTrackingOrigins(initConfiguration)
387+
!isAllowedTrackingOrigins(initConfiguration, errorStack ?? '')
385388
) {
386389
return
387390
}

‎packages/core/src/domain/extension/extensionUtils.spec.ts‎

Lines changed: 42 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,78 +1,67 @@
1+
import {
2+
STACK_WITH_INIT_IN_EXTENSION,
3+
STACK_WITH_INIT_IN_EXTENSION_FIREFOX,
4+
STACK_WITH_INIT_IN_PAGE,
5+
} from '../../../test'
16
import {
27
containsExtensionUrl,
38
EXTENSION_PREFIXES,
49
extractExtensionUrlFromStack,
510
isUnsupportedExtensionEnvironment,
611
} from './extensionUtils'
712

8-
describe('containsExtensionUrl', () => {
9-
it('should return true if string contains an extension URL', () => {
10-
EXTENSION_PREFIXES.forEach((prefix) => {
11-
expect(containsExtensionUrl(`${prefix}some/path`)).toBe(true)
13+
describe('extensionUtils', () => {
14+
describe('containsExtensionUrl', () => {
15+
it('should return true if string contains an extension URL', () => {
16+
EXTENSION_PREFIXES.forEach((prefix) => {
17+
expect(containsExtensionUrl(`${prefix}some/path`)).toBe(true)
18+
})
1219
})
13-
})
14-
15-
it('should return false if string does not contain extension URL', () => {
16-
expect(containsExtensionUrl('https://example.com')).toBe(false)
17-
expect(containsExtensionUrl('')).toBe(false)
18-
})
19-
})
2020

21-
describe('isUnsupportedExtensionEnvironment', () => {
22-
it('should return true when window location is a regular URL and error stack contains extension URL', () => {
23-
expect(
24-
isUnsupportedExtensionEnvironment('https://example.com', 'Error: at chrome-extension://abcdefg/content.js:10:15')
25-
).toBe(true)
21+
it('should return false if string does not contain extension URL', () => {
22+
expect(containsExtensionUrl('https://example.com')).toBe(false)
23+
expect(containsExtensionUrl('')).toBe(false)
24+
})
2625
})
2726

28-
it('should return false when both window location and error stack are regular URLs', () => {
29-
expect(
30-
isUnsupportedExtensionEnvironment('https://example.com', 'Error: at https://example.com/script.js:10:15')
31-
).toBe(false)
32-
})
27+
describe('isUnsupportedExtensionEnvironment', () => {
28+
it('should return true when window location is a regular URL and error stack init is in an extension', () => {
29+
expect(isUnsupportedExtensionEnvironment('https://example.com', STACK_WITH_INIT_IN_EXTENSION)).toBe(true)
30+
})
3331

34-
it('should return false when window location is an extension URL', () => {
35-
EXTENSION_PREFIXES.forEach((prefix) => {
36-
expect(
37-
isUnsupportedExtensionEnvironment(`${prefix}some/path`, 'Error: at chrome-extension://abcdefg/content.js:10:15')
38-
).toBe(false)
32+
it('should return false when both window location and error stack init are regular URLs', () => {
33+
expect(isUnsupportedExtensionEnvironment('https://example.com', STACK_WITH_INIT_IN_PAGE)).toBe(false)
3934
})
40-
})
4135

42-
it('should return false when error stack is empty', () => {
43-
expect(isUnsupportedExtensionEnvironment('https://example.com', '')).toBe(false)
44-
})
36+
it('should return false when error stack is empty', () => {
37+
expect(isUnsupportedExtensionEnvironment('https://example.com', '')).toBe(false)
38+
})
4539

46-
it('should handle each extension prefix in error stack', () => {
47-
EXTENSION_PREFIXES.forEach((prefix) => {
48-
expect(
49-
isUnsupportedExtensionEnvironment('https://example.com', `Error: at ${prefix}abcdefg/content.js:10:15`)
50-
).toBe(true)
40+
it('should handle each extension prefix in firefox', () => {
41+
expect(isUnsupportedExtensionEnvironment('https://example.com', STACK_WITH_INIT_IN_EXTENSION_FIREFOX)).toBe(true)
5142
})
52-
})
5343

54-
it('should handle case when stack trace is undefined', () => {
55-
expect(isUnsupportedExtensionEnvironment('https://example.com')).toBe(false)
56-
})
44+
it('should handle case when stack trace is undefined', () => {
45+
expect(isUnsupportedExtensionEnvironment('https://example.com')).toBe(false)
46+
})
5747

58-
it('should handle extension stack trace', () => {
59-
expect(
60-
isUnsupportedExtensionEnvironment('https://example.com', 'Error: at chrome-extension://abcdefg/content.js:10:15')
61-
).toBe(true)
48+
it('should handle extension stack trace', () => {
49+
expect(isUnsupportedExtensionEnvironment('https://example.com', STACK_WITH_INIT_IN_EXTENSION)).toBe(true)
50+
})
6251
})
63-
})
6452

65-
describe('extractExtensionUrlFromStack', () => {
66-
it('should extract extension URL from stack trace', () => {
67-
const stack = `Error
53+
describe('extract init caller', () => {
54+
it('should extract extension URL from stack trace', () => {
55+
const stack = `Error
6856
at foo (<anonymous>:549:44)
6957
at bar (<anonymous>:701:91)
70-
at e.init (chrome-extension://boceobohkgenpcpogecpjlnmnfbdigda/content-script-main.js:1:1009)`
71-
expect(extractExtensionUrlFromStack(stack)).toBe('chrome-extension://boceobohkgenpcpogecpjlnmnfbdigda')
72-
})
58+
at e.init (chrome-extension://abcd/content-script-main.js:1:1009)`
59+
expect(extractExtensionUrlFromStack(stack)).toBe('chrome-extension://abcd')
60+
})
7361

74-
it('should return undefined when no extension URL found', () => {
75-
const stack = 'Error at https://example.com/script.js:10:15'
76-
expect(extractExtensionUrlFromStack(stack)).toBeUndefined()
62+
it('should return undefined when no extension URL found', () => {
63+
const stack = 'Error at https://example.com/script.js:10:15'
64+
expect(extractExtensionUrlFromStack(stack)).toBeUndefined()
65+
})
7766
})
7867
})

‎packages/core/src/domain/extension/extensionUtils.ts‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,25 @@ export function containsExtensionUrl(str: string): boolean {
1212
* @returns true if running in an unsupported browser extension environment
1313
*/
1414
export function isUnsupportedExtensionEnvironment(windowLocation: string, stack: string = '') {
15-
// If we're on a regular web page but the error stack shows extension URLs,
16-
// then an extension is injecting RUM.
17-
return !containsExtensionUrl(windowLocation) && containsExtensionUrl(stack)
15+
// If the page itself is an extension page.
16+
if (containsExtensionUrl(windowLocation)) {
17+
return false
18+
}
19+
20+
// Since we generate the error on the init, we check the 2nd frame line.
21+
const frameLines = stack.split('\n').filter((line) => {
22+
const trimmedLine = line.trim()
23+
return trimmedLine.length && /^at\s+|@/.test(trimmedLine)
24+
})
25+
const target = frameLines[1] || ''
26+
27+
return containsExtensionUrl(target)
1828
}
1929

2030
export function extractExtensionUrlFromStack(stack: string = ''): string | undefined {
2131
for (const prefix of EXTENSION_PREFIXES) {
2232
const match = stack.match(new RegExp(`${prefix}[^/]+`))
33+
2334
if (match) {
2435
return match[0]
2536
}

‎packages/core/src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,3 +164,4 @@ export * from './domain/deflate'
164164
export * from './domain/connectivity'
165165
export * from './tools/stackTrace/handlingStack'
166166
export * from './tools/abstractHooks'
167+
export { WARN_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN } from './domain/allowedTrackingOrigins'

‎packages/core/test/browserExtension.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Base case, the page has the SDK in the init and the error stack is in the page.
2+
export const STACK_WITH_INIT_IN_PAGE = `Error
3+
at Object.init (http://localhost:8080/datadog-rum.js:3919:16)
4+
at http://localhost:8080/:10:14`
5+
6+
// Base case for extension, the extension has the SDK in the init and the error stack is in the extension.
7+
export const STACK_WITH_INIT_IN_EXTENSION = `Error
8+
at Object.init (chrome-extension://abcdef/dist/contentScript.js:254:14)
9+
at chrome-extension://abcdef/dist/contentScript.js:13304:14
10+
at chrome-extension://abcdef/dist/contentScript.js:13315:3`
11+
12+
export const STACK_WITH_INIT_IN_EXTENSION_FIREFOX = `Error
13+
Object.init@moz-extension://abcdef/dist/contentScript.js:254:14
14+
@moz-extension://abcdef/dist/contentScript.js:13304:14
15+
@moz-extension://abcdef/dist/contentScript.js:13315:3`

‎packages/core/test/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
export * from './browserChecks'
2+
export * from './browserExtension'
23
export type * from './buildEnv'
34
export * from './collectAsyncCalls'
45
export * from './cookie'

‎packages/logs/src/boot/logsPublicApi.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
createTrackingConsentState,
1212
defineContextMethod,
1313
startBufferingData,
14+
callMonitored,
1415
} from '@datadog/browser-core'
1516
import type { LogsInitConfiguration } from '../domain/configuration'
1617
import type { HandlerType } from '../domain/logger'
@@ -255,7 +256,7 @@ export interface LogsPublicApi extends PublicApi {
255256
}
256257

257258
export interface Strategy {
258-
init: (initConfiguration: LogsInitConfiguration) => void
259+
init: (initConfiguration: LogsInitConfiguration, errorStack?: string) => void
259260
initConfiguration: LogsInitConfiguration | undefined
260261
globalContext: ContextManager
261262
accountContext: ContextManager
@@ -293,7 +294,10 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs): LogsPublicApi {
293294
return makePublicApi<LogsPublicApi>({
294295
logger: mainLogger,
295296

296-
init: monitor((initConfiguration) => strategy.init(initConfiguration)),
297+
init: (initConfiguration) => {
298+
const errorStack = new Error().stack
299+
callMonitored(() => strategy.init(initConfiguration, errorStack))
300+
},
297301

298302
setTrackingConsent: monitor((trackingConsent) => {
299303
trackingConsentState.update(trackingConsent)

0 commit comments

Comments
 (0)