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

Allow custom release and dist for app variants #776

Closed
4 of 11 tasks
kuhnroyal opened this issue Mar 1, 2022 · 11 comments
Closed
4 of 11 tasks

Allow custom release and dist for app variants #776

kuhnroyal opened this issue Mar 1, 2022 · 11 comments
Assignees

Comments

@kuhnroyal
Copy link
Contributor

Platform:

  • Dart
  • Flutter Android or iOS
  • Flutter Web

IDE:

  • VSCode
  • IntelliJ/AS
  • XCode
  • Other, which one?

split-debug-info and obfuscate (Flutter Android or iOS) or CanvasKit (Flutter Web):

  • Enabled
  • Disabled

Platform installed with:

  • pub.dev
  • GitHub

The version of the SDK (See pubspec.lock):
6.4.0-beta.1


I have the following issue:

When configuring the packageLoader parameter, the LoadReleaseIntegration is executed too late, after the native SDK integration. This results in different versions/release/dist being used on native and Flutter side. Ideally version/release/dist need to be send to the native SDK when it is being initialized.

Steps to reproduce:

  • Configure SentryFlutter.init(packageLoader: ...) with a custom release

Actual result:

  • Have two different releases appear in Sentry (one from native and one from Flutter)

Expected result:

  • They should be the same version
@kuhnroyal
Copy link
Contributor Author

Workaround:

    /// Workaround for https://github.com/getsentry/sentry-dart/issues/776
  options
	..removeIntegration(options.integrations.firstWhere((i) => i is LoadReleaseIntegration))
    ..addIntegrationByIndex(
      3,
      LoadReleaseIntegration(() async {
        final info = await PackageInfo.fromPlatform();
        return PackageInfo(
          appName: info.appName,
          packageName: _package,
          version: info.version,
          buildNumber: info.buildNumber,
          buildSignature: info.buildSignature,
        );
      }),
    )

@marandaneto
Copy link
Contributor

Thanks for reporting @kuhnroyal , we'll look into it.

@marandaneto marandaneto moved this from Needs Discussion to Needs Investigation in Mobile & Cross Platform SDK Mar 3, 2022
@marandaneto
Copy link
Contributor

@kuhnroyal I'm trying to understand your setup, mind sharing your SDK init code?

packageLoader is internal, it's only to make it testable, you should not change that.
If you want a custom release, use options.release, options.dist.

@marandaneto marandaneto moved this from Needs Investigation to In Progress in Mobile & Cross Platform SDK Mar 4, 2022
@kuhnroyal
Copy link
Contributor Author

packageLoader is internal, it's only to make it testable, you should not change that.
If you want a custom release, use options.release, options.dist.

If I want to set a custom release via options.release, options.dist then I would have to run the PackageInfo plugin outside of the Sentry error handling first to get those.
Thee background is that I have multiple app flavors with different package names/ids but I need them to be reported as the same release (package).

@marandaneto
Copy link
Contributor

@kuhnroyal ok, sounds like a fair use case.

The problem is that, right now, LoadReleaseIntegration calls the MethodChannel and reads the data from PackageInfo, or your own custom packageLoader.

Reading it before initing the Native SDKs -> NativeSdkIntegration means that you're calling the MethodChannel before the Native SDKs are initialized, which means if the MethodChannel throws for some reason, the Native SDKs won't detect it.

I'm not sure that initing it afterward would help.
Sounds like the best deal here is, using a custom runZonedGuarded to init. the Flutter SDK, and read your release from the MethodChannel yourself, set them to options.release and options.dist, at least if it throws, you are able to take care of it.

What do you think? it's a trade-off, but your workaround would cause another problem, as stated, if the packageLoader throws, the Native SDKs won't detect it.

@kuhnroyal
Copy link
Contributor Author

Yea both approaches have the same problem.

Is it possible to update the native SDK (release, dist) after it has been initialized?

@marandaneto
Copy link
Contributor

Currently not, Maybe after #265
So you can initialize the Native SDKs with your own custom release/dist.

I'm not sure if I should close this issue or not, since the root of the problem comes by using the packageLoader which is "internal", also because you have a special setup, I believe most apps wouldn't need it, will keep it open, so people can learn from the discussion.

@marandaneto marandaneto moved this from In Progress to Backlog in Mobile & Cross Platform SDK Mar 4, 2022
@kuhnroyal
Copy link
Contributor Author

Just an FYI: The parameter is not marked as internal and for testing purposes you could use the PackageInfo.setMockInitialValues instead

@marandaneto
Copy link
Contributor

Just an FYI: The parameter is not marked as internal and for testing purposes you could use the PackageInfo.setMockInitialValues instead

You're totally right, I've marked them as internal #779

PackageInfo.setMockInitialValues TIL, We'll remove it then in the next major bump most probably.

@marandaneto marandaneto moved this from Backlog to Needs Discussion in Mobile & Cross Platform SDK May 19, 2022
@philipphofmann philipphofmann changed the title LoadReleaseIntegration called too late Allow custom release and dist for app variants May 19, 2022
@philipphofmann
Copy link
Member

There is a workaround by initializing the native SDK first and setting the release yourself, see getsentry/sentry-docs#5029. We can close this after releasing 6.6.0.

@philipphofmann philipphofmann moved this from Needs Discussion to In Progress in Mobile & Cross Platform SDK May 19, 2022
@marandaneto
Copy link
Contributor

Repository owner moved this from In Progress to Done in Mobile & Cross Platform SDK Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

3 participants