-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
docs(crashlytics): Improve Crashlytics documentation around catching sync/async exceptions. #10772
Conversation
…patcher.instance.onError
… FirebaseCrashlytics
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
// If you wish to record a "non-fatal" exception, please use `FirebaseCrashlytics.instance.recordFlutterError` instead | ||
FirebaseCrashlytics.instance.recordFlutterFatalError(errorDetails); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should look something like this:
const fatalError = true;
FlutterError.onError = (errorDetails) {
if (fatalError) {
// If you wish to record a "fatal" exception
FirebaseCrashlytics.instance.recordFlutterFatalError(errorDetails);
} else {
// If you wish to record a "non-fatal" exception
FirebaseCrashlytics.instance.recordFlutterError(errorDetails);
}
};
Also - please update this part to look like this:
const fatalError = true;
PlatformDispatcher.instance.onError = (error, stack) {
if (fatalError) {
// If you wish to record a "fatal" exception
FirebaseCrashlytics.instance.recordError(error, stack, fatal: true);
} else {
// If you wish to record a "non-fatal" exception
FirebaseCrashlytics.instance.recordError(error, stack);
}
return true;
};
It would also be cool if you updated the example app here so it looked like this:
const fatalError = true;
// Non-async exceptions
FlutterError.onError = (errorDetails) {
if (fatalError) {
// If you wish to record a "fatal" exception
FirebaseCrashlytics.instance.recordFlutterFatalError(errorDetails);
} else {
// If you wish to record a "non-fatal" exception
FirebaseCrashlytics.instance.recordFlutterError(errorDetails);
}
};
// Async exceptions
PlatformDispatcher.instance.onError = (error, stack) {
if (fatalError) {
// If you wish to record a "fatal" exception
FirebaseCrashlytics.instance.recordError(error, stack, fatal: true);
} else {
// If you wish to record a "non-fatal" exception
FirebaseCrashlytics.instance.recordError(error, stack);
}
return true;
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated it.
4c74317
@kevinthecheung - we've made some changes to Crashlytics documentation if you could give it a review, that would be great. Thanks. |
} else { | ||
FirebaseCrashlytics.instance.recordFlutterError(errorDetails); | ||
} | ||
// If you wish to record a "non-fatal" exception, please use `FirebaseCrashlytics.instance.recordFlutterError` instead |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throughout --
- pls change "wish" to "want"
- pls remove the word "please" (we don't use "please" in our tech docs)
FirebaseCrashlytics.instance.recordError(error, stack, fatal: true); | ||
if (fatalError) { | ||
// If you wish to record a "fatal" exception | ||
FirebaseCrashlytics.instance.recordError(error, stack, fatal: true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be more succinct to have fatal: fatalError
on this line instead of the if statement?
@K9i-0 - do you mind changing the above mentioned by @rachelsaunders & @mrober, please? We can get this PR merged once done. Thanks. |
Closing in favour of: #11275 |
Description
This PR updates the documentation to clarify error handling and exception reporting with FirebaseCrashlytics in Flutter applications. The changes aim to provide clearer instructions for developers integrating FirebaseCrashlytics.
The original text written for using runZonedGuarded was unnatural.
The documentation's phrasing has been updated based on the example code.
Related Issues
None. This PR is not related to any existing issues from the issue database.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
).This will ensure a smooth and quick review process. Updating the
pubspec.yaml
and changelogs is not required.///
).melos run analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?