[DebugSymbolicator] Improve source context resolution to avoid extra 404 http requests

getsentry / sentry-react-native

Official Sentry SDK for React Native

https://sentry.io

MIT License

1.58k stars 337 forks source link

[DebugSymbolicator] Improve source context resolution to avoid extra 404 http requests #1887

Open nabati opened 2 years ago

nabati commented 2 years ago

Environment

iOS Simulator Sentry SaaS.

react-native-sdk 3.2.3

Steps to Reproduce

For some reason, enabling sentry while enabling network inspect in react native debugger, I get a lot of false positives in the list of requested network resources. It seems Sentry initiates fetching the source for some files (?). When I disable Sentry, the requests and associated errors disappear.

It seems like Sentry is using trial and error to find the source code, and thus creating a lot of false positives in the logs?

Note that it also is successful in finding context.ts at some point.

Tested urls Failed http://localhost:8081//Users/xxxx/Projects/mevisio-native-app/src/context.ts http://localhost:8081/context.ts

Successful http://localhost:8081/src/context.ts

Expected Result

To not see these failed network requests when checking the network requests in react native debugger.

marandaneto commented 2 years ago

@nabati thanks for reporting, is it debug or release mode? if debug, can you try release mode?

@jennmueng could it be related to https://github.com/getsentry/sentry-react-native/issues/1346 ?

jennmueng commented 2 years ago

@marandaneto Yep, looks related to #1346. Looks like it's time we revisit the debug symbolicator integration.

skjolber commented 8 months ago

Could this be an issue of port forwarding, i.e.

adb reverse tcp:8080 tcp:8080

gives

Error: Unable to resolve module ./http://localhost:8081/index from /home/thomas/entur/git/abt-mobile-client-sdk/examples/client/.: 

None of these files exist:
  * http:/localhost:8081/index(.native|.android.js|.native.js|.js|.android.jsx|.native.jsx|.jsx|.android.json|.native.json|.json|.android.ts|.native.ts|.ts|.android.tsx|.native.tsx|.tsx)
  * ...

however http://localhost:8081/index.js does exist on my local developer machine. So this would indicate that the localhost in the above message is the Android device itself.

Workaround, add to sentry configuration:

integrations: function (integrations: any) { // see https://github.com/getsentry/sentry-react-native/issues/1346
    // filter default integrations
    return integrations.filter(function (integration:any) {
      return integration.name !== "DebugSymbolicator";
    });
  }

krystofwoldrich commented 8 months ago

Hi @skjolber, thank you for the message, the failing requests are caused by the DebugSymbolicator as mentioned above. By fetching the source context

https://github.com/getsentry/sentry-react-native/blob/395a14b289fce9cdff1e67433fccd855fc6e5632/src/js/integrations/debugsymbolicator.ts#L182-L191

krystofwoldrich commented 2 months ago

The extra 404 http requests could be avoided by implementing

https://metrobundler.dev/docs/configuration/#enhancemiddleware-deprecated

which exposes new sentry specific endpoint for source code context resolution.

krystofwoldrich commented 2 months ago

The extra requests also cause unnecessary builds of the Expo/React Native Web bundles if used in the application

https://github.com/getsentry/sentry-react-native/issues/4054

krystofwoldrich commented 1 month ago

The extra requests also trigger unintentional Web builds which can cause confusion (especially when the mobile build works, but web errors out)

example https://github.com/getsentry/sentry-react-native/issues/4134