[firebase_crashlytics] Crashlytics cannot report every error by design

[debkanchan]

From what we know from flutter/flutter#10761, flutter/website#5612 and the proposal flutter/flutter#34862, to catch all errors, there must be a few things set up.

1. FlutterError.onError callback must be assigned.
2. runApp() must be called inside runZoneGuarded.
3. WidgetsFlutterBinding.ensureInitialized() must be the first thing called inside runZoneGuarded

But if we try to implement Crashlytics the following way:

void main() {
    runZonedGuarded<Future<void>>(() async {
        WidgetsFlutterBinding.ensureInitialized();
        await Firebase.initializeApp();
        FlutterError.onError = FirebaseCrashlytics.instance.recordFlutterError;
        runApp(CrashyApp()); 
    }, (error, stackTrace) => FirebaseCrashlytics.instance.recordError)
}

Few problems occur:

1. In case any FlutterError occurs before onError is assigned it won't be caught.
2. In case any async error occurs after entering runZoneGuarded but before Firebase.initializeApp() is finished, calling FirebaseCrashlytics.instance.recordError will cause a Default app not initialised error.

In this situation it looks like there can always be some errors which may be never caught.

[debkanchan]

@rrousselGit

[rrousselGit]

cross-reference https://www.reddit.com/r/FlutterDev/comments/mpy2am/proper_flutter_error_reporting/

cc @Salakar

What do you think of a runCrashlyticsApp?

void main() {
  runCrashlyticsApp(
    MyApp();
  );
}

which would take care of everything and fixes more advanced issues?

[Salakar]

What do you think of a

Yes 👍

[debkanchan]

If implemented this will be a great and very monumental update.

Still I think eventually flutter should come up with better, straight forward error handling solutions which doesn't require much Google hence new dev friendly. Or atleast have easy to find proper documentation for total error handling which is proposed in flutter/website#5612

[untp]

You can solve 1 with:

void main() {
    runZonedGuarded<Future<void>>(() async {
        FlutterError.onError = (details) => FirebaseCrashlytics.instance.recordFlutterError(details);
        WidgetsFlutterBinding.ensureInitialized();
        await Firebase.initializeApp();
        runApp(CrashyApp()); 
    }, (error, stackTrace) => FirebaseCrashlytics.instance.recordError(error, stackTrace));
}

For 2, we can make Crashlytics independent from Firebase.initializeApp. But isCrashlyticsCollectionEnabled needs to be async.

For runCrashlyticsApp, I think it should be renamed to runAppWithCrashlytics.

[rrousselGit]

After thinking a bit, I don't think runCrashlyticsApp is the correct solution

This solves simple use-cases, but people may put logic in their main for other reasons than firebase, and we want to capture the errors for those things too.

What I'm considering at the moment is:

void main() {
  startCrashlytics(() async {
    await whatever;
    runApp(MyApp();
  });
}

[untp]

@rrousselGit Yes, this is better. We can create runWithCrashlytics function.
But there is something more important. Should we make Crashlytics independent from Firebase.initializeApp? The reason behind the Crashlytics crashes before initializeApp is, Crashlytics requires isCrashlyticsCollectionEnabled variable. isCrashlyticsCollectionEnabled is a sync getter, so it needs to be set when initialization. We can make isCrashlyticsCollectionEnabled async and then make Crashlytics independent from initializeApp. We can catch and send errors before initializeApp.

[rrousselGit]

That would be a breaking change, which is against our policy

There are alternative solutions to this problem.

[debkanchan]

I agree with both of you. Designing Crashlytics in such a way that it is the first piece of code that runs and runs independently will solve the problems and still provide flexibility and independence for developers to do custom things.