search

expo / sentry-expo

MIT License
202 stars 83 forks source link

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

Closed lachenmayer closed 1 year ago

lachenmayer commented 1 year ago

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 commented 1 year ago

Thank you for your contribution!