Skip to content
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.

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

fix: stop assuming that error.stack is defined #296

Merged
merged 3 commits into from
Nov 23, 2022

Conversation

lachenmayer
Copy link
Contributor

Checklist

Why

Hi there, I am experiencing an issue in our app where instead of an expected exception being logged in the redbox in development, the following error is logged instead:

Possible Unhandled Promise Rejection (id: 0):
TypeError: undefined is not an object (evaluating 'error.stack.replace')

How

The stack trace in the redbox lead me to the function in question. By adding a console.error log in the file node_modules/sentry-expo/build/integrations/bare.js I verified that an exception was thrown by my app which has an undefined error.stack property.

This happened to be an AssertionError, thrown by assert.equal from assert. I'm sure it's possible to reproduce this in another project using this code:

import assert from 'assert'

assert.equal(1, 2, 'This error should show up in the logs and redbox.')

As this is a straight port from Node.js's assert module, I'm sure the error exported from this is not 100% compatible with the 'native' Error. We ideally don't want to remove assert (which is used in a lot of places, including shared Node.js/React Native code), so (in my opinion) the best solution is just to fix this problem here.

From a type perspective, error is any, so we shouldn't assume that error.stack exists or is a string. (I can throw { whatever: true }.)

Test Plan

I am sorry but I don't have the time to create a minimal reproducible example. I tested this change by manually applying this patch using patch-package: https://gist.github.com/lachenmayer/e06438d58a5948bf8149bb4268e4c0dd

Before

This error is logged in the console & shown in the redbox:

Possible Unhandled Promise Rejection (id: 0):
TypeError: undefined is not an object (evaluating 'error.stack.replace')

After - expected behavior

Error logged after - the exact value is irrelevant, it's the error that was actually thrown by our app:

AssertionError: Section height calculations are incorrect. topOffset: 184 nextOffset: undefined
at node_modules/@sentry/utils/cjs/instrument.js:109:10 in <anonymous>
at node_modules/react-native/Libraries/Core/ExceptionsManager.js:95:4 in reportException
at node_modules/react-native/Libraries/Core/ExceptionsManager.js:141:19 in handleException
at node_modules/react-native/Libraries/Core/setUpErrorHandling.js:24:6 in handleError
at node_modules/sentry-expo/build/integrations/bare.js:103:30 in ErrorUtils.setGlobalHandler$argument_0
at node_modules/expo-error-recovery/build/ErrorRecovery.fx.js:12:21 in ErrorUtils.setGlobalHandler$argument_0
at node_modules/expo-error-recovery/build/ErrorRecovery.fx.js:8:4 in <global>
at node_modules/@react-native/polyfills/error-guard.js:49:36 in ErrorUtils.reportFatalError

@kbrandwijk
Copy link
Contributor

Thank you for your contribution!

@kbrandwijk kbrandwijk merged commit 85e519e into expo:main Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants