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

Enha: Don't supress error logs #1228

Merged
merged 6 commits into from
Jan 23, 2023
Merged

Enha: Don't supress error logs #1228

merged 6 commits into from
Jan 23, 2023

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Jan 17, 2023

📜 Description

Closes #543

Introduces and onError callback in the RunZonedGuarded intergrations, so that we can call FlutterError.dumpToConsole when an error is caught by the integration.

This is done this way, as it's the recommended way per flutter documentation.

Note: Consider calling [FlutterError.presentError](https://api.flutter.dev/flutter/foundation/FlutterError/presentError.html) from your custom error handler in order to see the logs in the console as well.

On web this does not yield the same output, but the error name and the StackTrace is there.

Bildschirm­foto 2023-01-23 um 11 03 21

💡 Motivation and Context

When executing code in custom runZonedGuarded, errors on web are not logged to the console anymore.

💚 How did you test it?

Added unit test.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

Discuss if we need to handle other cases as well.

@codecov
Copy link

codecov bot commented Jan 17, 2023

Codecov Report

❗ No coverage uploaded for pull request base (v7.0.0@68ea1ef). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff            @@
##             v7.0.0    #1228   +/-   ##
=========================================
  Coverage          ?   76.87%           
=========================================
  Files             ?       11           
  Lines             ?      333           
  Branches          ?        0           
=========================================
  Hits              ?      256           
  Misses            ?       77           
  Partials          ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@denrase
Copy link
Collaborator Author

denrase commented Jan 17, 2023

@marandaneto Not sure if this is the correct approach. Do we only need something like this for runZonedGuarded or also for Isolate.current.addErrorListener?

@denrase denrase marked this pull request as ready for review January 23, 2023 10:36
@marandaneto
Copy link
Contributor

@denrase can you check if this causes problems with the debugPrint integration?
Enable enablePrintBreadcrumbs and check the DebugPrintIntegration, I hope this somehow does not go to a recursive loop or something.

@denrase
Copy link
Collaborator Author

denrase commented Jan 23, 2023

@marandaneto Not seeing any difference with enablePrintBreadcrumbs when running on web and provoking an error thats thrown in runZonedGuarded.

@denrase denrase merged commit 16ab023 into v7.0.0 Jan 23, 2023
@denrase denrase deleted the enha/dont-supress-error-logs branch January 23, 2023 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants