Skip to content

Commit 5ecc496

Browse files
committed
Fix React Native PR review findings
1 parent d79517e commit 5ecc496

11 files changed

Lines changed: 195 additions & 47 deletions

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ The definition of the word exceptionless is: to be without exception. Exceptionl
1010

1111
You can install the npm package via `npm install @exceptionless/browser --save`
1212
or via cdn [`https://unpkg.com/@exceptionless/browser`](https://unpkg.com/@exceptionless/browser).
13-
Next, you just need to call startup during your apps startup to automatically
13+
Next, you just need to call startup during your app's startup to automatically
1414
capture unhandled errors.
1515

1616
```js

example/expo/screens/ErrorsScreen.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ function CrashyComponent(): ReactElement {
88
}
99

1010
export default function ErrorsScreen() {
11+
const [boundaryKey, setBoundaryKey] = useState(0);
1112
const [showCrashy, setShowCrashy] = useState(false);
1213
const [status, setStatus] = useState("");
1314
const stressRunning = useRef(false);
@@ -39,6 +40,7 @@ export default function ErrorsScreen() {
3940

4041
const resetErrorBoundary = () => {
4142
setShowCrashy(false);
43+
setBoundaryKey((key) => key + 1);
4244
setStatus("Error boundary reset");
4345
};
4446

@@ -88,6 +90,7 @@ export default function ErrorsScreen() {
8890
<View style={styles.section}>
8991
<Text style={styles.sectionTitle}>Error Boundary</Text>
9092
<ExceptionlessErrorBoundary
93+
key={boundaryKey}
9194
fallback={
9295
<View>
9396
<Text style={styles.errorText}>Component crashed! Error was sent to Exceptionless.</Text>

packages/react-native/expo-plugin/withExceptionless.cjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ const pkg = require("../package.json");
22

33
const withExceptionless = (config) => config;
44

5-
let plugin = withExceptionless;
5+
let plugin;
66

77
try {
88
const { createRunOncePlugin } = require("@expo/config-plugins");

packages/react-native/src/ExceptionlessErrorBoundary.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ export class ExceptionlessErrorBoundary extends Component<PropsWithChildren<Erro
3232
}
3333

3434
render(): ReactNode {
35-
if (this.state.hasError && this.props.fallback) {
36-
return this.props.fallback;
35+
if (this.state.hasError) {
36+
return this.props.fallback ?? null;
3737
}
3838

3939
return this.props.children;

packages/react-native/src/native/ExceptionlessNativeModule.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { NativeModules, Platform } from "react-native";
22

33
export interface CrashReportFrame {
44
address: string;
5-
image: string;
5+
image: string | null;
66
symbol: string | null;
77
offset: number | null;
88
}

packages/react-native/src/plugins/NativeCrashPlugin.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ export class NativeCrashPlugin implements IEventPlugin {
3939
}
4040

4141
const reports = await nativeModule.getPendingCrashReports();
42+
if (reports.length === 0) {
43+
this._client.config.services.log.warn("Native crash reporter indicated a pending crash, but no crash reports were returned.");
44+
return;
45+
}
46+
4247
for (const report of reports) {
4348
await this.submitCrashReport(report);
4449
}

packages/react-native/src/plugins/ReactNativeErrorPlugin.ts

Lines changed: 129 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ export class ReactNativeErrorPlugin implements IEventPlugin {
7676
}
7777

7878
const frames: StackFrameInfo[] = [];
79-
const lines = stack.split(/\r?\n/);
79+
const lines = stack.replaceAll("\r\n", "\n").replaceAll("\r", "\n").split("\n");
8080
for (const line of lines) {
8181
const frame = this.parseStackFrame(line.trim());
8282
if (frame) {
@@ -92,24 +92,27 @@ export class ReactNativeErrorPlugin implements IEventPlugin {
9292
return null;
9393
}
9494

95-
const reactNativeFrame = /^at\s+(.*?)\(\)\s+in\s+(.*)$/.exec(line);
96-
if (reactNativeFrame) {
97-
return this.createStackFrame(reactNativeFrame[1], reactNativeFrame[2]);
98-
}
95+
if (line.startsWith("at ")) {
96+
const frame = line.slice(3).trim();
97+
const reactNativeMarker = "() in ";
98+
const reactNativeMarkerIndex = frame.indexOf(reactNativeMarker);
99+
if (reactNativeMarkerIndex > 0) {
100+
return this.createStackFrame(frame.slice(0, reactNativeMarkerIndex), frame.slice(reactNativeMarkerIndex + reactNativeMarker.length));
101+
}
99102

100-
const atFrame = /^at\s+(.*?)\s+\((.*)\)$/.exec(line);
101-
if (atFrame) {
102-
return this.createStackFrame(atFrame[1], atFrame[2]);
103-
}
103+
if (frame.endsWith(")")) {
104+
const locationStartIndex = frame.lastIndexOf(" (");
105+
if (locationStartIndex > 0) {
106+
return this.createStackFrame(frame.slice(0, locationStartIndex), frame.slice(locationStartIndex + 2, -1));
107+
}
108+
}
104109

105-
const anonymousAtFrame = /^at\s+(.*)$/.exec(line);
106-
if (anonymousAtFrame) {
107-
return this.createStackFrame("<anonymous>", anonymousAtFrame[1]);
110+
return this.createStackFrame("<anonymous>", frame);
108111
}
109112

110-
const hermesFrame = /^(.*?)@(.*)$/.exec(line);
111-
if (hermesFrame) {
112-
return this.createStackFrame(hermesFrame[1] || "<anonymous>", hermesFrame[2]);
113+
const hermesLocationIndex = line.indexOf("@");
114+
if (hermesLocationIndex >= 0) {
115+
return this.createStackFrame(line.slice(0, hermesLocationIndex) || "<anonymous>", line.slice(hermesLocationIndex + 1));
113116
}
114117

115118
return null;
@@ -130,7 +133,7 @@ export class ReactNativeErrorPlugin implements IEventPlugin {
130133
}
131134

132135
private parseLocation(location: string): ParsedLocation {
133-
const trimmedLocation = location.trim();
136+
let trimmedLocation = location.trim();
134137
if (trimmedLocation === "native" || trimmedLocation === "<anonymous>") {
135138
return {
136139
fileName: trimmedLocation,
@@ -140,53 +143,138 @@ export class ReactNativeErrorPlugin implements IEventPlugin {
140143
};
141144
}
142145

143-
const reactNativeLineAndColumn = /^(?:address at )?(.*):line (\d+):col (\d+)$/.exec(trimmedLocation);
144-
if (reactNativeLineAndColumn) {
145-
return {
146-
fileName: reactNativeLineAndColumn[1],
147-
lineNumber: Number(reactNativeLineAndColumn[2]),
148-
column: Number(reactNativeLineAndColumn[3]),
149-
isNative: false
150-
};
146+
if (trimmedLocation.startsWith("address at ")) {
147+
trimmedLocation = trimmedLocation.slice("address at ".length);
151148
}
152149

153-
const lineAndColumn = /^(.*):(\d+):(\d+)$/.exec(trimmedLocation);
150+
const reactNativeLocation = this.tryParseReactNativeLocation(trimmedLocation);
151+
if (reactNativeLocation) {
152+
return reactNativeLocation;
153+
}
154+
155+
const lineAndColumn = this.tryParseDelimitedLocation(trimmedLocation, true);
154156
if (lineAndColumn) {
155-
return {
156-
fileName: lineAndColumn[1],
157-
lineNumber: Number(lineAndColumn[2]),
158-
column: Number(lineAndColumn[3]),
159-
isNative: false
160-
};
157+
return lineAndColumn;
161158
}
162159

163-
const lineOnly = /^(.*):(\d+)$/.exec(trimmedLocation);
160+
const lineOnly = this.tryParseDelimitedLocation(trimmedLocation, false);
164161
if (lineOnly) {
162+
return lineOnly;
163+
}
164+
165+
return {
166+
fileName: trimmedLocation,
167+
lineNumber: 0,
168+
column: 0,
169+
isNative: false
170+
};
171+
}
172+
173+
private tryParseReactNativeLocation(location: string): ParsedLocation | null {
174+
const columnMarker = ":col ";
175+
const lineMarker = ":line ";
176+
const columnMarkerIndex = location.lastIndexOf(columnMarker);
177+
if (columnMarkerIndex <= 0) {
178+
return null;
179+
}
180+
181+
const lineMarkerIndex = location.lastIndexOf(lineMarker, columnMarkerIndex);
182+
if (lineMarkerIndex <= 0) {
183+
return null;
184+
}
185+
186+
const lineNumber = this.tryParseNonNegativeInteger(location.slice(lineMarkerIndex + lineMarker.length, columnMarkerIndex));
187+
const column = this.tryParseNonNegativeInteger(location.slice(columnMarkerIndex + columnMarker.length));
188+
if (lineNumber === null || column === null) {
189+
return null;
190+
}
191+
192+
return {
193+
fileName: location.slice(0, lineMarkerIndex),
194+
lineNumber,
195+
column,
196+
isNative: false
197+
};
198+
}
199+
200+
private tryParseDelimitedLocation(location: string, hasColumn: boolean): ParsedLocation | null {
201+
const lastDelimiterIndex = location.lastIndexOf(":");
202+
if (lastDelimiterIndex <= 0) {
203+
return null;
204+
}
205+
206+
const lastNumber = this.tryParseNonNegativeInteger(location.slice(lastDelimiterIndex + 1));
207+
if (lastNumber === null) {
208+
return null;
209+
}
210+
211+
if (!hasColumn) {
165212
return {
166-
fileName: lineOnly[1],
167-
lineNumber: Number(lineOnly[2]),
213+
fileName: location.slice(0, lastDelimiterIndex),
214+
lineNumber: lastNumber,
168215
column: 0,
169216
isNative: false
170217
};
171218
}
172219

220+
const lineDelimiterIndex = location.lastIndexOf(":", lastDelimiterIndex - 1);
221+
if (lineDelimiterIndex <= 0) {
222+
return null;
223+
}
224+
225+
const lineNumber = this.tryParseNonNegativeInteger(location.slice(lineDelimiterIndex + 1, lastDelimiterIndex));
226+
if (lineNumber === null) {
227+
return null;
228+
}
229+
173230
return {
174-
fileName: trimmedLocation,
175-
lineNumber: 0,
176-
column: 0,
231+
fileName: location.slice(0, lineDelimiterIndex),
232+
lineNumber,
233+
column: lastNumber,
177234
isNative: false
178235
};
179236
}
180237

238+
private tryParseNonNegativeInteger(value: string): number | null {
239+
if (!value) {
240+
return null;
241+
}
242+
243+
for (let i = 0; i < value.length; i++) {
244+
const charCode = value.charCodeAt(i);
245+
if (charCode < 48 || charCode > 57) {
246+
return null;
247+
}
248+
}
249+
250+
return Number(value);
251+
}
252+
181253
private normalizeFrameName(name: string): string {
182-
const trimmedName = name.trim().replace(/\(\)$/, "");
254+
const trimmed = name.trim();
255+
const trimmedName = trimmed.endsWith("()") ? trimmed.slice(0, -2) : trimmed;
183256
if (!trimmedName || trimmedName === "?") {
184257
return "<anonymous>";
185258
}
186259

187-
const syntheticHermesName = /^\?anon_\d+_?(.*)$/.exec(trimmedName);
188-
if (syntheticHermesName) {
189-
return syntheticHermesName[1] || "<anonymous>";
260+
const syntheticHermesPrefix = "?anon_";
261+
if (trimmedName.startsWith(syntheticHermesPrefix)) {
262+
let suffixIndex = syntheticHermesPrefix.length;
263+
while (suffixIndex < trimmedName.length) {
264+
const charCode = trimmedName.charCodeAt(suffixIndex);
265+
if (charCode < 48 || charCode > 57) {
266+
break;
267+
}
268+
269+
suffixIndex++;
270+
}
271+
272+
if (trimmedName[suffixIndex] === "_") {
273+
const suffix = trimmedName.slice(suffixIndex + 1);
274+
return suffix || "<anonymous>";
275+
}
276+
277+
return "<anonymous>";
190278
}
191279

192280
return trimmedName;

packages/react-native/src/plugins/ReactNativeLifeCyclePlugin.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export class ReactNativeLifeCyclePlugin implements IEventPlugin {
3333
AppState.addEventListener("change", (nextAppState: AppStateStatus) => {
3434
if (nextAppState === "active") {
3535
void this._client?.startup();
36-
} else if (nextAppState === "background" || nextAppState === "inactive") {
36+
} else if (nextAppState === "background") {
3737
if (this._client?.config.sessionsEnabled) {
3838
void this._client?.submitSessionEnd();
3939
}

packages/react-native/test/plugins/ReactNativeErrorPlugin.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,24 @@ describe("ReactNativeErrorPlugin", () => {
163163
]);
164164
});
165165

166+
test("should not parse numeric error headers as stack frames", async () => {
167+
const error = new Error("404");
168+
error.stack = ["Error: 404", "at loadRoute() in http://localhost:8083/index.bundle:line 10:col 20"].join("\n");
169+
context.eventContext.setException(error);
170+
171+
await plugin.run(context);
172+
173+
expect(getError(context.event)?.stack_trace).toEqual([
174+
expect.objectContaining({
175+
column: 20,
176+
file_name: "http://localhost:8083/index.bundle",
177+
is_signature_target: true,
178+
line_number: 10,
179+
name: "loadRoute"
180+
})
181+
]);
182+
});
183+
166184
test("should attach React component stack to error data", async () => {
167185
const error = new Error("Component crashed inside error boundary!");
168186
const componentStack = "\n at CrashyComponent (http://localhost:8083/index.bundle:10:20)";

packages/react-native/test/plugins/ReactNativeLifeCyclePlugin.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,29 @@ describe("ReactNativeLifeCyclePlugin", () => {
6363

6464
addEventListenerSpy.mockRestore();
6565
});
66+
67+
test("should end native session only when app enters background", async () => {
68+
const { Platform } = await import("react-native");
69+
const { AppState } = await import("react-native");
70+
Object.defineProperty(Platform, "OS", { value: "ios", writable: true });
71+
client.config.useSessions();
72+
73+
const addEventListenerSpy = vi.spyOn(AppState, "addEventListener");
74+
const submitSessionEndSpy = vi.spyOn(client, "submitSessionEnd").mockResolvedValue("reference-id");
75+
const suspendSpy = vi.spyOn(client, "suspend").mockResolvedValue();
76+
77+
await plugin.startup({ client, log: client.config.services.log });
78+
79+
const appStateHandler = addEventListenerSpy.mock.calls[0]?.[1];
80+
expect(appStateHandler).toBeDefined();
81+
appStateHandler?.("inactive");
82+
appStateHandler?.("background");
83+
84+
expect(submitSessionEndSpy).toHaveBeenCalledTimes(1);
85+
expect(suspendSpy).toHaveBeenCalledTimes(1);
86+
87+
addEventListenerSpy.mockRestore();
88+
submitSessionEndSpy.mockRestore();
89+
suspendSpy.mockRestore();
90+
});
6691
});

0 commit comments

Comments
 (0)