-
Notifications
You must be signed in to change notification settings - Fork 49
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
feat: Add normalized ReportableError to errors #1559
Conversation
We want to do things like add tags and other features to sync errors the way that we do in other packages. To do so, we're backporting ReportableError from Autopush to Syncstorage. This also addresses some clippy fixes required by 1.78 Closes SYNC-4262
* More clippy updates
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 is missing the middleware integration piece
…nto feat/SYNC-4262_rep_error
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.
Darnit, oops, I had a couple pending comment nitpicks, but go ahead and merge as is if you want
.wrap_fn(middleware::weave::set_weave_timestamp) | ||
.wrap_fn(tokenserver::logging::handle_request_log_line) | ||
.wrap_fn(middleware::sentry::report_error) | ||
//.wrap_fn(middleware::sentry::report_error) |
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.
//.wrap_fn(middleware::sentry::report_error) |
} | ||
|
||
/// Experimental: return key value pairs for Sentry Event's extra data | ||
/// TODO: should probably return Vec<(&str, Value)> or Vec<(String, Value)> |
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.
/// TODO: should probably return Vec<(&str, Value)> or Vec<(String, Value)> |
// These are our wrappers | ||
.wrap_fn(middleware::sentry::report_error) | ||
//.wrap_fn(middleware::sentry::report_error) |
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.
//.wrap_fn(middleware::sentry::report_error) |
I'll file a quick follow-up PR that has these fixes. |
We want to do things like add tags and other features to sync errors the
way that we do in other packages. To do so, we're backporting
ReportableError from Autopush to Syncstorage.
This also addresses:
Closes SYNC-4262