Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions packages/core/src/js/integrations/debugsymbolicator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ async function processEvent(event: Event, hint: EventHint): Promise<Event> {
async function symbolicate(rawStack: string, skipFirstFrames: number = 0): Promise<SentryStackFrame[] | null> {
try {
const parsedStack = parseErrorStack(rawStack);
if (parsedStack.length === 0) {
debug.warn('parseErrorStack returned empty array, skipping symbolication');
return null;
}

const prettyStack = await symbolicateStackTrace(parsedStack);
if (!prettyStack) {
Expand Down
63 changes: 58 additions & 5 deletions packages/core/src/js/integrations/debugsymbolicatorutils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { StackFrame as SentryStackFrame } from '@sentry/core';

import { debug } from '@sentry/core';
import { debug, parseStackFrames } from '@sentry/core';
import { defaultStackParser } from '@sentry/react';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The fallback stack parser from @sentry/react may not support Hermes bytecode-offset stack traces from production React Native builds, causing symbolication to fail silently.
Severity: MEDIUM

Suggested Fix

Ensure the fallback parser can handle all common React Native stack trace formats, including Hermes bytecode offsets. This could involve using a more appropriate RN-specific parser or extending the existing defaultStackParser and adding test cases that explicitly cover production Hermes stack formats (e.g., p@1:132161).

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: packages/core/src/js/integrations/debugsymbolicatorutils.ts#L4

Potential issue: The new fallback logic uses `defaultStackParser` from `@sentry/react`,
a browser-oriented parser. This parser is not guaranteed to handle all React Native
stack trace formats, particularly the Hermes bytecode-offset format (e.g., `p@1:132161`)
common in production builds. While tests confirm it handles V8-style Hermes traces,
there is no coverage for the bytecode format. When the primary
`ReactNative.Devtools.parseErrorStack` is unavailable and an error with a bytecode stack
occurs, the fallback will likely fail to parse the frames. The current implementation
handles this by returning an empty array, which causes symbolication to be silently
skipped, leading to unsymbolicated stack traces in Sentry reports for production errors.

Did we get this right? 👍 / 👎 to inform future reviews.


import type * as ReactNative from '../vendor/react-native';

Expand Down Expand Up @@ -72,13 +73,65 @@ function getSentryMetroSourceContextUrl(): string | undefined {
}

/**
* Loads and calls RN Core Devtools parseErrorStack function.
* Converts Sentry StackFrames to React Native StackFrames.
* This is the reverse of convertReactNativeFramesToSentryFrames in debugsymbolicator.ts
*/
function convertSentryFramesToReactNativeFrames(frames: SentryStackFrame[]): Array<ReactNative.StackFrame> {
// Reverse the frames because Sentry parser returns them in reverse order compared to RN
return frames.reverse().map((frame): ReactNative.StackFrame => {
const rnFrame: ReactNative.StackFrame = {
methodName: frame.function || '?',
};

if (frame.filename !== undefined) {
rnFrame.file = frame.filename;
}

if (frame.lineno !== undefined) {
rnFrame.lineNumber = frame.lineno;
}

if (frame.colno !== undefined) {
rnFrame.column = frame.colno;
}

return rnFrame;
});
}

/**
* Parses an error stack string into React Native StackFrames.
* Uses RN Devtools parseErrorStack by default for compatibility.
* Falls back to Sentry's built-in stack parser if Devtools is not available.
*
* @param errorStack - Raw stack trace string from Error.stack
* @returns Array of React Native StackFrame objects
*/
export function parseErrorStack(errorStack: string): Array<ReactNative.StackFrame> {
if (!ReactNativeLibraries.Devtools) {
throw new Error('React Native Devtools not available.');
// Try using RN Devtools first for maximum compatibility with existing tooling
if (ReactNativeLibraries.Devtools?.parseErrorStack) {
try {
return ReactNativeLibraries.Devtools.parseErrorStack(errorStack);
} catch (error) {
debug.warn('RN Devtools parseErrorStack failed, falling back to Sentry stack parser');
}
}

// Fallback: Use Sentry's stack parser (works without RN Devtools dependency)
try {
// Create a temporary Error object with the stack
const error = new Error();
error.stack = errorStack;

// Use Sentry's parser to parse the stack
const sentryFrames = parseStackFrames(defaultStackParser, error);

// Convert Sentry frames back to RN format
return convertSentryFramesToReactNativeFrames(sentryFrames);
} catch (error) {
debug.error('Failed to parse error stack:', error);
return [];
}
return ReactNativeLibraries.Devtools.parseErrorStack(errorStack);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/js/utils/rnlibrariesinterface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export type { EmitterSubscription } from 'react-native/Libraries/vendor/emitter/

export interface ReactNativeLibrariesInterface {
Devtools?: {
parseErrorStack: (errorStack: string) => Array<ReactNative.StackFrame>;
parseErrorStack?: (errorStack: string) => Array<ReactNative.StackFrame>;
symbolicateStackTrace: (
stack: Array<ReactNative.StackFrame>,
extraData?: Record<string, unknown>,
Expand Down
55 changes: 55 additions & 0 deletions packages/core/test/integrations/debugsymbolicator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,61 @@ describe('Debug Symbolicator Integration', () => {
});
});

it('should not wipe original frames when parseErrorStack returns empty array', async () => {
(parseErrorStack as jest.Mock).mockReturnValue([]);

const originalFrames = [
{
function: 'originalFoo',
filename: '/original/path/foo.js',
lineno: 10,
colno: 5,
},
{
function: 'originalBar',
filename: '/original/path/bar.js',
lineno: 20,
colno: 15,
},
];

const symbolicatedEvent = await processEvent(
{
exception: {
values: [
{
type: 'Error',
value: 'Error: test',
stacktrace: {
frames: originalFrames,
},
},
],
},
},
{
originalException: {
stack: mockRawStack,
},
},
);

// Original frames should be preserved when parseErrorStack returns empty array
expect(symbolicatedEvent).toStrictEqual(<Event>{
exception: {
values: [
{
type: 'Error',
value: 'Error: test',
stacktrace: {
frames: originalFrames,
},
},
],
},
});
});

it('should symbolicate error with different amount of exception hints ', async () => {
// Example: Sentry captures an Error with 20 Causes, but limits the captured exceptions to
// 5 in event.exception. Meanwhile, hint.originalException contains all 20 items.
Expand Down
68 changes: 68 additions & 0 deletions packages/core/test/integrations/debugsymbolicatorutils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { parseErrorStack } from '../../src/js/integrations/debugsymbolicatorutils';

describe('parseErrorStack', () => {
it('should parse Chrome-style stack trace', () => {
const stack = `Error: Test error
at foo (http://localhost:8081/index.bundle:10:15)
at bar (http://localhost:8081/index.bundle:20:25)`;

const frames = parseErrorStack(stack);

expect(frames).toHaveLength(2);
expect(frames[0]).toMatchObject({
methodName: 'foo',
file: 'http://localhost:8081/index.bundle',
lineNumber: 10,
});
expect(frames[0].column).toBeDefined();
expect(frames[1]).toMatchObject({
methodName: 'bar',
file: 'http://localhost:8081/index.bundle',
lineNumber: 20,
});
expect(frames[1].column).toBeDefined();
});

it('should handle anonymous functions', () => {
const stack = `Error: Test error
at <anonymous> (http://localhost:8081/index.bundle:10:15)`;

const frames = parseErrorStack(stack);

expect(frames.length).toBeGreaterThanOrEqual(1);
expect(frames[0].methodName).toBeDefined();
expect(frames[0].lineNumber).toBe(10);
});

it('should handle empty stack', () => {
const frames = parseErrorStack('');

expect(frames).toEqual([]);
});

it('should handle malformed stack gracefully', () => {
const frames = parseErrorStack('Not a valid stack trace');

expect(Array.isArray(frames)).toBe(true);
});

it('should preserve Metro bundle URLs with query params', () => {
const stack = `Error: Test error
at App (http://localhost:8081/index.bundle?platform=ios&dev=true:1:1)`;

const frames = parseErrorStack(stack);

expect(frames[0].file).toContain('platform=ios');
expect(frames[0].methodName).toBe('App');
});

it('should handle frames without line/column info', () => {
const stack = `Error: Test error
at native`;

const frames = parseErrorStack(stack);

// Should not crash, may return empty or partial frames
expect(Array.isArray(frames)).toBe(true);
});
});
Loading