Symbolicate unhandled promise rejections #40914

ospfranco · 2023-10-13T05:40:15Z

Summary:

For a very long time when a promise rejects without an attached catch we get this warning screen without a correct stack trace, only some internal calls to the RN internals.

I created an issue for discussion in the react-native-community repo and we figured out it was only a matter of symbolication. While it cannot be done on release without external packages and source maps, at least while developing we can provide a symbolicated stack-trace so developers can better debug the source of rejected promise.

I got the stack trace symbolicated and the correct code frame. I'm missing some help trying to display it in the warning view but at the very least I can now correctly show the line of the error and log the codeframe to the console.

Changelog:

Test Plan:

I simply created a throwing function on a dummy app, and checked the output of the console and the warning view:

import React from 'react';
import {SafeAreaView, Text} from 'react-native';

async function throwme() {
  throw new Error('UNHANDLED');
}

function App(): JSX.Element {
  throwme();

  return (
    <SafeAreaView>
      <Text>Throw test</Text>
    </SafeAreaView>
  );
}

export default App;

Here is the output

Edit: I got the warning window working properly:

packages/react-native/Libraries/LogBox/Data/LogBoxData.js

packages/react-native/Libraries/promiseRejectionTrackingOptions.js

github-actions · 2023-10-13T07:11:30Z

	Warnings
⚠️	One hour and a half have passed and the E2E jobs haven't finished yet.

Generated by 🚫 dangerJS against e5eed45

a7medev · 2023-10-13T09:21:39Z

packages/react-native/Libraries/promiseRejectionTrackingOptions.js

+            message: {content: warning, substitutions: []},
+            isComponentError: false,
+            codeFrame: stack.codeFrame,
+            stack: error.stack,


Can't we just pass the symbolicated stack variable here instead of the original obfuscated error.stack?

Logbox already supports doing the symbolication, can we leverage the logic there?

Can you provide more context on how to do that? I can see that he already provided it with the error.stack but still the correct first frame wasn't shown

You may need to use LogBox.addException for this, and update parseLogBoxException to treat these as non-fatal

@ospfranco can you please give this a try?

Nope, doesn't seem to be working, didn't change the level yet, but stack parsing doesn't seem to work.

ospfranco · 2023-10-13T12:36:17Z

I got it working :)

JuanRdBO · 2023-10-13T13:17:30Z

Pls merge this now 💪

javache

Can you try getting rid of the symbolication code in promiseRejectionTrackingOptions? We should aim to reuse the existing one.

javache · 2023-10-13T13:49:12Z

packages/react-native/Libraries/LogBox/Data/LogBoxData.js

@@ -198,16 +201,17 @@ export function addLog(log: LogData): void {
  // otherwise spammy logs would pause rendering.
  setImmediate(() => {
    try {
-      const stack = parseErrorStack(errorForStackTrace?.stack);
+      const defaultStack = parseErrorStack(errorForStackTrace?.stack);


Try this to reuse the same parsing logic for both

Suggested change

const defaultStack = parseErrorStack(errorForStackTrace?.stack);

const stack = parseErrorStack(log.stack ?? errorForStackTrace.stack);

javache · 2023-10-13T13:50:25Z

packages/react-native/Libraries/promiseRejectionTrackingOptions.js

+        const parseErrorStack = require('./Core/Devtools/parseErrorStack');
+        const symbolicateStackTrace = require('./Core/Devtools/symbolicateStackTrace');
+        const LogBox = require('./LogBox/LogBox').default;
+
+        const parsedStack = parseErrorStack(error.stack);
+
+        symbolicateStackTrace(parsedStack)


This shouldn't live here. Logbox can handle this better by first showing the unsymbolicated stacktrace and then replacing the relevant log when the symbolication is ready.

ospfranco · 2023-10-13T14:22:12Z

Which of the existing ones? But now that I got it working I now better why it fails. It's more a question of architecture. Maybe add an extra case in AddException? But which logic should I follow to match the error?

javache · 2023-10-13T15:14:45Z

Which of the existing ones? But now that I got it working I now better why it fails. It's more a question of architecture. Maybe add an extra case in AddException? But which logic should I follow to match the error?

Logbox will auto-symbolicate the current trace given a valid stack property:

react-native/packages/react-native/Libraries/LogBox/UI/LogBoxInspector.js

Line 41 in edb7332

LogBoxData.symbolicateLogNow(log);

…ogbox symbolicate it

ospfranco · 2023-10-13T17:29:03Z

yeah, now I got your point @javache. Thanks a lot!

javache · 2023-10-16T08:38:29Z

packages/react-native/Libraries/promiseRejectionTrackingOptions.js

+        const parsedStack = parseErrorStack(error.stack);
+
+        LogBox.addLog({
+          level: 'warn',
+          message: {content: warning, substitutions: []},
+          componentStack: [],
+          stack: parsedStack,
+          category: 'possible_unhandled_promise_rejection',
+        });


Can we avoid parsing the error stack here, and leave that op to LogBox?

Consider whether addException may be better here, since it already supports passing in a stack?

I just tried but addException tackes an Array<StackFrame> on the stack property not a string. Tried modifying that but getting another error:

So it's a chicken and egg problem trying to use that. I can modify addLog to check for a string stack and parsing it there though, would that be a good compromise?

javache

Some final nits, but this seems good for merging in.

javache · 2023-10-16T12:27:56Z

packages/react-native/Libraries/promiseRejectionTrackingOptions.js

@@ -10,6 +10,8 @@

 import typeof {enable} from 'promise/setimmediate/rejection-tracking';

+const LogBox = require('./LogBox/LogBox').default;


Use import instead of require here

javache · 2023-10-16T12:31:34Z

packages/react-native/Libraries/promiseRejectionTrackingOptions.js

+      const warning =
+        `Possible Unhandled Promise Rejection (id: ${id}):\n` +
+        `${message ?? ''}\n` +
+        (stack == null ? '' : stack);
+
+      // Print pretty unhandled rejections while on DEV
+      if (__DEV__) {
+        LogBox.addLog({
+          level: 'warn',
+          message: {content: warning, substitutions: []},
+          componentStack: [],
+          stack: error.stack,
+          category: 'possible_unhandled_promise_rejection',
+        });
+
+        return;
+      } else {
+        console.warn(warning);
+      }


We never write to stack anymore, can you cleanup warning?

And I think this will log twice in !__DEV__ (although this module is only ever used if (__DEV__), so we could remove this check altogether really.

Done, reverted the double warning and just handled the special case while on dev

…info if possible (#42079) Summary: This is a continuation of my [last PR](#40914) which improved the symbolication of unhandled promise rejections. While I was developing another library I noticed I still got an error stack of the log adding and not of the error itself. The library I'm trying to debug does not throw a standard error object but rather a custom one, but it still contains the stack field. By passing this stack field to the logbox call I was able to get a better symbolicated stack trace. The exact line of the failure is not displayed but at least the correct file is. ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [GENERAL] [ADDED] - Unhandled promise rejection - attach non-standard Error object stack info if possible For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests Pull Request resolved: #42079 Test Plan: Test any unhandled promise rejection with a non-standard error (line 23, toString must not return `[object Error]`) and see if the correct (or at least a better) stack trace is shown. Here is the one I got before and after this change: <img src="https://github.com/facebook/react-native/assets/1634213/3d07faad-9535-42c9-8032-b4d8fe407e88" width="200" /> <img src="https://github.com/facebook/react-native/assets/1634213/2c39bd82-c7a1-4f58-8ac4-5c479bb96b6e" width="200" /> Reviewed By: huntie Differential Revision: D52431711 Pulled By: cipolleschi fbshipit-source-id: be2172d3b1e2fc3f72812faac372c83bc6dface2

…info if possible (facebook#42079) Summary: This is a continuation of my [last PR](facebook#40914) which improved the symbolication of unhandled promise rejections. While I was developing another library I noticed I still got an error stack of the log adding and not of the error itself. The library I'm trying to debug does not throw a standard error object but rather a custom one, but it still contains the stack field. By passing this stack field to the logbox call I was able to get a better symbolicated stack trace. The exact line of the failure is not displayed but at least the correct file is. ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [GENERAL] [ADDED] - Unhandled promise rejection - attach non-standard Error object stack info if possible For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests Pull Request resolved: facebook#42079 Test Plan: Test any unhandled promise rejection with a non-standard error (line 23, toString must not return `[object Error]`) and see if the correct (or at least a better) stack trace is shown. Here is the one I got before and after this change: <img src="https://github.com/facebook/react-native/assets/1634213/3d07faad-9535-42c9-8032-b4d8fe407e88" width="200" /> <img src="https://github.com/facebook/react-native/assets/1634213/2c39bd82-c7a1-4f58-8ac4-5c479bb96b6e" width="200" /> Reviewed By: huntie Differential Revision: D52431711 Pulled By: cipolleschi fbshipit-source-id: be2172d3b1e2fc3f72812faac372c83bc6dface2

…info if possible (#42079) Summary: This is a continuation of my [last PR](#40914) which improved the symbolication of unhandled promise rejections. While I was developing another library I noticed I still got an error stack of the log adding and not of the error itself. The library I'm trying to debug does not throw a standard error object but rather a custom one, but it still contains the stack field. By passing this stack field to the logbox call I was able to get a better symbolicated stack trace. The exact line of the failure is not displayed but at least the correct file is. ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [GENERAL] [ADDED] - Unhandled promise rejection - attach non-standard Error object stack info if possible For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests Pull Request resolved: #42079 Test Plan: Test any unhandled promise rejection with a non-standard error (line 23, toString must not return `[object Error]`) and see if the correct (or at least a better) stack trace is shown. Here is the one I got before and after this change: <img src="https://github.com/facebook/react-native/assets/1634213/3d07faad-9535-42c9-8032-b4d8fe407e88" width="200" /> <img src="https://github.com/facebook/react-native/assets/1634213/2c39bd82-c7a1-4f58-8ac4-5c479bb96b6e" width="200" /> Reviewed By: huntie Differential Revision: D52431711 Pulled By: cipolleschi fbshipit-source-id: be2172d3b1e2fc3f72812faac372c83bc6dface2

Apply stack Symbolication patch part 1 & 2 to 0.71-stable (#41377) (#42079) (original #40914)

facebook#41377) Summary: For a very long time when a promise rejects without an attached catch we get this warning screen without a correct stack trace, only some internal calls to the RN internals. <img src="https://github.com/facebook/react-native/assets/1634213/75aa7615-ee3e-4229-80d6-1744130de6e5" width="200" /> I created [an issue for discussion](react-native-community/discussions-and-proposals#718) in the react-native-community repo and we figured out it was only a matter of symbolication. While it cannot be done on release without external packages and source maps, at least while developing we can provide a symbolicated stack-trace so developers can better debug the source of rejected promise. I got the stack trace symbolicated and the correct code frame. I'm missing some help trying to display it in the warning view but at the very least I can now correctly show the line of the error and log the codeframe to the console. <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [GENERAL] [FIXED] - Show correct stack frame on unhandled promise rejections on development mode. For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests Pull Request resolved: facebook#40914 Test Plan: I simply created a throwing function on a dummy app, and checked the output of the console and the warning view: ```ts import React from 'react'; import {SafeAreaView, Text} from 'react-native'; async function throwme() { throw new Error('UNHANDLED'); } function App(): JSX.Element { throwme(); return ( <SafeAreaView> <Text>Throw test</Text> </SafeAreaView> ); } export default App; ``` Here is the output <img src="https://github.com/facebook/react-native/assets/1634213/2c100e4d-618e-4143-8d64-4095e8370f4f" width="200" /> Edit: I got the warning window working properly: <img src="https://github.com/facebook/react-native/assets/1634213/f02a2568-da3e-4daa-8132-e05cbe591737" width="200" /> Reviewed By: yungsters Differential Revision: D50324344 Pulled By: javache fbshipit-source-id: 66850312d444cf1ae5333b493222ae0868d47056 Co-authored-by: Oscar Franco <[email protected]>

…object stack info if possible (facebook#42079) Summary: This is a continuation of my [last PR](facebook#40914) which improved the symbolication of unhandled promise rejections. While I was developing another library I noticed I still got an error stack of the log adding and not of the error itself. The library I'm trying to debug does not throw a standard error object but rather a custom one, but it still contains the stack field. By passing this stack field to the logbox call I was able to get a better symbolicated stack trace. The exact line of the failure is not displayed but at least the correct file is. <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [GENERAL] [ADDED] - Unhandled promise rejection - attach non-standard Error object stack info if possible For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests Pull Request resolved: facebook#42079 Test Plan: Test any unhandled promise rejection with a non-standard error (line 23, toString must not return `[object Error]`) and see if the correct (or at least a better) stack trace is shown. Here is the one I got before and after this change: <img src="https://github.com/facebook/react-native/assets/1634213/3d07faad-9535-42c9-8032-b4d8fe407e88" width="200" /> <img src="https://github.com/facebook/react-native/assets/1634213/2c39bd82-c7a1-4f58-8ac4-5c479bb96b6e" width="200" /> Reviewed By: huntie Differential Revision: D52431711 Pulled By: cipolleschi fbshipit-source-id: be2172d3b1e2fc3f72812faac372c83bc6dface2

Symbolicate unhandled promise rejections and try to log them

8d1ce80

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 13, 2023

ospfranco commented Oct 13, 2023

View reviewed changes

packages/react-native/Libraries/LogBox/Data/LogBoxData.js Outdated Show resolved Hide resolved

ospfranco commented Oct 13, 2023

View reviewed changes

packages/react-native/Libraries/promiseRejectionTrackingOptions.js Outdated Show resolved Hide resolved

facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Oct 13, 2023

ospfranco added 3 commits October 13, 2023 07:53

Linting

cb897d0

change paths for devtools

c488217

change paths for devtools

516deee

a7medev reviewed Oct 13, 2023

View reviewed changes

ospfranco added 2 commits October 13, 2023 14:13

Fix flow errors

ff6be9c

Allow passing stack into addLog

c460d5f

ospfranco requested a review from javache October 13, 2023 12:47

javache requested changes Oct 13, 2023

View reviewed changes

javache requested a review from motiz88 October 13, 2023 13:53

ospfranco added 2 commits October 13, 2023 19:26

Do not symbolicate stack on promiseRejectionTrackingOptions but let l…

a54fd6b

…ogbox symbolicate it

remove unused codeFrame

2a8a491

ospfranco requested a review from javache October 13, 2023 17:29

Revert outdated change to rejectionTrackingOptions type

352ab9f

javache reviewed Oct 16, 2023

View reviewed changes

ospfranco added 2 commits October 16, 2023 11:30

Parse error stack on LogBoxData

07a48c2

re-insert blank line

1443452

ospfranco requested a review from javache October 16, 2023 09:38

javache approved these changes Oct 16, 2023

View reviewed changes

PR comments

e5eed45

ospfranco mentioned this pull request Dec 27, 2023

Unhandled promise rejection - attach non-standard Error object stack info if possible #42079

Closed

cipolleschi mentioned this pull request Dec 28, 2023

[RN][CI] Fix e2e test setup #42087

Merged

cipolleschi mentioned this pull request Jan 4, 2024

[RN][iOS] Disable Bitcode for Hermes #42142

Merged

joe-sam mentioned this pull request Jan 7, 2024

Apply stack Symbolication patch part 1 & 2 to 0.71-stable (#41377) (#42079) (original #40914) #42163

Closed

This was referenced Jan 8, 2024

[Local][RN][General] 0.72 - migrate boost away from JFrog #42175

Merged

[0.73][RN][General] Migrate boost download url away from JFrog #42176

Merged

joe-sam mentioned this pull request Jan 10, 2024

Apply stack Symbolication patch part 1 & 2 to 0.71-stable (#41377) (#42079) (original #40914) #42222

Merged

This was referenced Jan 10, 2024

[RN][iOS] Fix symbol not found _jump_fcontext with use_frameworks! #42227

Merged

[RN][iOS] Fix symbol not found _jump_fcontext with use_frameworks! #42228

Merged

[RN][iOS] Fix min ios version regression and update podfile.lock #42232

Closed

hurali97 added a commit that referenced this pull request Jan 15, 2024

Merge pull request #42222 from joe-sam/patch-symbolication_0.71

568031e

Apply stack Symbolication patch part 1 & 2 to 0.71-stable (#41377) (#42079) (original #40914)

cipolleschi mentioned this pull request Jan 19, 2024

[RN][iOS] Fix race condition between A11yManager and UIManager #42382

Merged

cipolleschi mentioned this pull request Jan 30, 2024

[RN][iOS] Fix warning when loading RCTUIManager and A11yManager #42734

Merged

cipolleschi mentioned this pull request Feb 19, 2024

[RN][iOS] Fix compiler flags passed to libraries #43088

Merged

cipolleschi mentioned this pull request Apr 5, 2024

[Yoga][0.73] Fix archive for MacOS Catalyst #43896

Merged

This was referenced Jun 28, 2024

[CI] Move machines to M1 for 0.73 #45214

Merged

[CI] Move machines to M1 for 0.72 #45215

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Symbolicate unhandled promise rejections #40914

Symbolicate unhandled promise rejections #40914

ospfranco commented Oct 13, 2023 •

edited

Loading

github-actions bot commented Oct 13, 2023 •

edited

Loading

a7medev Oct 13, 2023

javache Oct 13, 2023

a7medev Oct 13, 2023

javache Oct 13, 2023

a7medev Oct 13, 2023

ospfranco Oct 13, 2023 •

edited

Loading

ospfranco commented Oct 13, 2023 •

edited

Loading

JuanRdBO commented Oct 13, 2023

javache left a comment

javache Oct 13, 2023

javache Oct 13, 2023

ospfranco commented Oct 13, 2023 •

edited

Loading

javache commented Oct 13, 2023

ospfranco commented Oct 13, 2023

javache Oct 16, 2023

ospfranco Oct 16, 2023

javache left a comment

javache Oct 16, 2023

javache Oct 16, 2023

ospfranco Oct 16, 2023

	const defaultStack = parseErrorStack(errorForStackTrace?.stack);
	const stack = parseErrorStack(log.stack ?? errorForStackTrace.stack);

		@@ -10,6 +10,8 @@

		import typeof {enable} from 'promise/setimmediate/rejection-tracking';

		const LogBox = require('./LogBox/LogBox').default;

Symbolicate unhandled promise rejections #40914

Symbolicate unhandled promise rejections #40914

Conversation

ospfranco commented Oct 13, 2023 • edited Loading

Summary:

Changelog:

Test Plan:

github-actions bot commented Oct 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ospfranco Oct 13, 2023 • edited Loading

Choose a reason for hiding this comment

ospfranco commented Oct 13, 2023 • edited Loading

JuanRdBO commented Oct 13, 2023

javache left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ospfranco commented Oct 13, 2023 • edited Loading

javache commented Oct 13, 2023

ospfranco commented Oct 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javache left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ospfranco commented Oct 13, 2023 •

edited

Loading

github-actions bot commented Oct 13, 2023 •

edited

Loading

ospfranco Oct 13, 2023 •

edited

Loading

ospfranco commented Oct 13, 2023 •

edited

Loading

ospfranco commented Oct 13, 2023 •

edited

Loading