getsentry / sentry-dart

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

pendingTimer when testing with SentryNavigatorObserver on sentry_flutter 8.2.0 #2103

Closed alestiago closed 1 month ago

alestiago commented 4 months ago

Platform

Flutter Mobile

Obfuscation

Enabled

Debug Info

Enabled

Doctor

[✓] Flutter (Channel stable, 3.22.1, on macOS 14.5 23F79 darwin-arm64, locale en-GB)
    • Flutter version 3.22.1 on channel stable at /Users/alestiago/Developer/flutter/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision a14f74ff3a (4 weeks ago), 2024-05-22 11:08:21 -0500
    • Engine revision 55eae6864b
    • Dart version 3.4.1
    • DevTools version 2.34.3

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
    • Android SDK at /Users/alestiago/Library/Android/sdk
    • Platform android-34, build-tools 33.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.12+0-b1504.28-7817840)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 15.2)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 15C500b
    • CocoaPods version 1.13.0

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

[✓] Android Studio (version 2021.2)
    • Android Studio at /Applications/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.12+0-b1504.28-7817840)

[✓] IntelliJ IDEA Ultimate Edition (version 2023.1.4)
    • IntelliJ at /Applications/IntelliJ IDEA.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 2023.1.4)
    • IntelliJ at /Applications/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.89.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.90.0

[✓] Connected device (3 available)
    • macOS (desktop)                 • macos                 • darwin-arm64   • macOS 14.5
      23F79 darwin-arm64
    • Mac Designed for iPad (desktop) • mac-designed-for-ipad • darwin         • macOS 14.5
      23F79 darwin-arm64
    • Chrome (web)                    • chrome                • web-javascript • Google Chrome
      125.0.6422.176

[✓] Network resources
    • All expected network resources are available.

• No issues found!

Version

8.2.0

Steps to Reproduce

  1. Create a Flutter Application:

    flutter create my_app --platforms ios,android
  2. Add package:sentry_flutter 8.2.0 as a dependency:

    flutter pub add sentry_flutter:8.2.0
  3. Update lib/main.dart to initialise Sentry:

    Future<void> main() async {
    runZonedGuarded(() async {
    await SentryFlutter.init(
      (options) {
        options.dsn = 'https://example@sentry.io/add-your-dsn-here';
      },
    );
    
    runApp(const MyApp());
    }, (exception, stackTrace) async {
    await Sentry.captureException(exception, stackTrace: stackTrace);
    });
    }
  4. Update the _incrementCounter function to navigate somewhere:

    void _incrementCounter() {
    Navigator.push(context, MaterialPageRoute(builder: (context) {
      return const Text('Hello Sentry');
    }));
    }
  5. Update the test in widget_test to navigate and assert for the text:

 testWidgets('Counter increments smoke test', (WidgetTester tester) async {
    await tester.pumpWidget(const MyApp());

    await tester.tap(find.byIcon(Icons.add));
    await tester.pumpAndSettle();

    expect(find.text('Hello Sentry'), findsWidgets);
  });
  1. Run the test, the test should succeed.

    fluter test
  2. Add a SentryNavigatorObserver to the MaterialApp in main.dart:

    MaterialApp(
    navigatorObservers: [
    SentryNavigatorObserver(),
    ],
    // other parameters
  3. Run the test again, the test now fails.

    fluter test

The test fails due to:

The following assertion was thrown running a test:
A Timer is still pending even after the widget tree was disposed.
'package:flutter_test/src/binding.dart':
Failed assertion: line 1527 pos 12: '!timersPending'
  1. Change the package:sentry_flutter version from 8.2.0 to 8.1.0:

    flutter pub add sentry_flutter:8.1.0
  2. Run the test again, now the test succeeds.

    fluter test

Expected Result

The test succeeds in package:sentry_flutter v8.1.0, it should also succeed in v8.2.0.

Actual Result

A test fails in package:sentry_flutter v8.2.0, but not in v8.1.0.

Are you willing to submit a PR?

None

buenaflor commented 4 months ago

Thanks for the issue and the detailed instructions! We'll have a look

buenaflor commented 4 months ago

For some quick fix you can wrap it in a runAsync

    await tester.runAsync(() async {
      await tester.pumpWidget(const MyApp());

      await tester.tap(find.byIcon(Icons.add));
      await tester.pumpAndSettle();

      expect(find.text('Hello Sentry'), findsWidgets);
    });

the reason for the failure is the change from

    return _appStartCompleter.future;

to

    return _appStartCompleter.future
        .timeout(_timeoutDuration, onTimeout: () => null);

still trying to figure out if therere is a way around that

alestiago commented 3 months ago

@buenaflor any updates regarding a proper fix to this issue besides the quick-fix?

buenaflor commented 3 months ago

Sorry :bow: I haven't come to this issue yet. I'll keep you posted

alestiago commented 2 months ago

Just chiming in again since it has been over a month, are there any updates?

buenaflor commented 2 months ago

hey, we're currently looking at re-designing the parts that are affected by these app start completers since this has also caused some issues in other areas for some users.

buenaflor commented 2 months ago

hey this is going to be addressed as part of improving the app start integration so this should be fixed soon :)