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

Avoid duplicating Breadcrumbs on Android #254

Merged
merged 2 commits into from
Nov 16, 2022

Conversation

lucas-zimerman
Copy link
Collaborator

Unhandled Exceptions doesn't crash the app, unlike React Native, so we shouldn't validate the Handled flag for clearing the breadcrumbs.

For context: Both SDKs have the same Breadcrumbs, we clear them on the Android Wrapper because the Android SDK will apply the same breadcrumbs to the captured event.

Fixes #232

@lucas-zimerman lucas-zimerman changed the title Ignore breadcrumbs if unhandled on Android Avoid duplicating Breadcrumbs on Android Nov 6, 2022
@lucas-zimerman lucas-zimerman marked this pull request as ready for review November 6, 2022 14:02
@marandaneto
Copy link
Contributor

Pay attention at getsentry/team-mobile#11 (comment)
I guess getsentry/team-mobile#11 should be implemented in order to fix this correctly otherwise those checks based on the handled flag also have side effects.

@lucas-zimerman
Copy link
Collaborator Author

Pay attention at getsentry/team-mobile#11 (comment) I guess getsentry/team-mobile#11 should be implemented in order to fix this correctly otherwise those checks based on the handled flag also have side effects.

I have tried to use the SentryEnvelope to read the byte array generated by the SDK.

EnvelopeReader reader = new EnvelopeReader(options.getSerializer());
InputStream targetStream = new ByteArrayInputStream(envelope.getBytes(Charset.forName("UTF-8")));
SentryEnvelope javaEnvelope = reader.read(targetStream);
hub.captureEnvelope(javaEnvelope);

The envelope was captured but now the envelope is different from the previously ones, for example:

  • tags: missing isSideLoaded, os.rooted.
  • Missing App Build, ID, Name, Start Time, Version, Permissions
  • Missing user.id
  • OS: Different fields

https://sentry.io/organizations/sentry-sdks/issues/3268302122/events/507506e2637e4bd69616965ffd5cd079/?project=5522756

https://sentry.io/organizations/sentry-sdks/issues/3268302122/events/28d12291411b40738d4ad475b846207f/?project=5522756#context-app

@marandaneto
Copy link
Contributor

marandaneto commented Nov 11, 2022

@lucas-zimerman the event is enriched with device context after its being written in the disk (by a file observer), that's how it works on Android and it should not, that's the reason about getsentry/team-mobile#11

Copy link
Member

@kahest kahest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM!

One thing to note - we should add a comment to getsentry/team-mobile#11 that the changes of this PR need to be revisited/reverted once getsentry/team-mobile#11 is done.

@lucas-zimerman lucas-zimerman merged commit b83bd0f into main Nov 16, 2022
@lucas-zimerman lucas-zimerman deleted the fix/duplidated-breadcrumbs branch November 16, 2022 12:52
@expcapitaldev
Copy link

hi,guys, I am using sentry-cordova and have sometimes double breadcrubms, can we do something with that ? I do not see similar fix for sentry-cordova project

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.

Duplicate Breadcrumbs on Android
4 participants