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

Without dist explicitly set it shows on Android but not on iOS #280

Open
bruno-garcia opened this issue Aug 29, 2022 · 12 comments
Open

Without dist explicitly set it shows on Android but not on iOS #280

bruno-garcia opened this issue Aug 29, 2022 · 12 comments

Comments

@bruno-garcia
Copy link
Member

e.g:

<script> (function (w) { var i = (w.SENTRY_RELEASE = w.SENTRY_RELEASE || {}); i.id = ‘675adc71dca71be2c5e5’; })(this); </script>
@souredoutlook
Copy link
Member

souredoutlook commented Aug 31, 2022

Hey all, slight confusion here on my part.

Sorry @bruno-garcia for the misleading Slack message.

The script tag seems to be working as expected, but one thing I noticed was that the dist value on Android seems to get added to the events, even if it's not added to the Sentry.init() options. It is present in the config.xml, like so:

<widget android-versionCode="27696605" id="some_id" ios-CFBundleVersion="27696605" osx-CFBundleVersion="27696605" version="1.0.34" xmlns="http://www.w3.org/ns/widgets" xmlns:android="http://schemas.android.com/apk/res/android" xmlns:cdv="http://cordova.apache.org/ns/1.0">

This is only happening on Android, not iOs. Any advice for how to work around this?

@lucas-zimerman lucas-zimerman changed the title Without dist explicitly set it shows on Android but no on iOS Without dist explicitly set it shows on Android but not on iOS Aug 31, 2022
@lucas-zimerman
Copy link
Collaborator

Hey all, slight confusion here on my part.

Sorry @bruno-garcia for the misleading Slack message.

The script tag seems to be working as expected, but one thing I noticed was that the dist value on Android seems to get added to the events, even if it's not added to the Sentry.init() options. It is present in the config.xml, like so:

<widget android-versionCode="27696605" id="some_id" ios-CFBundleVersion="27696605" osx-CFBundleVersion="27696605" version="1.0.34" xmlns="http://www.w3.org/ns/widgets" xmlns:android="http://schemas.android.com/apk/res/android" xmlns:cdv="http://cordova.apache.org/ns/1.0">

This is only happening on Android, not iOs. Any advice for how to work around this?

Just to be clear, the issue here is the dist parameter being set on Android, despite it not being set by the user?

@souredoutlook
Copy link
Member

souredoutlook commented Aug 31, 2022

@lucas-zimerman yes that's the concern. The workaround is to manually remove it from events but this customer would rather it not get set at all if they don't declare it. To my knowledge, this is how sentry-react-native handles this but I might be mistaken.

@lucas-zimerman
Copy link
Collaborator

@lucas-zimerman yes that's the concern. The workaround is to manually remove it from events but this customer would rather it not get set at all if they don't declare it. To my knowledge, this is how sentry-react-native handles this but I might be mistaken.

By the way, do you have a sample that you are testing Cordova? I was going to check what's going on but I found another issue that is not allowing me to validate it on my sample app 😅

@souredoutlook
Copy link
Member

hi @lucas-zimerman

I don't have a sample app to test this, unfortunately.

I'm going to move forward with recommending they remove the field in beforeSend as a temporary workaround.

@lucas-zimerman
Copy link
Collaborator

@souredoutlook Ok so, I was able to reproduce it and we could do two things:

  1. remove the default Dist from the events on Android.
  2. when uploading the sourcemaps for Android, we could set the default dist for Android when uploading the sourcemaps if it weren't set by the user.

I believe the second option is more adequate since it'll not generate any break changes if users are using the default dist.
Any thoughts @bruno-garcia ?

@github-actions
Copy link

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@github-actions
Copy link

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@bruno-garcia
Copy link
Member Author

bruno-garcia commented Nov 18, 2022

I'll defer to @kahest as I dunno how to proceed

@kahest
Copy link
Member

kahest commented Nov 18, 2022

Thanks for investigating this @lucas-zimerman!
Generally I'd say go with option 2 to prevent breaking things, but that would mean changing this in sentry-cli, right?

@lucas-zimerman
Copy link
Collaborator

lucas-zimerman commented Nov 24, 2023

Implementing this solution getsentry/sentry-capacitor#480 will fix this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

5 participants