Skip to content

feat(sdk): Add Hermes Debug Info flag to React Native Context #3290

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Sep 19, 2023
Merged

feat(sdk): Add Hermes Debug Info flag to React Native Context #3290

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a769be7
fix(hermes): Convert Hermes bytecode col pos to 1-based format used b…
krystofwoldrich Sep 15, 2023
64125d5
Add hermes bytecode check and update tests
krystofwoldrich Sep 18, 2023
6ed4276
Add changelog
krystofwoldrich Sep 18, 2023
867d4d0
feat(sdk): Add Hermes Debug Info flag to React Native Context
krystofwoldrich Sep 18, 2023
6227986
fix lint
krystofwoldrich Sep 18, 2023
25d3697
Export hermes debug info logic to function, fix tests
krystofwoldrich Sep 19, 2023
729a995
Merge remote-tracking branch 'origin/main' into kw-add-hermes-debug-info
krystofwoldrich Sep 19, 2023
078eb28
Fix changelog
krystofwoldrich Sep 19, 2023
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,15 @@

## Unreleased

### Features

- Add Hermes Debug Info flag to React Native Context ([#3290](https://github.com/getsentry/sentry-react-native/pull/3290))
- This flag equals `true` when Hermes Bundle contains Debug Info (Hermes Source Map was not emitted)

### Fixes

- Create profiles for start up transactions ([#3281](https://github.com/getsentry/sentry-react-native/pull/3281))
- Fix Hermes Bytecode Symbolication one line off ([#3283](https://github.com/getsentry/sentry-react-native/pull/3283))

### Dependencies

Expand Down
14 changes: 13 additions & 1 deletion src/js/integrations/reactnativeinfo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export interface ReactNativeContext extends Context {
hermes_version?: string;
react_native_version: string;
component_stack?: string;
hermes_debug_info?: boolean;
}

/** Loads React Native context at runtime */
Expand Down Expand Up @@ -50,7 +51,18 @@ export class ReactNativeInfo implements Integration {
reactNativeContext.js_engine = 'hermes';
const hermesVersion = getHermesVersion();
if (hermesVersion) {
reactNativeContext.hermes_version = getHermesVersion();
reactNativeContext.hermes_version = hermesVersion;
}

// Check if Hermes Bundle has debug info
for (const value of event.exception?.values || event.threads?.values || []) {
for (const frame of value.stacktrace?.frames || []) {
// platform === undefined we assume it's javascript (only native frames use the platform attribute)
if (frame.platform === undefined && frame.lineno === 1) {
reactNativeContext.hermes_debug_info = true;
break;
}
}
}
} else if (reactNativeError?.jsEngine) {
reactNativeContext.js_engine = reactNativeError.jsEngine;
Expand Down
11 changes: 10 additions & 1 deletion src/js/integrations/rewriteframes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { RewriteFrames } from '@sentry/integrations';
import type { StackFrame } from '@sentry/types';
import { Platform } from 'react-native';

import { isExpo } from '../utils/environment';
import { isExpo, isHermesEnabled } from '../utils/environment';

export const ANDROID_DEFAULT_BUNDLE_NAME = 'app:///index.android.bundle';
export const IOS_DEFAULT_BUNDLE_NAME = 'app:///main.jsbundle';
Expand Down Expand Up @@ -35,6 +35,15 @@ export function createReactNativeRewriteFrames(): RewriteFrames {
if (frame.filename === '[native code]' || frame.filename === 'native') {
return frame;
}
// Is React Native frame

// Check Hermes Bytecode Frame and convert to 1-based column
if (isHermesEnabled() && frame.lineno === 1 && frame.colno !== undefined) {
// hermes bytecode columns are 0-based, while v8 and jsc are 1-based
// Hermes frames without debug info have always line = 1 and col points to a bytecode pos
// https://github.com/facebook/react/issues/21792#issuecomment-873171991
frame.colno += 1;
}

// Expo adds hash to the end of bundle names
if (isExpo() && Platform.OS === 'android') {
Expand Down
84 changes: 84 additions & 0 deletions test/integrations/reactnativeinfo.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,90 @@ describe('React Native Info', () => {
test: 'context',
});
});

it('add hermes_debug_info to react_native_context based on exception frames', async () => {
mockedIsHermesEnabled = jest.fn().mockReturnValue(true);

const mockedEvent: Event = {
exception: {
values: [
{
stacktrace: {
frames: [
{
platform: 'java',
lineno: 2,
},
{
lineno: 1,
},
],
},
},
],
},
};
const actualEvent = await executeIntegrationFor(mockedEvent, {});

expectMocksToBeCalledOnce();
expect(actualEvent?.contexts?.react_native_context?.hermes_debug_info).toEqual(true);
});

it('add hermes_debug_info to react_native_context based on threads frames', async () => {
mockedIsHermesEnabled = jest.fn().mockReturnValue(true);

const mockedEvent: Event = {
threads: {
values: [
{
stacktrace: {
frames: [
{
platform: 'java',
lineno: 2,
},
{
lineno: 1,
},
],
},
},
],
},
};
const actualEvent = await executeIntegrationFor(mockedEvent, {});

expectMocksToBeCalledOnce();
expect(actualEvent?.contexts?.react_native_context?.hermes_debug_info).toEqual(true);
});

it('does not add hermes_debug_info to react_native_context (no hermes bytecode frames)', async () => {
mockedIsHermesEnabled = jest.fn().mockReturnValue(true);

const mockedEvent: Event = {
threads: {
values: [
{
stacktrace: {
frames: [
{
platform: 'java',
lineno: 2,
},
{
lineno: 2,
},
],
},
},
],
},
};
const actualEvent = await executeIntegrationFor(mockedEvent, {});

expectMocksToBeCalledOnce();
expect(actualEvent?.contexts?.react_native_context?.hermes_debug_info).toEqual(undefined);
});
});

function expectMocksToBeCalledOnce() {
Expand Down
41 changes: 22 additions & 19 deletions test/integrations/rewriteframes.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { Event } from '@sentry/types';
import { Platform } from 'react-native';

import { createReactNativeRewriteFrames } from '../../src/js/integrations/rewriteframes';
import { isExpo } from '../../src/js/utils/environment';
import { isExpo, isHermesEnabled } from '../../src/js/utils/environment';
import { mockFunction } from '../testutils';

jest.mock('../../src/js/utils/environment');
Expand All @@ -29,6 +29,7 @@ describe('RewriteFrames', () => {

beforeEach(() => {
mockFunction(isExpo).mockReturnValue(false);
mockFunction(isHermesEnabled).mockReturnValue(false);
jest.resetAllMocks();
});

Expand Down Expand Up @@ -668,6 +669,8 @@ describe('RewriteFrames', () => {
});

it('should parse React Native errors on Android Hermes', async () => {
mockFunction(isHermesEnabled).mockReturnValue(true);

const ANDROID_REACT_NATIVE_HERMES = {
message: 'Error: lets throw!',
name: 'Error',
Expand Down Expand Up @@ -714,28 +717,28 @@ describe('RewriteFrames', () => {
filename: 'app:///index.android.bundle',
function: 'value',
lineno: 1,
colno: 31561,
colno: 31562,
in_app: true,
},
{
filename: 'app:///index.android.bundle',
function: 'value',
lineno: 1,
colno: 32776,
colno: 32777,
in_app: true,
},
{
filename: 'app:///index.android.bundle',
function: 'anonymous',
lineno: 1,
colno: 31603,
colno: 31604,
in_app: true,
},
{
filename: 'app:///index.android.bundle',
function: 'value',
lineno: 1,
colno: 33176,
colno: 33177,
in_app: true,
},
{
Expand All @@ -747,42 +750,42 @@ describe('RewriteFrames', () => {
filename: 'app:///index.android.bundle',
function: 'receiveTouches',
lineno: 1,
colno: 122512,
colno: 122513,
in_app: true,
},
{
filename: 'app:///index.android.bundle',
function: 'Ue',
lineno: 1,
colno: 77571,
colno: 77572,
in_app: true,
},
{
filename: 'app:///index.android.bundle',
function: 'Ne',
lineno: 1,
colno: 77238,
colno: 77239,
in_app: true,
},
{
filename: 'app:///index.android.bundle',
function: '_e',
lineno: 1,
colno: 127755,
colno: 127756,
in_app: true,
},
{
filename: 'app:///index.android.bundle',
function: 'anonymous',
lineno: 1,
colno: 77747,
colno: 77748,
in_app: true,
},
{
filename: 'app:///index.android.bundle',
function: 'z',
lineno: 1,
colno: 74642,
colno: 74643,
in_app: true,
},
{
Expand All @@ -794,37 +797,37 @@ describe('RewriteFrames', () => {
filename: 'app:///index.android.bundle',
function: 'A',
lineno: 1,
colno: 74709,
colno: 74710,
in_app: true,
},
{
filename: 'app:///index.android.bundle',
function: 'N',
lineno: 1,
colno: 74267,
colno: 74268,
in_app: true,
},
{
filename: 'app:///index.android.bundle',
function: 'C',
lineno: 1,
colno: 74126,
colno: 74127,
in_app: true,
},
{ filename: 'native', function: 'apply', in_app: true },
{
filename: 'app:///index.android.bundle',
function: 'k',
lineno: 1,
colno: 74094,
colno: 74095,
in_app: true,
},
{ filename: 'native', function: 'apply', in_app: true },
{
filename: 'app:///index.android.bundle',
function: 'b',
lineno: 1,
colno: 74037,
colno: 74038,
in_app: true,
},
{ filename: 'native', function: 'apply', in_app: true },
Expand All @@ -835,21 +838,21 @@ describe('RewriteFrames', () => {
filename: 'app:///index.android.bundle',
function: '_performSideEffectsForTransition',
lineno: 1,
colno: 230843,
colno: 230844,
in_app: true,
},
{
filename: 'app:///index.android.bundle',
function: 'anonymous',
lineno: 1,
colno: 224280,
colno: 224281,
in_app: true,
},
{
filename: 'app:///index.android.bundle',
function: 'onPress',
lineno: 1,
colno: 452701,
colno: 452702,
in_app: true,
},
],
Expand Down