From abb7058e207628f7d3e5ebee6c410c2cf6c361f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kry=C5=A1tof=20Wold=C5=99ich?= <31292499+krystofwoldrich@users.noreply.github.com> Date: Mon, 18 Sep 2023 17:58:08 +0200 Subject: [PATCH] fix(hermes): Convert Hermes bytecode col pos to 1-based format (#3283) --- CHANGELOG.md | 1 + src/js/integrations/rewriteframes.ts | 11 ++++++- test/integrations/rewriteframes.test.ts | 41 +++++++++++++------------ 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b4dc1630..469bc0643 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/js/integrations/rewriteframes.ts b/src/js/integrations/rewriteframes.ts index 38773400f..77157e230 100644 --- a/src/js/integrations/rewriteframes.ts +++ b/src/js/integrations/rewriteframes.ts @@ -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'; @@ -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') { diff --git a/test/integrations/rewriteframes.test.ts b/test/integrations/rewriteframes.test.ts index 58867ef34..80e4b6544 100644 --- a/test/integrations/rewriteframes.test.ts +++ b/test/integrations/rewriteframes.test.ts @@ -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'); @@ -29,6 +29,7 @@ describe('RewriteFrames', () => { beforeEach(() => { mockFunction(isExpo).mockReturnValue(false); + mockFunction(isHermesEnabled).mockReturnValue(false); jest.resetAllMocks(); }); @@ -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', @@ -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, }, { @@ -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, }, { @@ -794,21 +797,21 @@ 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 }, @@ -816,7 +819,7 @@ describe('RewriteFrames', () => { filename: 'app:///index.android.bundle', function: 'k', lineno: 1, - colno: 74094, + colno: 74095, in_app: true, }, { filename: 'native', function: 'apply', in_app: true }, @@ -824,7 +827,7 @@ describe('RewriteFrames', () => { filename: 'app:///index.android.bundle', function: 'b', lineno: 1, - colno: 74037, + colno: 74038, in_app: true, }, { filename: 'native', function: 'apply', in_app: true }, @@ -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, }, ],