expo / sentry-expo

MIT License
202 stars 83 forks source link

Using EAS update, there's no `revisionId` in releases #297

Closed SimenB closed 1 year ago

SimenB commented 1 year ago

Summary

When using classix updates (expo publish), a revision ID is used as the release in Sentry, meaning one can track issues between JS updates, and mark them as resolved.

This field doesn't exist with EAS Updates.

https://github.com/expo/sentry-expo/blob/b79ac904372e5593cb6976f9d7ccb313bf9b3f54/src/sentry.ts#L20-L41


In my own project, I've migrated to expo-update's updateId, like so: `${nativeApplicationVersion}-${Updates.updateId}`. This is sorta fine except that this ID is different between ios and android - ideally we'd have access to the update group ID.

There's even a comment about this is the code I linked: // Want to make sure this still exists in EAS update: equal on iOS & Android

Managed or bare workflow? If you have ios/ or android/ directories in your project, the answer is bare!

managed

What platform(s) does this occur on?

Android, iOS

SDK Version (managed workflow only)

46

Environment

  expo-env-info 1.0.4 environment info:
    System:
      OS: macOS 13.0
      Shell: 5.8.1 - /bin/zsh
    Binaries:
      Node: 16.18.0 - ~/.nvm/versions/node/v16.18.0/bin/node
      Yarn: 3.2.4 - /opt/homebrew/bin/yarn
      npm: 8.19.2 - ~/.nvm/versions/node/v16.18.0/bin/npm
    Managers:
      CocoaPods: 1.11.3 - /opt/homebrew/bin/pod
    SDKs:
      iOS SDK:
        Platforms: DriverKit 22.1, iOS 16.1, macOS 13.0, tvOS 16.1, watchOS 9.1
    IDEs:
      Android Studio: Dolphin 2021.3.1 Patch 1 Dolphin 2021.3.1 Patch 1
      Xcode: 14.1/14B47b - /usr/bin/xcodebuild
    npmGlobalPackages:
      eas-cli: 2.6.0
      expo-cli: 6.0.8
    Expo Workflow: managed

Reproducible demo or steps to reproduce from a blank project

Just use expo-sentry with EAS Update and see no new release is shown in Sentry after making an update.

kbrandwijk commented 1 year ago

That is correct. We are currently working on the sentry-expo release that adds official support for EAS Update, which includes using updateId as dist value in Sentry (keeping release matching the actual bundle-version-buildnumber like it is on 'normal' builds).

kbrandwijk commented 1 year ago

To elaborate, based on your comments on the docs PR, Sentry.init allows you to pass in release and dist. If you don't, it will use the default values from sentry-expo, which will be bundle-version-buildnumber for release and updateId (if an update) or buildnumber (for regular builds) for dist.

So you can tweak this any way you want, as long as you pass in release and dist yourself in Sentry.init in your app.

SimenB commented 1 year ago

Thanks!

Reading through https://docs.sentry.io/product/releases/, I think release is better for the update ID. If I e.g. want to mark an issue as fixed I don't have the option to choose the dist, only release (or nothing at all).

image

Wouldn't it make more sense to have the JS bundle then be a "release"?

I might very well be wrong tho, far from a Sentry expert 🙂

github-actions[bot] commented 1 year ago

This issue is stale because it has been open for 60 days with no activity. If there is no activity in the next 7 days, the issue will be closed.

SimenB commented 1 year ago

Sort of addressed in #302 I guess?

adamandreasson commented 1 year ago

I agree with @SimenB. We can't get sourcemaps to work properly using dists, so we have configured release name to be

`${Constants.expoConfig.version}-${Updates.updateId}`

Our Sentry now functions essentially the same way it used to with expo-updates. What is the reasoning for using dists? We think it makes sense that the js bundle should map to the release in Sentry, not the binary build.

SimenB commented 1 year ago

Agreed. When I press "resolve in next release" in sentry, I have to make an entirely new build for it to work. Seems counter intuitive.

Would also still like a matching id on all platforms.

matheuscouto commented 1 year ago

I agree with @SimenB. Still can't find a way to pull the group id instead of the platform-specific id within the client, and I'd love to do either that or have some config on expo sentry to use either the platform-specific or the group id automatically.

kbrandwijk commented 1 year ago

Update group is available in the Updates.manifest.metadata.updateGroup field

matheuscouto commented 1 year ago

Gotcha! Thanks @kbrandwijk for that info, do you think it's safe to use it as dist? Asking because manifest.metadata does not have defined types

kbrandwijk commented 1 year ago

Gotcha! Thanks @kbrandwijk for that info, do you think it's safe to use it as dist? Asking because manifest.metadata does not have defined types

Yeah that field is safe to use.

SimenB commented 1 year ago

Any particular reason why it's not the default dist?

kbrandwijk commented 1 year ago

Any particular reason why it's not the default dist?

Not really, most of the people we talked to about this had separate releases for iOS and Android already in Sentry, so looking for a different field didn't seem necessary. We can reconsider, but a bigger problem is that updates are applied to builds based on runtimeVersion, so that means that a single update can actually apply to multiple different builds. For that reason, we might want to rethink how we identify builds and updates.