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

Exception capture context is overwritten by native scope sync #2146

Closed
Tracked by #4043 ...
lucas-zimerman opened this issue Mar 18, 2022 · 6 comments · Fixed by #3170 or #4124
Closed
Tracked by #4043 ...

Exception capture context is overwritten by native scope sync #2146

lucas-zimerman opened this issue Mar 18, 2022 · 6 comments · Fixed by #3170 or #4124

Comments

@lucas-zimerman
Copy link
Collaborator

Environment

How do you use Sentry?
Sentry SaaS (sentry.io)
Which SDK and version?
e.g: Sentry React Native 3.3.3

Steps to Reproduce

Sample code:

import { StatusBar } from 'expo-status-bar';
import React from 'react';
import { StyleSheet, Text, View } from 'react-native';
import * as Sentry from '@sentry/react-native';
Sentry.init({
  dsn:   'https://d870ad989e7046a8b9715a57f59b23b5@o447951.ingest.sentry.io/5428561',
  debug: true,
});

Sentry.captureException(new Error(`${Date.now()}: a test error occurred`),
context => context.addBreadcrumb({ message: 'error with breadcrumbs' }));

export default function App() {
  return (
    <View style={styles.container}>
      <Text>Open up App.js to start working on your app!</Text>
      <StatusBar style="auto" />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    backgroundColor: '#fff',
    alignItems: 'center',
    justifyContent: 'center',
  },
});

Expected Result

The exception captured with the breadcrumb error with breadcrumbs

Actual Result

Breadcrumb got removed
https://sentry.io/organizations/sentry-sdks/issues/3114951206/?project=5428561&query=is%3Aunresolved&statsPeriod=1h

if (event.exception?.values?.[0]?.mechanism?.handled) {
event.breadcrumbs = [];
}

This code doesnt differentiate from synced breadcrumbs and local breadcrumbs, resulting on local breadcrumbs getting removed when setting an isolated scope/context.

@marandaneto
Copy link
Contributor

I believe the right solution here would be to align the way how we capture events, which is what iOS does, on RN, we call the native bridge to get all device context and assemble the event on RN with already all the context, so this won't need to be done on Android, after the envelope is written to the disk, and because of that, breadcrumbs won't be duplicated.
This is a side-effect of this hacky solution of enriching the events on the Android side.
We can patch the current version to fix it, but ideally, we'd do it properly.

@marandaneto marandaneto moved this from Needs Discussion to Backlog in Mobile & Cross Platform SDK Mar 25, 2022
@lucas-zimerman
Copy link
Collaborator Author

I agree with this solution

@marandaneto
Copy link
Contributor

@krystofwoldrich have you tested that this is fixed after #3170?

@krystofwoldrich
Copy link
Member

@github-project-automation github-project-automation bot moved this from Done to Needs Discussion in Mobile & Cross Platform SDK Jul 26, 2023
@krystofwoldrich krystofwoldrich moved this from Needs Discussion to Backlog in Mobile & Cross Platform SDK Jul 26, 2023
@krystofwoldrich krystofwoldrich changed the title (Android) breadcrumb removed if set on error's context/scope. Exception capture context is overwritten by native scope sync Sep 27, 2023
@krystofwoldrich
Copy link
Member

Although we are not removing the breadcrumbs anymore, the captureContext is not working due to native scope sync.

Device context integration overwrites the captureContext with the context from the native layer. This works under the assumption that all scope changes are synced to the native layer. But this is not true for the captureContex which operates on a scope clone that is not (and should not be) synchronized to the native layer.

const nativeBreadcrumbs = Array.isArray(native['breadcrumbs'])
? native['breadcrumbs'].map(breadcrumbFromObject)
: undefined;
if (nativeBreadcrumbs) {
event.breadcrumbs = nativeBreadcrumbs;
}

@krystofwoldrich
Copy link
Member

The breadcrumb origin and fixing the breadcrumbs merging, which should not rely on using purely breadcrumbs from the native layer, will be needed also for supporting Expo DOM Components.

@antonis antonis self-assigned this Oct 9, 2024
@antonis antonis moved this from Backlog to In Progress in Mobile & Cross Platform SDK Oct 9, 2024
@antonis antonis moved this from In Progress to Needs Review in Mobile & Cross Platform SDK Oct 31, 2024
@github-project-automation github-project-automation bot moved this from Needs Review to Done in Mobile & Cross Platform SDK Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment