Skip to content

Commit d79235f

Browse files
xgouchetxgouchet
authored
Merge pull request #39 from DataDog/xgouchet/RUMM-1308/improve_errors
RUMM-1308/improve errors
2 parents 7436bcc + f2d371a commit d79235f

File tree

7 files changed

+196
-55
lines changed

7 files changed

+196
-55
lines changed

‎example/android/app/build.gradle‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ import com.android.build.OutputFile
7777

7878
project.ext.react = [
7979
enableHermes: false, // clean and rebuild if changing
80+
// the entry file for bundle generation
81+
entryFile: "index.tsx",
8082
]
8183

8284
apply from: "../../node_modules/react-native/react.gradle"

‎example/src/screens/AboutScreen.tsx‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React, {Component} from 'react';
2-
import { View, Text } from 'react-native';
2+
import { View, Text, TouchableNativeFeedback } from 'react-native';
33
import styles from './styles';
44
import {about} from '../resources/strings.json';
55

@@ -8,6 +8,15 @@ export default class AboutScreen extends Component<any, any> {
88
render(){
99
return <View style={styles.defaultScreen}>
1010
<Text>Result: {about} </Text>
11+
<TouchableNativeFeedback
12+
accessibilityLabel="click_me_about"
13+
onPress={() => {
14+
console.error("Not implemented", Error("Oups"));
15+
}}>
16+
<View style={styles.button}>
17+
<Text>Click me</Text>
18+
</View>
19+
</TouchableNativeFeedback>
1120
</View>
1221
}
1322
}

‎src/__tests__/rum/instrumentation/DdRumErrorTracking.test.tsx‎

Lines changed: 93 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,44 +29,57 @@ let baseErrorHandler = (error: any, isFatal?: boolean) => {
2929
};
3030
let originalErrorHandler = undefined;
3131

32+
let baseConsoleErrorCalled = false;
33+
let baseConsoleError = (...params: unknown) => {
34+
baseConsoleErrorCalled = true
35+
}
36+
let originalConsoleError = undefined
37+
3238
const flushPromises = () => new Promise(setImmediate);
3339

3440
beforeEach(() => {
3541
DdRum.addError.mockClear();
3642
baseErrorHandlerCalled = false;
3743
originalErrorHandler = ErrorUtils.getGlobalHandler();
3844
ErrorUtils.setGlobalHandler(baseErrorHandler);
45+
originalConsoleError = console.error;
46+
console.error = baseConsoleError;
3947
jest.setTimeout(20000)
4048
})
4149

4250
afterEach(() => {
4351
DdRumErrorTracking['isTracking'] = false
4452
ErrorUtils.setGlobalHandler(originalErrorHandler)
53+
console.error = originalConsoleError
4554
})
4655

4756

48-
it('M intercept and send a RUM event W onError() {empty stack trace}', async () => {
57+
it('M intercept and send a RUM event W onGlobalError() {empty stack trace}', async () => {
4958
// GIVEN
50-
DdRumErrorTracking.startTracking()
59+
DdRumErrorTracking.startTracking();
60+
const is_fatal = Math.random() < 0.5;
5161
const error = new Error('Something bad happened');
5262

5363
// WHEN
54-
DdRumErrorTracking.onError(error, false);
64+
DdRumErrorTracking.onGlobalError(error, is_fatal);
5565
await flushPromises();
5666

5767
// THEN
5868
expect(DdRum.addError.mock.calls.length).toBe(1);
5969
expect(DdRum.addError.mock.calls[0][0]).toBe(String(error));
6070
expect(DdRum.addError.mock.calls[0][1]).toBe("SOURCE");
6171
expect(DdRum.addError.mock.calls[0][2]).toBe("");
62-
expect(DdRum.addError.mock.calls[0][4]).toStrictEqual(error);
72+
const attributes = DdRum.addError.mock.calls[0][4];
73+
expect(attributes["_dd.error.raw"]).toStrictEqual(error);
74+
expect(attributes["_dd.error.is_crash"]).toStrictEqual(is_fatal);
6375
expect(baseErrorHandlerCalled).toStrictEqual(true);
6476
})
6577

6678

67-
it('M intercept and send a RUM event W onError() {with source file info}', async () => {
79+
it('M intercept and send a RUM event W onGlobalError() {with source file info}', async () => {
6880
// GIVEN
69-
DdRumErrorTracking.startTracking()
81+
DdRumErrorTracking.startTracking();
82+
const is_fatal = Math.random() < 0.5;
7083
const error = {
7184
sourceURL: "./path/to/file.js",
7285
line: 1038,
@@ -75,37 +88,105 @@ it('M intercept and send a RUM event W onError() {with source file info}', async
7588
};
7689

7790
// WHEN
78-
DdRumErrorTracking.onError(error, false);
91+
DdRumErrorTracking.onGlobalError(error, is_fatal);
7992
await flushPromises();
8093

8194
// THEN
8295
expect(DdRum.addError.mock.calls.length).toBe(1);
8396
expect(DdRum.addError.mock.calls[0][0]).toBe(String(error));
8497
expect(DdRum.addError.mock.calls[0][1]).toBe("SOURCE");
8598
expect(DdRum.addError.mock.calls[0][2]).toBe("at ./path/to/file.js:1038:57");
86-
expect(DdRum.addError.mock.calls[0][4]).toStrictEqual(error);
99+
const attributes = DdRum.addError.mock.calls[0][4];
100+
expect(attributes["_dd.error.raw"]).toStrictEqual(error);
101+
expect(attributes["_dd.error.is_crash"]).toStrictEqual(is_fatal);
87102
expect(baseErrorHandlerCalled).toStrictEqual(true);
88103
})
89104

90105

91-
it('M intercept and send a RUM event W onError() {with component stack}', async () => {
106+
it('M intercept and send a RUM event W onGlobalError() {with component stack}', async () => {
92107
// GIVEN
93-
DdRumErrorTracking.startTracking()
108+
DdRumErrorTracking.startTracking();
109+
const is_fatal = Math.random() < 0.5;
94110
const error = {
95111
componentStack: ["doSomething() at ./path/to/file.js:67:3", "nestedCall() at ./path/to/file.js:1064:9", "root() at ./path/to/index.js:10:1"],
96112
message: "Something bad happened"
97113
};
98114

99115
// WHEN
100-
DdRumErrorTracking.onError(error, false);
116+
DdRumErrorTracking.onGlobalError(error, is_fatal);
101117
await flushPromises();
102118

103119
// THEN
104120
expect(DdRum.addError.mock.calls.length).toBe(1);
105121
expect(DdRum.addError.mock.calls[0][0]).toBe(String(error));
106122
expect(DdRum.addError.mock.calls[0][1]).toBe("SOURCE");
107123
expect(DdRum.addError.mock.calls[0][2]).toBe("doSomething() at ./path/to/file.js:67:3,nestedCall() at ./path/to/file.js:1064:9,root() at ./path/to/index.js:10:1");
108-
expect(DdRum.addError.mock.calls[0][4]).toStrictEqual(error);
124+
const attributes = DdRum.addError.mock.calls[0][4];
125+
expect(attributes["_dd.error.raw"]).toStrictEqual(error);
126+
expect(attributes["_dd.error.is_crash"]).toStrictEqual(is_fatal);
109127
expect(baseErrorHandlerCalled).toStrictEqual(true);
110128
})
111129

130+
it('M intercept and send a RUM event W onConsole() {Error with source file info}', async () => {
131+
// GIVEN
132+
DdRumErrorTracking.startTracking();
133+
const message= "Something bad happened";
134+
const error = {
135+
sourceURL: "./path/to/file.js",
136+
line: 1038,
137+
column: 57,
138+
message: message
139+
};
140+
141+
// WHEN
142+
DdRumErrorTracking.onConsoleError(message, error);
143+
await flushPromises();
144+
145+
// THEN
146+
expect(DdRum.addError.mock.calls.length).toBe(1);
147+
expect(DdRum.addError.mock.calls[0][0]).toBe(message + " " + JSON.stringify(error));
148+
expect(DdRum.addError.mock.calls[0][1]).toBe("CONSOLE");
149+
expect(DdRum.addError.mock.calls[0][2]).toBe("at ./path/to/file.js:1038:57");
150+
expect(DdRum.addError.mock.calls[0][4]).toStrictEqual({})
151+
expect(baseConsoleErrorCalled).toStrictEqual(true);
152+
})
153+
154+
it('M intercept and send a RUM event W onConsole() {Error with component stack}', async () => {
155+
// GIVEN
156+
DdRumErrorTracking.startTracking();
157+
const message= "Something bad happened";
158+
const error = {
159+
componentStack: ["doSomething() at ./path/to/file.js:67:3", "nestedCall() at ./path/to/file.js:1064:9", "root() at ./path/to/index.js:10:1"],
160+
message: "Something bad happened"
161+
};
162+
163+
// WHEN
164+
DdRumErrorTracking.onConsoleError(message, error);
165+
await flushPromises();
166+
167+
// THEN
168+
expect(DdRum.addError.mock.calls.length).toBe(1);
169+
expect(DdRum.addError.mock.calls[0][0]).toBe(message + " " + JSON.stringify(error));
170+
expect(DdRum.addError.mock.calls[0][1]).toBe("CONSOLE");
171+
expect(DdRum.addError.mock.calls[0][2]).toBe("doSomething() at ./path/to/file.js:67:3,nestedCall() at ./path/to/file.js:1064:9,root() at ./path/to/index.js:10:1");
172+
expect(DdRum.addError.mock.calls[0][4]).toStrictEqual({})
173+
expect(baseConsoleErrorCalled).toStrictEqual(true);
174+
})
175+
176+
it('M intercept and send a RUM event W onConsole() {message only}', async () => {
177+
// GIVEN
178+
DdRumErrorTracking.startTracking();
179+
const message= "Something bad happened";
180+
181+
// WHEN
182+
DdRumErrorTracking.onConsoleError(message);
183+
await flushPromises();
184+
185+
// THEN
186+
expect(DdRum.addError.mock.calls.length).toBe(1);
187+
expect(DdRum.addError.mock.calls[0][0]).toBe(message);
188+
expect(DdRum.addError.mock.calls[0][1]).toBe("CONSOLE");
189+
expect(DdRum.addError.mock.calls[0][2]).toBe("");
190+
expect(DdRum.addError.mock.calls[0][4]).toStrictEqual({})
191+
expect(baseConsoleErrorCalled).toStrictEqual(true);
192+
})

‎src/rum/instrumentation/DdRumErrorTracking.tsx‎

Lines changed: 62 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { DdRum } from '../../foundation';
99

1010
const EMPTY_STACK_TRACE = ""
1111
const TYPE_SOURCE = "SOURCE"
12+
const TYPE_CONSOLE = "CONSOLE"
1213

1314
/**
1415
* Provides RUM auto-instrumentation feature to track errors as RUM events.
@@ -20,39 +21,87 @@ export class DdRumErrorTracking {
2021
// eslint-disable-next-line
2122
private static defaultErrorHandler = (_error: any, _isFatal?: boolean) => {}
2223

24+
// eslint-disable-next-line
25+
private static defaultConsoleError = (..._params: unknown[]) => {}
26+
2327
/**
2428
* Starts tracking errors and sends a RUM Error event every time an error is detected.
2529
*/
2630
static startTracking(): void {
2731
// extra safety to avoid wrapping the Error handler twice
28-
if (this.isTracking) {
32+
if (DdRumErrorTracking.isTracking) {
33+
console.log("DdRumErrorTracking already started");
2934
return
3035
}
3136

3237
if (ErrorUtils) {
33-
this.defaultErrorHandler = ErrorUtils.getGlobalHandler();
38+
DdRumErrorTracking.defaultErrorHandler = ErrorUtils.getGlobalHandler();
39+
DdRumErrorTracking.defaultConsoleError = console.error;
3440

35-
ErrorUtils.setGlobalHandler(this.onError);
41+
42+
ErrorUtils.setGlobalHandler(DdRumErrorTracking.onGlobalError);
43+
console.error = DdRumErrorTracking.onConsoleError;
3644

37-
this.isTracking = true;
45+
DdRumErrorTracking.isTracking = true;
3846
}
3947

4048
}
4149

4250
// eslint-disable-next-line
43-
static onError(error: any, isFatal?: boolean) {
44-
// Stack trace
45-
let stack = EMPTY_STACK_TRACE
46-
if (error.hasOwnProperty('componentStack')) {
47-
stack = String(error.componentStack);
48-
} else if (error.hasOwnProperty('sourceURL') && error.hasOwnProperty('line') && error.hasOwnProperty('column')) {
49-
stack = "at " + error.sourceURL + ":" + error.line + ":" + error.column
51+
static onGlobalError(error: any, isFatal?: boolean) {
52+
DdRum.addError(
53+
String(error),
54+
TYPE_SOURCE,
55+
DdRumErrorTracking.getErrorStackTrace(error),
56+
new Date().getTime(),
57+
{ "_dd.error.is_crash": isFatal, "_dd.error.raw": error }
58+
).then(() => {
59+
DdRumErrorTracking.defaultErrorHandler(error, isFatal);
60+
});
61+
}
62+
63+
static onConsoleError(...params: unknown[]) {
64+
let stack: string = EMPTY_STACK_TRACE
65+
for (let i = 0; i < params.length; i += 1) {
66+
const param = params[i];
67+
const paramStack = DdRumErrorTracking.getErrorStackTrace(param)
68+
if (paramStack != undefined && paramStack != EMPTY_STACK_TRACE){
69+
stack = paramStack;
70+
break;
71+
}
5072
}
5173

52-
DdRum.addError(String(error), TYPE_SOURCE, stack, new Date().getTime(), error).then(() => {
53-
this.defaultErrorHandler(error, isFatal)
74+
const message = params.map((param) => {
75+
if (typeof param === 'string') { return param }
76+
else if (param instanceof Error) { return String(param) }
77+
else { return JSON.stringify(param)}
78+
}).join(' ');
79+
80+
81+
DdRum.addError(
82+
message,
83+
TYPE_CONSOLE,
84+
stack,
85+
new Date().getTime(),
86+
{}
87+
).then(() => {
88+
DdRumErrorTracking.defaultConsoleError.apply(console, params);
5489
});
90+
5591
}
5692

93+
private static getErrorStackTrace(error: any | undefined): string {
94+
let stack = EMPTY_STACK_TRACE
95+
if (error == undefined) {
96+
stack = EMPTY_STACK_TRACE;
97+
} else if (typeof error === 'string') {
98+
stack = EMPTY_STACK_TRACE;
99+
} else if ('componentStack' in error) {
100+
stack = String(error.componentStack);
101+
} else if (('sourceURL' in error) && ('line' in error) && ('column' in error)) {
102+
stack = `at ${error.sourceURL}:${error.line}:${error.column}`;
103+
}
104+
return stack
105+
}
57106
}
58107

‎src/rum/instrumentation/DdRumReactNavigationTracking.tsx‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,16 @@ export default class DdRumReactNavigationTracking {
3434
return;
3535
}
3636

37-
if (this.registeredContainer != null && this.registeredContainer !== navigationRef) {
37+
if (DdRumReactNavigationTracking.registeredContainer != null && this.registeredContainer !== navigationRef) {
3838
console.error('Cannot track new navigation container while another one is still tracked');
39-
} else if (this.registeredContainer == null) {
40-
const listener = this.resolveNavigationStateChangeListener();
41-
this.handleRouteNavigation(navigationRef.getCurrentRoute());
39+
} else if (DdRumReactNavigationTracking.registeredContainer == null) {
40+
const listener = DdRumReactNavigationTracking.resolveNavigationStateChangeListener();
41+
DdRumReactNavigationTracking.handleRouteNavigation(navigationRef.getCurrentRoute());
4242
navigationRef.addListener("state", listener);
43-
this.registeredContainer = navigationRef;
43+
DdRumReactNavigationTracking.registeredContainer = navigationRef;
4444
}
4545

46-
this.registerAppStateListenerIfNeeded();
46+
DdRumReactNavigationTracking.registerAppStateListenerIfNeeded();
4747
}
4848

4949
/**
@@ -52,8 +52,8 @@ export default class DdRumReactNavigationTracking {
5252
*/
5353
static stopTrackingViews(navigationRef: NavigationContainerRef | null): void {
5454
if (navigationRef != null) {
55-
navigationRef.removeListener("state", this.navigationStateChangeListener);
56-
this.registeredContainer = null;
55+
navigationRef.removeListener("state", DdRumReactNavigationTracking.navigationStateChangeListener);
56+
DdRumReactNavigationTracking.registeredContainer = null;
5757
}
5858
}
5959

@@ -67,25 +67,25 @@ export default class DdRumReactNavigationTracking {
6767
}
6868

6969
private static resolveNavigationStateChangeListener(): NavigationListener {
70-
if (this.navigationStateChangeListener == null) {
70+
if (DdRumReactNavigationTracking.navigationStateChangeListener == null) {
7171
// eslint-disable-next-line @typescript-eslint/no-explicit-any
72-
this.navigationStateChangeListener = (event: EventArg<string, boolean, any>) => {
72+
DdRumReactNavigationTracking.navigationStateChangeListener = (event: EventArg<string, boolean, any>) => {
7373
let nestedRoute = event.data?.state?.routes[event.data?.state?.index];
7474
while (nestedRoute.state != undefined) {
7575
nestedRoute = nestedRoute.state.routes[nestedRoute.state.index];
7676
}
7777

78-
this.handleRouteNavigation(nestedRoute);
78+
DdRumReactNavigationTracking.handleRouteNavigation(nestedRoute);
7979
};
8080
}
81-
return this.navigationStateChangeListener;
81+
return DdRumReactNavigationTracking.navigationStateChangeListener;
8282
}
8383

8484
private static registerAppStateListenerIfNeeded() {
85-
if (this.appStateListener == null) {
86-
this.appStateListener = (appStateStatus: AppStateStatus) => {
85+
if (DdRumReactNavigationTracking.appStateListener == null) {
86+
DdRumReactNavigationTracking.appStateListener = (appStateStatus: AppStateStatus) => {
8787

88-
const currentRoute = this.registeredContainer?.getCurrentRoute();
88+
const currentRoute = DdRumReactNavigationTracking.registeredContainer?.getCurrentRoute();
8989
const currentViewKey = currentRoute?.key;
9090
const currentViewName = currentRoute?.name;
9191

@@ -101,7 +101,7 @@ export default class DdRumReactNavigationTracking {
101101
};
102102

103103
// AppState is singleton, so we should add a listener only once in the app lifetime
104-
AppState.addEventListener("change", this.appStateListener);
104+
AppState.addEventListener("change", DdRumReactNavigationTracking.appStateListener);
105105
}
106106
}
107107

0 commit comments

Comments
 (0)