pocket-casts-android icon indicating copy to clipboard operation
pocket-casts-android copied to clipboard

build: enable source context upload to Sentry

Open wzieba opened this issue 11 months ago • 1 comments

Description

Fixes #

Testing Instructions

Screenshots or Screencast

Checklist

  • [x] If this is a user-facing change, I have added an entry in CHANGELOG.md
  • [x] Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
  • [x] I have considered whether it makes sense to add tests for my changes
  • [x] All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
  • [x] Any jetpack compose components I added or changed are covered by compose previews

I have tested any UI changes...

  • [ ] with different themes
  • [ ] with a landscape orientation
  • [ ] with the device set to have a large display and font size
  • [ ] for accessibility with TalkBack

wzieba avatar Mar 20 '24 12:03 wzieba

Thanks @ParaskP7 for the reviews! Let me give you more context

I got curious and was trying to find a release issue on Sentry that would give me SourceFile instead of the correct name of the file.

Because Sentry pushed the change to SaaS, you won't see SourceFile anymore - new and old events are now deobfuscated correctly on Sentry.

wzieba avatar Jul 03 '24 05:07 wzieba

Because Sentry pushed the change to SaaS, you won't see SourceFile anymore - new and old events are now deobfuscated correctly on Sentry.

Thanks for the additional context here @wzieba , this is exactly what I was thinking as well, but then, my more targeted question is, why do we need to set the includeSourceContext then, that is, if it is anyway working already as expected, that's what I don't understand? 🤔

ParaskP7 avatar Jul 03 '24 08:07 ParaskP7

if it is anyway working already as expected, that's what I don't understand? 🤔

"it" here means "deobfuscation". Deobfuscation is now working correctly on Sentry and this is the result of Sentry enabling our contribution. This PR is not fixing deobfuscation.

why do we need to set the includeSourceContext then

Because deobfuscation is now fixed, we're unblocked with sending source code to Sentry by setting includeSourceContext option.

I think examples can help here:

Stack trace with not correctly working deobfuscation and without source context

image

Stack trace with correctly working deobfuscation and without source context (before this PR)

image

Stack trace with correctly working deobfuscation and with source context (after this PR)

image

wzieba avatar Jul 03 '24 20:07 wzieba

Oh cool @wzieba , this helps a lot, thanks for providing examples to explain that to me, much appreciated! 🙇

ParaskP7 avatar Jul 04 '24 07:07 ParaskP7