Skip to content

Commit da378f2

Browse files
BeltranBulbarellaDDBeltranBulbarellaDD
committed
Moved error stack to the init function
1 parent dc19986 commit da378f2

File tree

17 files changed

+117
-61
lines changed

17 files changed

+117
-61
lines changed

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

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ describe('checkForAllowedTrackingOrigins', () => {
2121
})
2222

2323
it('should not warn if not in extension environment', () => {
24-
const result = isAllowedTrackingOrigins(DEFAULT_CONFIG, 'https://app.example.com')
24+
const result = isAllowedTrackingOrigins(
25+
DEFAULT_CONFIG,
26+
'Error: at https://example.com/script.js:1:1',
27+
'https://app.example.com'
28+
)
2529
expect(displayWarnSpy).not.toHaveBeenCalled()
2630
expect(displayErrorSpy).not.toHaveBeenCalled()
2731
expect(result).toBe(true)
@@ -34,6 +38,7 @@ describe('checkForAllowedTrackingOrigins', () => {
3438
...DEFAULT_CONFIG,
3539
allowedTrackingOrigins: ['https://app.example.com'],
3640
},
41+
'Error: at https://example.com/script.js:1:1',
3742
'https://app.example.com'
3843
)
3944
expect(displayWarnSpy).not.toHaveBeenCalled()
@@ -47,6 +52,7 @@ describe('checkForAllowedTrackingOrigins', () => {
4752
...DEFAULT_CONFIG,
4853
allowedTrackingOrigins: [/^https:\/\/.*\.example\.com$/],
4954
},
55+
'Error: at https://example.com/script.js:1:1',
5056
'https://app.example.com'
5157
)
5258
expect(displayWarnSpy).not.toHaveBeenCalled()
@@ -60,6 +66,7 @@ describe('checkForAllowedTrackingOrigins', () => {
6066
...DEFAULT_CONFIG,
6167
allowedTrackingOrigins: [(origin: string) => origin.includes('example.com')],
6268
},
69+
'Error: at https://example.com/script.js:1:1',
6370
'https://app.example.com'
6471
)
6572
expect(displayWarnSpy).not.toHaveBeenCalled()
@@ -77,6 +84,7 @@ describe('checkForAllowedTrackingOrigins', () => {
7784
(origin: string) => origin.startsWith('https://app.'),
7885
],
7986
},
87+
'Error: at https://example.com/script.js:1:1',
8088
'https://app.example.com'
8189
)
8290
expect(displayWarnSpy).not.toHaveBeenCalled()
@@ -92,8 +100,8 @@ describe('checkForAllowedTrackingOrigins', () => {
92100
...DEFAULT_CONFIG,
93101
allowedTrackingOrigins: ['https://different.com'],
94102
},
103+
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15',
95104
'https://example.com',
96-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
97105
)
98106
expect(displayErrorSpy).toHaveBeenCalledWith(ERROR_NOT_ALLOWED_TRACKING_ORIGIN)
99107
expect(result).toBe(false)
@@ -105,8 +113,8 @@ describe('checkForAllowedTrackingOrigins', () => {
105113
...DEFAULT_CONFIG,
106114
allowedTrackingOrigins: [/^https:\/\/specific-[a-z]+\.com$/],
107115
},
108-
'https://example.com',
109-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
116+
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15',
117+
'https://example.com'
110118
)
111119
expect(displayErrorSpy).toHaveBeenCalledWith(ERROR_NOT_ALLOWED_TRACKING_ORIGIN)
112120
expect(result).toBe(false)
@@ -118,8 +126,8 @@ describe('checkForAllowedTrackingOrigins', () => {
118126
...DEFAULT_CONFIG,
119127
allowedTrackingOrigins: [(origin: string) => origin.includes('specific-id')],
120128
},
121-
'https://example.com',
122-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
129+
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15',
130+
'https://example.com'
123131
)
124132
expect(displayErrorSpy).toHaveBeenCalledWith(ERROR_NOT_ALLOWED_TRACKING_ORIGIN)
125133
expect(result).toBe(false)
@@ -135,8 +143,8 @@ describe('checkForAllowedTrackingOrigins', () => {
135143
(origin: string) => origin.includes('specific-id'),
136144
],
137145
},
138-
'https://example.com',
139-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
146+
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15',
147+
'https://example.com'
140148
)
141149
expect(displayErrorSpy).toHaveBeenCalledWith(ERROR_NOT_ALLOWED_TRACKING_ORIGIN)
142150
expect(result).toBe(false)
@@ -148,8 +156,8 @@ describe('checkForAllowedTrackingOrigins', () => {
148156
...DEFAULT_CONFIG,
149157
allowedTrackingOrigins: ['https://example.com'],
150158
},
151-
'https://example.com.extra.com',
152-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
159+
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15',
160+
'https://example.com.extra.com'
153161
)
154162
expect(displayErrorSpy).toHaveBeenCalledWith(ERROR_NOT_ALLOWED_TRACKING_ORIGIN)
155163
expect(result).toBe(false)
@@ -161,8 +169,8 @@ describe('checkForAllowedTrackingOrigins', () => {
161169
...DEFAULT_CONFIG,
162170
allowedTrackingOrigins: [/^chrome-extension:\/\//],
163171
},
164-
'chrome-extension://abcdefghijklmno',
165-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
172+
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15',
173+
'chrome-extension://abcdefghijklmno'
166174
)
167175
expect(displayErrorSpy).not.toHaveBeenCalled()
168176
expect(result).toBe(true)
@@ -176,8 +184,8 @@ describe('checkForAllowedTrackingOrigins', () => {
176184
...DEFAULT_CONFIG,
177185
allowedTrackingOrigins: undefined,
178186
},
179-
'https://example.com',
180-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
187+
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15',
188+
'https://example.com'
181189
)
182190
expect(displayWarnSpy).toHaveBeenCalledWith(WARN_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN)
183191
expect(result).toBe(true)
@@ -189,8 +197,8 @@ describe('checkForAllowedTrackingOrigins', () => {
189197
...DEFAULT_CONFIG,
190198
allowedTrackingOrigins: [],
191199
},
192-
'https://example.com',
193-
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15'
200+
'Error: at chrome-extension://abcdefghijklmno/content.js:10:15',
201+
'https://example.com'
194202
)
195203
expect(displayErrorSpy).toHaveBeenCalledWith(ERROR_NOT_ALLOWED_TRACKING_ORIGIN)
196204
expect(result).toBe(false)
@@ -202,8 +210,8 @@ describe('checkForAllowedTrackingOrigins', () => {
202210
...DEFAULT_CONFIG,
203211
allowedTrackingOrigins: undefined,
204212
},
205-
'https://example.com',
206-
'Error: at https://example.com/script.js:10:15'
213+
'Error: at https://example.com/script.js:10:15',
214+
'https://example.com'
207215
)
208216
expect(displayWarnSpy).not.toHaveBeenCalled()
209217
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.spec.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
isExperimentalFeatureEnabled,
88
resetExperimentalFeatures,
99
} from '../../tools/experimentalFeatures'
10+
import { WARN_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN } from '../allowedTrackingOrigins'
1011
import { SessionPersistence } from '../session/sessionConstants'
1112
import { TrackingConsent } from '../trackingConsent'
1213
import type { InitConfiguration } from './configuration'
@@ -233,4 +234,17 @@ describe('validateAndBuildConfiguration', () => {
233234
expect(serializedConfiguration).toEqual(SERIALIZED_EXHAUSTIVE_INIT_CONFIGURATION)
234235
})
235236
})
237+
238+
describe('validateAndBuildConfiguration errorStack threading', () => {
239+
it('uses provided errorStack in isAllowedTrackingOrigins (triggers extension warning)', () => {
240+
const warnSpy = spyOn(display, 'warn')
241+
242+
const initConfiguration = { clientToken: 't' } as any
243+
const errorStack = 'Error: at chrome-extension://abcdefgh/content.js:10:15'
244+
245+
validateAndBuildConfiguration(initConfiguration, errorStack)
246+
247+
expect(warnSpy).toHaveBeenCalledWith(WARN_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN)
248+
})
249+
})
236250
})

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ export function isSampleRate(sampleRate: unknown, name: string) {
314314
return true
315315
}
316316

317-
export function validateAndBuildConfiguration(initConfiguration: InitConfiguration): Configuration | undefined {
317+
export function validateAndBuildConfiguration(initConfiguration: InitConfiguration, errorStack?: string): Configuration | undefined {
318318
if (!initConfiguration || !initConfiguration.clientToken) {
319319
display.error('Client Token is not configured, we will not send any data.')
320320
return
@@ -337,7 +337,7 @@ export function validateAndBuildConfiguration(initConfiguration: InitConfigurati
337337
!isString(initConfiguration.version, 'Version') ||
338338
!isString(initConfiguration.env, 'Env') ||
339339
!isString(initConfiguration.service, 'Service') ||
340-
!isAllowedTrackingOrigins(initConfiguration)
340+
!isAllowedTrackingOrigins(initConfiguration, errorStack!, undefined)
341341
) {
342342
return
343343
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export function containsExtensionUrl(str: string): boolean {
66

77
/**
88
* Utility function to detect if the SDK is being initialized in an unsupported browser extension environment.
9+
* Note: Because we check error stack, this will not work if the error stack is too long as Browsers truncate it.
910
*
1011
* @param windowLocation - The current window location to check
1112
* @param stack - The error stack to check for extension URLs

‎packages/core/src/index.ts‎

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

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ export interface LogsPublicApi extends PublicApi {
255255
}
256256

257257
export interface Strategy {
258-
init: (initConfiguration: LogsInitConfiguration) => void
258+
init: (initConfiguration: LogsInitConfiguration, errorStack?: string) => void
259259
initConfiguration: LogsInitConfiguration | undefined
260260
globalContext: ContextManager
261261
accountContext: ContextManager
@@ -293,7 +293,10 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs): LogsPublicApi {
293293
return makePublicApi<LogsPublicApi>({
294294
logger: mainLogger,
295295

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

298301
setTrackingConsent: monitor((trackingConsent) => {
299302
trackingConsentState.update(trackingConsent)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export function createPreStartStrategy(
5454
}
5555

5656
return {
57-
init(initConfiguration) {
57+
init(initConfiguration, errorStack) {
5858
if (!initConfiguration) {
5959
display.error('Missing configuration')
6060
return
@@ -75,7 +75,7 @@ export function createPreStartStrategy(
7575
return
7676
}
7777

78-
const configuration = validateAndBuildLogsConfiguration(initConfiguration)
78+
const configuration = validateAndBuildLogsConfiguration(initConfiguration, errorStack)
7979
if (!configuration) {
8080
return
8181
}

‎packages/logs/src/domain/configuration.spec.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { InitConfiguration } from '@datadog/browser-core'
2-
import { display } from '@datadog/browser-core'
2+
import { display, WARN_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN } from '@datadog/browser-core'
33
import {
44
EXHAUSTIVE_INIT_CONFIGURATION,
55
type CamelToSnakeCase,
@@ -94,6 +94,19 @@ describe('validateAndBuildLogsConfiguration', () => {
9494
})
9595
})
9696

97+
describe('validateAndBuildLogsConfiguration errorStack threading', () => {
98+
it('uses provided errorStack to detect extension context and warn', () => {
99+
const warnSpy = spyOn(display, 'warn')
100+
101+
const initConfiguration = { clientToken: 't' } as any
102+
const errorStack = 'Error: at chrome-extension://abcdefgh/content.js:10:15'
103+
104+
validateAndBuildLogsConfiguration(initConfiguration, errorStack)
105+
106+
expect(warnSpy).toHaveBeenCalledWith(WARN_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN)
107+
})
108+
})
109+
97110
describe('validateAndBuildForwardOption', () => {
98111
let displaySpy: jasmine.Spy<typeof display.error>
99112
const allowedValues = ['foo', 'bar']

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,16 @@ export interface LogsConfiguration extends Configuration {
7272
export const DEFAULT_REQUEST_ERROR_RESPONSE_LENGTH_LIMIT = 32 * ONE_KIBI_BYTE
7373

7474
export function validateAndBuildLogsConfiguration(
75-
initConfiguration: LogsInitConfiguration
75+
initConfiguration: LogsInitConfiguration,
76+
errorStack?: string
7677
): LogsConfiguration | undefined {
7778
if (initConfiguration.usePciIntake === true && initConfiguration.site && initConfiguration.site !== 'datadoghq.com') {
7879
display.warn(
7980
'PCI compliance for Logs is only available for Datadog organizations in the US1 site. Default intake will be used.'
8081
)
8182
}
8283

83-
const baseConfiguration = validateAndBuildConfiguration(initConfiguration)
84+
const baseConfiguration = validateAndBuildConfiguration(initConfiguration, errorStack)
8485

8586
const forwardConsoleLogs = validateAndBuildForwardOption<ConsoleApiName>(
8687
initConfiguration.forwardConsoleLogs,

0 commit comments

Comments
 (0)