Skip to content

Commit

Permalink
fix(hermes): Convert Hermes bytecode col pos to 1-based format (#3283)
Browse files Browse the repository at this point in the history
  • Loading branch information
@krystofwoldrich
krystofwoldrich authored Sep 18, 2023
1 parent 4cc74aa commit abb7058
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 20 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### 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
11 changes: 10 additions & 1 deletion src/js/integrations/rewriteframes.ts
View file
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
41 changes: 22 additions & 19 deletions test/integrations/rewriteframes.test.ts
View file
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

0 comments on commit abb7058

Please sign in to comment.