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

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

Closed
SimenB opened this issue Nov 7, 2022 · 13 comments
Closed

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

SimenB opened this issue Nov 7, 2022 · 13 comments

Comments

@SimenB
Copy link
Contributor

SimenB commented Nov 7, 2022

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.

const DEFAULT_OPTIONS = {
enableNativeNagger: false, // Otherwise this will trigger an Alert(), let's rely on the logs instead
release: getDefaultRelease(),
dist: MANIFEST.revisionId ? MANIFEST.version : `${Application.nativeBuildVersion}`,
...(IS_BARE_WORKFLOW ? {} : { enableNative: false, enableNativeCrashHandling: false }),
};
/**
* We assign the appropriate release based on if the app is running in development,
* on an OTA Update, or on a no-publish build.
*/
function getDefaultRelease(): string {
if (__DEV__) {
return 'DEVELOPMENT';
} else if (MANIFEST.revisionId) {
// Want to make sure this still exists in EAS update: equal on iOS & Android
return MANIFEST.revisionId;
} else {
// This is the default set by Sentry's native Xcode & Gradle scripts
return `${Application.applicationId}@${Application.nativeApplicationVersion}+${Application.nativeBuildVersion}`;
}
}


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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

SimenB commented Nov 8, 2022

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
Copy link

github-actions bot commented Jan 7, 2023

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.

@github-actions github-actions bot added the stale label Jan 7, 2023
@SimenB
Copy link
Contributor Author

SimenB commented Jan 7, 2023

Sort of addressed in #302 I guess?

@github-actions github-actions bot removed the stale label Jan 7, 2023
@adamandreasson
Copy link

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
Copy link
Contributor Author

SimenB commented Jan 18, 2023

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
Copy link

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
Copy link
Contributor

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

@matheuscouto
Copy link

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
Copy link
Contributor

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
Copy link
Contributor Author

SimenB commented Jun 16, 2023

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

@kbrandwijk
Copy link
Contributor

kbrandwijk commented Jun 16, 2023

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.

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

No branches or pull requests

4 participants