Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ [RUMF-1175] collect reports and csp violation #1332

Merged
merged 26 commits into from
Mar 4, 2022
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
2f3b989
✨ Create report observable
amortemousque Feb 21, 2022
b391c17
Add reports in Log browser SDK
amortemousque Feb 21, 2022
715c46b
Add report in RUM browser SDK
amortemousque Feb 21, 2022
fcd4950
Add new forwardReports config option
amortemousque Feb 25, 2022
f74020d
Move report api stubs in its own file
amortemousque Mar 1, 2022
37180c2
Rum track report under FF
amortemousque Mar 1, 2022
43ef98b
Fix configuration validation
amortemousque Mar 1, 2022
875cdc3
Add tests
amortemousque Mar 1, 2022
e28db68
👌 Update browser support
amortemousque Mar 1, 2022
2cdc941
👌 use toStackTraceString
amortemousque Mar 2, 2022
998a897
Fix report init option allowedValues check
amortemousque Mar 2, 2022
e248579
Send report id instead of type + add more info for non error report
amortemousque Mar 2, 2022
1049d11
👌 Simplify report types
amortemousque Mar 2, 2022
5555282
👌 Add report test when init option is not specified
amortemousque Mar 2, 2022
c74dfd2
👌 Rename CustomReport to RawReport
amortemousque Mar 2, 2022
5f2f273
Merge branch 'main' into aymeric/collect-reports-and-csp-violation
amortemousque Mar 2, 2022
7c9a4f9
Update browser support
amortemousque Mar 2, 2022
3b5571d
👌 Report should be UNHANDLED
amortemousque Mar 2, 2022
9b236d8
👌 Add test on getFileFromStackTraceString
amortemousque Mar 2, 2022
a6f592f
Report id becomes subtype
amortemousque Mar 2, 2022
885f8e2
👌 Add policy to the stack message
amortemousque Mar 3, 2022
ac86464
Merge branch 'main' into aymeric/collect-reports-and-csp-violation
amortemousque Mar 3, 2022
fe11c96
Fix eslint issue + update format
amortemousque Mar 3, 2022
18d6978
Merge branch 'main' into aymeric/collect-reports-and-csp-violation
amortemousque Mar 3, 2022
2079e55
Fix ie compatibility
amortemousque Mar 4, 2022
7124d93
Merge branch 'main' into aymeric/collect-reports-and-csp-violation
amortemousque Mar 4, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions packages/core/src/domain/report/browser.types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
export interface BrowserWindow {
ReportingObserver?: ReportingObserverConstructor
}

export interface ReportingObserver {
disconnect(): void
observe(): void
takeRecords(): Report[]
}

export interface ReportingObserverConstructor {
new (callback: ReportingObserverCallback, option: ReportingObserverOption): ReportingObserver
}

export type ReportType = 'intervention' | 'deprecation'

export interface Report {
type: ReportType
url: string
body: DeprecationReportBody | InterventionReportBody
}

export interface ReportingObserverCallback {
(reports: Report[], observer: ReportingObserver): void
}

export interface ReportingObserverOption {
types: ReportType[]
buffered: boolean
}

interface DeprecationReportBody {
id: string
message: string
lineNumber: number
columnNumber: number
sourceFile: string
anticipatedRemoval?: Date
}

interface InterventionReportBody {
id: string
message: string
lineNumber: number
columnNumber: number
sourceFile: string
}
60 changes: 60 additions & 0 deletions packages/core/src/domain/report/reportObservable.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import type { Subscription } from '../../tools/observable'
import { stubReportingObserver, stubCspEventListener } from '../../../test/stubReportApis'
import { initReportObservable, RawReportType } from './reportObservable'

describe('report observable', () => {
let reportingObserverStub: { reset(): void; raiseReport(type: string): void }
let cspEventListenerStub: { dispatchEvent(): void }
let consoleSubscription: Subscription
let notifyReport: jasmine.Spy

beforeEach(() => {
reportingObserverStub = stubReportingObserver()
cspEventListenerStub = stubCspEventListener()
notifyReport = jasmine.createSpy('notifyReport')
})

afterEach(() => {
reportingObserverStub.reset()
consoleSubscription.unsubscribe()
})
;[RawReportType.deprecation, RawReportType.intervention].forEach((type) => {
it(`should notify ${type} reports`, () => {
consoleSubscription = initReportObservable([type]).subscribe(notifyReport)
reportingObserverStub.raiseReport(type)

const [report] = notifyReport.calls.mostRecent().args

expect(report).toEqual(
jasmine.objectContaining({
message: `${type}: foo bar`,
subtype: 'NavigatorVibrate',
type,
})
)
})
})

it(`should compute stack for ${RawReportType.intervention}`, () => {
consoleSubscription = initReportObservable([RawReportType.intervention]).subscribe(notifyReport)
reportingObserverStub.raiseReport(RawReportType.intervention)

const [report] = notifyReport.calls.mostRecent().args

expect(report.stack).toEqual(`NavigatorVibrate: foo bar
at <anonymous> @ http://foo.bar/index.js:20:10`)
})

it(`should notify ${RawReportType.cspViolation}`, () => {
consoleSubscription = initReportObservable([RawReportType.cspViolation]).subscribe(notifyReport)
cspEventListenerStub.dispatchEvent()

expect(notifyReport).toHaveBeenCalledOnceWith({
message: `csp_violation: 'blob' blocked by 'worker-src' directive`,
type: 'csp_violation',
subtype: 'worker-src',
stack: `worker-src: 'blob' blocked by 'worker-src' directive of the policy "worker-src 'none'"
at <anonymous> @ http://foo.bar/index.js:17:8`,
})
})
})
124 changes: 124 additions & 0 deletions packages/core/src/domain/report/reportObservable.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
import { toStackTraceString } from '../../tools/error'
import { mergeObservables, Observable } from '../../tools/observable'
import { DOM_EVENT, includes, addEventListener, safeTruncate } from '../../tools/utils'
import { monitor } from '../internalMonitoring'
import type { Report, BrowserWindow, ReportType } from './browser.types'

export const RawReportType = {
intervention: 'intervention',
deprecation: 'deprecation',
cspViolation: 'csp_violation',
} as const

export type RawReportType = typeof RawReportType[keyof typeof RawReportType]

export interface RawReport {
type: RawReportType
subtype: string
message: string
stack?: string
}

export function initReportObservable(apis: RawReportType[]) {
const observables: Array<Observable<RawReport>> = []

if (includes(apis, RawReportType.cspViolation)) {
observables.push(createCspViolationReportObservable())
}

const reportTypes = apis.filter((api: RawReportType): api is ReportType => api !== RawReportType.cspViolation)
if (reportTypes.length) {
observables.push(createReportObservable(reportTypes))
}

return mergeObservables<RawReport>(...observables)
}

function createReportObservable(reportTypes: ReportType[]) {
const observable = new Observable<RawReport>(() => {
if (!(window as BrowserWindow).ReportingObserver) {
return
}

const handleReports = monitor((reports: Report[]) =>
reports.forEach((report) => {
observable.notify(buildRawReportFromReport(report))
})
)

const observer = new (window as BrowserWindow).ReportingObserver!(handleReports, {
types: reportTypes,
buffered: true,
})

observer.observe()
return () => {
observer.disconnect()
}
})

return observable
}

function createCspViolationReportObservable() {
const observable = new Observable<RawReport>(() => {
const handleCspViolation = monitor((event: SecurityPolicyViolationEvent) => {
observable.notify(buildRawReportFromCspViolation(event))
})

const { stop } = addEventListener(document, DOM_EVENT.SECURITY_POLICY_VIOLATION, handleCspViolation)

return stop
})
return observable
}

function buildRawReportFromReport({ type, body }: Report): RawReport {
return {
type,
subtype: body.id,
message: `${type}: ${body.message}`,
stack: buildStack(body.id, body.message, body.sourceFile, body.lineNumber, body.columnNumber),
}
}

function buildRawReportFromCspViolation(event: SecurityPolicyViolationEvent): RawReport {
const type = RawReportType.cspViolation
const message = `'${event.blockedURI}' blocked by '${event.effectiveDirective}' directive`
Copy link
Contributor Author

@amortemousque amortemousque Mar 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some thought, I think it is valuable to add the originalPolicy to the crafted message.
Could be done like so:
${event.blockedURI}' blocked by '${event.effectiveDirective}' directive of the policy "${event.originalPolicy}"

This way we will have message like:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some directives could list a large number of domains 🤔
I'd say that it could be added in the stack to keep a minimal message.
We should maybe also truncate the policy to avoid sending too much data.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it to the stack. Let me know what you think

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

return {
type: RawReportType.cspViolation,
subtype: event.effectiveDirective,
message: `${type}: ${message}`,
stack: buildStack(
event.effectiveDirective,
`${message} of the policy "${safeTruncate(event.originalPolicy, 100, '...')}"`,
event.sourceFile,
event.lineNumber,
event.columnNumber
),
}
}

function buildStack(
name: string,
message: string,
sourceFile: string | undefined,
lineNumber: number | undefined,
columnNumber: number | undefined
): string | undefined {
return (
sourceFile &&
toStackTraceString({
name,
message,
stack: [
{
func: '?',
url: sourceFile,
line: lineNumber,
column: columnNumber,
},
],
})
)
}
2 changes: 2 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export {
export { trackRuntimeError } from './domain/error/trackRuntimeError'
export { computeStackTrace, StackTrace } from './domain/tracekit'
export { defineGlobal, makePublicApi } from './boot/init'
export { initReportObservable, RawReport, RawReportType } from './domain/report/reportObservable'
export {
startInternalMonitoring,
InternalMonitoring,
Expand Down Expand Up @@ -50,6 +51,7 @@ export {
createHandlingStack,
RawError,
toStackTraceString,
getFileFromStackTraceString,
} from './tools/error'
export { Context, ContextArray, ContextValue } from './tools/context'
export { areCookiesAuthorized, getCookie, setCookie, deleteCookie, COOKIE_ACCESS_DELAY } from './browser/cookie'
Expand Down
17 changes: 16 additions & 1 deletion packages/core/src/tools/error.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { StackTrace } from '../domain/tracekit'
import { createHandlingStack, formatUnknownError } from './error'
import { createHandlingStack, formatUnknownError, getFileFromStackTraceString } from './error'

describe('formatUnknownError', () => {
const NOT_COMPUTED_STACK_TRACE: StackTrace = { name: undefined, message: undefined, stack: [] } as any
Expand Down Expand Up @@ -72,6 +72,21 @@ describe('formatUnknownError', () => {
})
})

describe('getFileFromStackTraceString', () => {
it('should get the first source file of the stack', () => {
amortemousque marked this conversation as resolved.
Show resolved Hide resolved
expect(
getFileFromStackTraceString(`TypeError: oh snap!
at foo(1, bar) @ http://path/to/file.js:52:15
at <anonymous> @ http://path/to/file.js:12
at <anonymous>(baz) @ http://path/to/file.js`)
).toEqual('http://path/to/file.js:52:15')
})

it('should get undefined if no source file is in the stack', () => {
expect(getFileFromStackTraceString(`TypeError: oh snap!`)).not.toBeDefined()
})
})

describe('createHandlingStack', () => {
let handlingStack: string
function internalCall() {
Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/tools/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const ErrorSource = {
LOGGER: 'logger',
NETWORK: 'network',
SOURCE: 'source',
REPORT: 'report',
} as const

export enum ErrorHandling {
Expand Down Expand Up @@ -72,6 +73,10 @@ export function toStackTraceString(stack: StackTrace) {
return result
}

export function getFileFromStackTraceString(stack: string) {
return /@ (.+)/.exec(stack)?.[1]
}

export function formatErrorMessage(stack: StackTrace) {
return `${stack.name || 'Error'}: ${stack.message!}`
}
Expand Down
10 changes: 10 additions & 0 deletions packages/core/src/tools/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,16 @@ describe('utils', () => {
expect(truncated.length).toBe(7)
expect(truncated).toBe('12345😎')
})

it('should add the suffix when the string is truncated', () => {
const truncated = safeTruncate('12345😎890', 6, '...')
expect(truncated).toBe('12345😎...')
})

it('should not add the suffix when the string is not truncated', () => {
const truncated = safeTruncate('1234😎', 5, '...')
expect(truncated).toBe('1234😎')
})
})

it('should perform a draw', () => {
Expand Down
14 changes: 8 additions & 6 deletions packages/core/src/tools/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export const enum DOM_EVENT {
INPUT = 'input',
PLAY = 'play',
PAUSE = 'pause',
SECURITY_POLICY_VIOLATION = 'securitypolicyviolation',
}

export enum ResourceType {
Expand Down Expand Up @@ -318,13 +319,14 @@ export function findCommaSeparatedValue(rawString: string, name: string) {
return matches ? matches[1] : undefined
}

export function safeTruncate(candidate: string, length: number) {
export function safeTruncate(candidate: string, length: number, suffix = '') {
const lastChar = candidate.charCodeAt(length - 1)
// check if it is the high part of a surrogate pair
if (lastChar >= 0xd800 && lastChar <= 0xdbff) {
return candidate.slice(0, length + 1)
}
return candidate.slice(0, length)
const isLastCharSurrogatePair = lastChar >= 0xd800 && lastChar <= 0xdbff
const correctedLength = isLastCharSurrogatePair ? length + 1 : length

if (candidate.length <= correctedLength) return candidate

return `${candidate.slice(0, correctedLength)}${suffix}`
}

export interface EventEmitter {
Expand Down
Loading