-
Notifications
You must be signed in to change notification settings - Fork 58
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
CPLAT-7721 Add error logging to ErrorBoundary #370
Conversation
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
2ac4788
to
96714f3
Compare
96714f3
to
be378d7
Compare
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.
Looking good, some comments/questions
Logger get logger => _logger; | ||
Logger _logger; | ||
/// The value that will be used when creating a [Logger] to log errors from this component. | ||
String get loggerName { |
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.
Is there a use-case for making this public? Consumers know which logger/loggerName they passed in; we shouldn't need to expose that info via an API method.
Logger _logger; | ||
/// The value that will be used when creating a [Logger] to log errors from this component. | ||
String get loggerName { | ||
if (props.logger != null) return props.logger.name; |
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.
Related to the above comment; if this isn't public, this won't be necessary.
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.
+1
+ Dart 1 port of #370
QA +1 @Workiva/release-management-p |
Motivation
Consumers currently have to set their own
onComponentDidCatch
prop callback, and then manually pass the errors passed to that callback into their own instance ofLogger
.Changes
ErrorBoundary
so that consumers get it for free, and they can simply setprops.loggerName
to customize the logger name if they choose.ErrorBoundaryComponent.logger
if they want to log it to the service of their choice.Release Notes
Add logging functionality to the ErrorBoundary
Review
See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.
Please review: @greglittlefield-wf @kealjones-wk @joebingham-wk @sydneyjodon-wk @maxwellpeterson-wf
QA Checklist
Merge Checklist
While we perform many automated checks before auto-merging, some manual checks are needed: