search

getsentry / sentry-dart

Sentry SDK for Dart and Flutter
https://sentry.io/for/flutter/
MIT License
746 stars 233 forks source link

`RunZonedGuardedIntegration` is not awaiting result #730

Closed blaugold closed 2 years ago

blaugold commented 2 years ago

Platform:

IDE:

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

Platform installed with:

Output of the command flutter doctor -v below:

[✓] Flutter (Channel stable, 2.8.1, on macOS 12.1 21C52 darwin-arm, locale en-DE) • Flutter version 2.8.1 at /Users/gabriel/fvm/versions/stable • Upstream repository https://github.com/flutter/flutter.git • Framework revision 77d935af4d (7 weeks ago), 2021-12-16 08:37:33 -0800 • Engine revision 890a5fca2e • Dart version 2.15.1

[✓] Android toolchain - develop for Android devices (Android SDK version 32.0.0) • Android SDK at /Users/gabriel/Library/Android/sdk • Platform android-32, build-tools 32.0.0 • Java binary at: /Users/gabriel/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/203.7935034/Android Studio.app/Contents/jre/Contents/Home/bin/java • Java version OpenJDK Runtime Environment (build 11.0.10+0-b96-7249189) • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 13.2.1) • Xcode at /Applications/Xcode.app/Contents/Developer • CocoaPods version 1.11.2

[✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2020.3) • Android Studio at /Users/gabriel/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/203.7935034/Android Studio.app/Contents • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart • Java version OpenJDK Runtime Environment (build 11.0.10+0-b96-7249189)

[✓] IntelliJ IDEA Community Edition (version 2021.3.1) • IntelliJ at /Users/gabriel/Library/Application Support/JetBrains/Toolbox/apps/IDEA-C/ch-0/213.6461.79/IntelliJ IDEA CE.app • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart

[✓] IntelliJ IDEA Community Edition (version 2021.3) • IntelliJ at /Users/gabriel/Library/Application Support/JetBrains/Toolbox/apps/IDEA-C/ch-0/213.5744.223/IntelliJ IDEA CE.app • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart

[✓] VS Code (version 1.63.2) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.32.0

[✓] Connected device (2 available) • macOS (desktop) • macos • darwin-arm64 • macOS 12.1 21C52 darwin-arm • Chrome (web) • chrome • web-javascript • Google Chrome 97.0.4692.99

• No issues found!

The version of the SDK (See pubspec.lock): 6.2.2

I have the following issue:

RunZonedGuardedIntegration does not await the result of running the app runner.

Steps to reproduce:

import 'package:sentry/sentry.dart';

import 'sentry_credentials.dart';

Future<void> main() async {
  await Sentry.init(
    (options) {
      options.dsn = sentryDsn;
    },
    appRunner: runApp,
  );

  // Required for the app to exit. Otherweise something is keeping the isolate running.
  await Sentry.close();
}

Future<void> runApp() async {
  await Future<void>.delayed(const Duration(seconds: 5));

  await Sentry.captureMessage('Hello');
}

Actual result: Sentry.close runs before runApp and Sentry.captureMessage is a noop.

Expected result: I expected that since Sentry.init returns a Future it would be possible to await that Future, to in turn wait for the app runner to finish.

marandaneto commented 2 years ago

IIRC, this was not possible due to the nature of the runZonedGuarded and how Flutter Apps work. runApp should be called within a runZonedGuarded to be able to capture uncaught errors, but if awaited, the SDK won't be fully initialized before the App is running, this would lead to another issue, @denrase investigated this before and that was the only solution. Related to https://github.com/getsentry/sentry-dart/pull/280

For a Console App or CLI, which is your code snippet, you don't need to pass appRunner, See the README using a custom runZonedGuarded without the appRunner.

Happy to improve if there's any other way, does that help?

blaugold commented 2 years ago

Thanks for the context and the advice.

It would be nice if CLI apps could use the appRunner option as well. It would mean a little less boilerplate (the custom runZonedGuarded), but RunZonedGuardedIntegration also implements breadcrumbs for print calls.

I think, returning a Future from Sentry.init that completes after all integrations have been called and appRunner has completed (with or without error), should not interfere with SDK initialization.

The solution should be pretty simple. What do you think about something like this?:

// In `RunZonedGuardedIntegration.call`

final completer = Completer();

runZonedGuarded(
  () async {
    try {
      await _runner();
    } finally {
      completer.complete();
    }
  },
 ...
)

return completer.future;
marandaneto commented 2 years ago

@blaugold I'm not entirely sure if this has been tested/tried by @denrase, but sounds like an alternative. Would you like to submit a PR and add tests to it as well?