Skip to content

Fix: don't symbolicate console asserts that comes from metro. #4770

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

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft

Fix: don't symbolicate console asserts that comes from metro. #4770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
4f7645e
Merge branch 'main' into lz/fix-console
lucas-zimerman Apr 22, 2025
5136274
add fix for console loop
lucas-zimerman Apr 23, 2025
3f1ff8a
fix changelog
lucas-zimerman Apr 23, 2025
f93d17a
merge main
lucas-zimerman Apr 23, 2025
e6f5273
Update samples/react-native/src/Screens/ErrorsScreen.tsx
lucas-zimerman Apr 23, 2025
1b1f86f
apply requested changes
lucas-zimerman Apr 24, 2025
cb9ef09
apply requested changes / add sample staccktrace / new test for cons…
lucas-zimerman Apr 25, 2025
2ec3cda
Merge branch 'main' of https://github.com/getsentry/sentry-react-nati…
lucas-zimerman Apr 25, 2025
37be219
Merge branch 'lz/fix-console' of https://github.com/getsentry/sentry-…
lucas-zimerman Apr 25, 2025
5be8e7c
Merge branch 'main' into lz/fix-console
lucas-zimerman Apr 28, 2025
04431cc
fix changelog
lucas-zimerman Apr 28, 2025
53a7d5f
Merge branch 'main' into lz/fix-console
lucas-zimerman Apr 29, 2025
85a69df
Merge branch 'main' into lz/fix-console
lucas-zimerman May 8, 2025
f474700
fix changelog
lucas-zimerman May 8, 2025
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

### Fixes

- App slowdown when using Sentry console Integration and filtering by `assert` ([#4770](https://github.com/getsentry/sentry-react-native/pull/4770))
- Expo Updates Context is passed to native after native init to be available for crashes ([#4808](https://github.com/getsentry/sentry-react-native/pull/4808))

### Dependencies
Expand Down
47 changes: 47 additions & 0 deletions packages/core/src/js/integrations/debugsymbolicator.consoleissue.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
Sample stack trace when calling `symbolicateStackTrace` from debugsymbolicator, with console integration enabled, with assert filter.

LOG Assertion called: true message 'this' is expected an Event object, but got [{"isTrusted":false}] stack : Error
at getCurrentStackTrace (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:125938:26)
at anonymous (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:125950:57)
at apply (native)
at anonymous (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:154465:27)
at pd (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:36235:19)
at setPassiveListener (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:36640:7)
at dispatchEvent (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:36939:27)
at setReadyState (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:36129:27)
at open (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:36041:27)
at apply (native)
at anonymous (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:155583:34)
at anonymous (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:29261:17)
at tryCallTwo (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:204678:9)
at doResolve (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:204817:25)
at Promise (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:204697:14)
at fetch (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:29218:25)
at ?anon_0_ (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:28649:33)
at next (native)
at asyncGeneratorStep (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:28665:26)
at _next (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:28684:29)
at anonymous (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:28689:14)
at tryCallTwo (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:204678:9)
at doResolve (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:204817:25)
at Promise (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:204697:14)
at anonymous (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:28681:25)
at apply (native)
at _symbolicateStackTrace (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:28658:40)
at apply (native)
at symbolicateStackTrace (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:28637:40)
at symbolicate (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:28616:52)
at handleSymbolicate (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:28516:42)
at symbolicate (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:28507:33)
at appendNewLog (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:28152:25)
at anonymous (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:28196:21)
at apply (native)
at anonymous (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:35288:26)
at _callTimer (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:35167:17)
at _callReactNativeMicrotasksPass (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:35212:17)
at callReactNativeMicrotasks (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:35418:44)
at __callReactNativeMicrotasks (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:3569:48)
at anonymous (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:3342:45)
at __guard (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:3541:15)
at flushedQueue (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:3341:21)
at callFunctionReturnFlushedQueue (http://IP/index.bundle//&platform=android&dev=true&lazy=true&minify=false&app=io.sentry.reactnative.sample&modulesOnly=false&runModule=true:3326:33)
6 changes: 4 additions & 2 deletions packages/core/src/js/integrations/debugsymbolicator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Event, EventHint, Exception, Integration, StackFrame as SentryStackFrame } from '@sentry/core';
import { logger } from '@sentry/core';
import { consoleSandbox, logger } from '@sentry/core';

import type { ExtendedError } from '../utils/error';
import { getFramesToPop, isErrorLike } from '../utils/error';
Expand Down Expand Up @@ -69,7 +69,9 @@ async function symbolicate(rawStack: string, skipFirstFrames: number = 0): Promi
try {
const parsedStack = parseErrorStack(rawStack);

const prettyStack = await symbolicateStackTrace(parsedStack);
// Avoid capturing console logs as events when calling symbolicateStackTrace to avoid infinite loops.
// See file `debugsymbolicator.consoleissue.txt` for a sample stack-trace with this problem.
const prettyStack = await consoleSandbox(async () => symbolicateStackTrace(parsedStack));
if (!prettyStack) {
logger.error('React Native DevServer could not symbolicate the stack trace.');
return null;
Expand Down
59 changes: 58 additions & 1 deletion packages/core/test/integrations/debugsymbolicator.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
jest.mock('../../src/js/integrations/debugsymbolicatorutils');

import type { Client, Event, EventHint, StackFrame } from '@sentry/core';
import {
type Client,
type Event,
type EventHint,
type StackFrame,
captureConsoleIntegration,
setCurrentClient,
} from '@sentry/core';

import { debugSymbolicatorIntegration } from '../../src/js/integrations/debugsymbolicator';
import {
Expand All @@ -10,6 +17,7 @@ import {
symbolicateStackTrace,
} from '../../src/js/integrations/debugsymbolicatorutils';
import type * as ReactNative from '../../src/js/vendor/react-native';
import { getDefaultTestClientOptions, TestClient } from '../mocks/client';

async function processEvent(mockedEvent: Event, mockedHint: EventHint): Promise<Event | null> {
return debugSymbolicatorIntegration().processEvent!(mockedEvent, mockedHint, {} as Client);
Expand Down Expand Up @@ -844,5 +852,54 @@ describe('Debug Symbolicator Integration', () => {
},
});
});

it('should not capture console events when symbolicating the stack trace', async () => {
(symbolicateStackTrace as jest.Mock).mockImplementation(() => {
// eslint-disable-next-line no-console
console.assert(false, 'should not be logged as an event', '<object>');
return Promise.resolve({
stack: [],
} as ReactNative.SymbolicatedStackTrace);
});
Comment on lines +857 to +863
Copy link
Member

Choose a reason for hiding this comment

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

Let's also add a test where the assert is executed in the Promise. I believe that test would fail at the moment. Because only the code till the first await statement will be in the sandbox.

Copy link
Collaborator Author

@lucas-zimerman lucas-zimerman Apr 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

That surely breaks the test :c

We are a bit limited on what we could do here since we don't have the control on when the console log is going to happen.

I have a code proposal where we implement a delayed console sandbox, where we can control when to start logging console events after the callback was finished:

export function delayedConsoleSandBox<T>(callback: () => T, timer: number): T {
  if (!('console' in GLOBAL_OBJ)) {
    return callback();
  }
+++ let isPromise = false;
  const console = GLOBAL_OBJ.console as Console;
  const wrappedFuncs: Partial<LoggerConsoleMethods> = {};

  const wrappedLevels = Object.keys(originalConsoleMethods) as ConsoleLevel[];

+++  const delayedRestore = () => {
+++    setTimeout(() => {
+++      // Revert restoration to wrapped state
+++      wrappedLevels.forEach(level => {
+++        console[level] = wrappedFuncs[level] as LoggerMethod;
+++      });
+++      }, timer);
+++  };

  // Restore all wrapped console methods
  wrappedLevels.forEach(level => {
    const originalConsoleMethod = originalConsoleMethods[level] as LoggerMethod;
    wrappedFuncs[level] = console[level] as LoggerMethod | undefined;
    console[level] = originalConsoleMethod;
  });

  try {
    const result = callback();

+++    if (result instanceof Promise) {
+++      isPromise = true;
+++      result.finally(() => delayedRestore());
+++      return result;
+++    }
+++    else {
      return result;
+++    }
  } finally {
+++    if (!isPromise) {
      delayedRestore();
+++    }
  }
}

What do you think @krystofwoldrich @antonis ? It's just the normal code with some support for promises and also a timeout for restoring the sandbox after the promise/function finishes

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this would work, but I'm concerned about the fact, that all console log (not only the ones from the network call) will be in the sandbox until the network call (symbolication) resolves.

@lucas-zimerman @antonis Let me know what do you think, but to avoid a complex logic, since as @lucas-zimerman mention this is only for local builds, I would return to the original ignoring of the particular error causing issues.

Copy link
Member

Choose a reason for hiding this comment

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

One more options that I think would be feasible is to Proxy all method calls of the XHR object https://github.com/facebook/react-native/blob/f0527d86623112eee1e80da7884081f7ecf59bea/packages/react-native/Libraries/Network/XMLHttpRequest.js#L82

so all of the method are executed in consoleSandbox, since they are all synchronous it should work nicely, not sandboxing anything outside of the network call.

Copy link
Collaborator

Choose a reason for hiding this comment

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

One more options that I think would be feasible is to Proxy all method calls of the XHR object

That sounds like a good approach to me 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good to me, I will work on it


const client = new TestClient(getDefaultTestClientOptions());
setCurrentClient(client);
client.addIntegration(captureConsoleIntegration({ levels: ['assert'] }));
client.init();

const symbolicatedEvent = await processEvent(
{
threads: {
values: [
{
stacktrace: {
frames: mockSentryParsedFrames,
},
},
],
},
},
{
syntheticException: {
stack: mockRawStack,
framesToPop: 2,
} as unknown as Error,
},
);

// An event with the console log will be captured here if failed
expect(client.event).toBeUndefined();
expect(symbolicatedEvent).toStrictEqual(<Event>{
threads: {
values: [
{
stacktrace: {
frames: [],
},
},
],
},
});
});
});
});
2 changes: 2 additions & 0 deletions samples/react-native/src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import Animated, {

// Import the Sentry React Native SDK
import * as Sentry from '@sentry/react-native';
import * as SentryCore from '@sentry/core';
import { FeedbackWidget } from '@sentry/react-native';

import ErrorsScreen from './Screens/ErrorsScreen';
Expand Down Expand Up @@ -81,6 +82,7 @@ Sentry.init({
integrations(integrations) {
integrations.push(
reactNavigationIntegration,
SentryCore.captureConsoleIntegration(),
Sentry.reactNativeTracingIntegration({
// The time to wait in ms until the transaction will be finished, For testing, default is 1000 ms
idleTimeoutMs: 5_000,
Expand Down
21 changes: 21 additions & 0 deletions samples/react-native/src/Screens/ErrorsScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,27 @@ const ErrorsScreen = (_props: Props) => {
/>
) : null}
<View style={styles.mainViewBottomWhiteSpace} />
<Button
title="Log console assert"
onPress={() => {
// Sample code from https://gitlab.cin.ufpe.br/vrs2/iot-trafficlight-final/-/blob/main/node_modules/event-target-shim/dist/event-target-shim.mjs?ref_type=heads#L40
// This code is used by Metro and the assert is triggered when interacting with Debug Symbolicator integration.
// When using consoleintegration and filtering assert, it could trigger an infinite loop where debug symbolicator
// triggers the console integration, then it creates an event, that again calls debug symbolicator and the loop restart.
function pd(event: string) {
// const retv = privateData.get(event);
const retv = null;
console.assert(
retv != null,
"'this' is expected an Event object, but got",
event
);
return retv;
}
pd('<object>');
}}
/>
<View style={styles.mainViewBottomWhiteSpace} />
</ScrollView>
</>
);
Expand Down
Loading