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.

Sign up for GitHub

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

Jump to bottom

feat: Add extra to sentry. [CONSVC-1886] #333

Merged
merged 17 commits into from
Jan 27, 2023
Merged

feat: Add extra to sentry. [CONSVC-1886] #333

merged 17 commits into from
Jan 27, 2023

Conversation

jrconlin
Copy link
Member

@jrconlin jrconlin commented Sep 12, 2022 •
edited
Loading

  • Update sentry to latest
  • convert from chain-error to thiserror

@jrconlin jrconlin requested review from ethowitz, taddes and a team September 12, 2022 20:34
@jrconlin jrconlin changed the title Feat/1886 sentry feat: Add extra to sentry. [CONSVC-1886] Sep 12, 2022
pjenvey
pjenvey requested changes Jan 24, 2023
View reviewed changes
autopush-common/src/db/commands.rs Outdated Show resolved Hide resolved
tests/test_integration_all_rust.py Outdated
@@ -690,6 +690,11 @@ def _ws_url(self):
@max_logs(conn=4)
def test_sentry_output(self):
# Ensure bad data doesn't throw errors
## The latest sentry library no longer uses the same
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a fix for this (capturing via the new sentry envelope API) here: feat/1886-sentry...feat/1886-sentry-test

autopush/src/client.rs Outdated Show resolved Hide resolved
autopush/src/server/mod.rs Outdated Show resolved Hide resolved
autopush-common/src/errors.rs Outdated Show resolved Hide resolved
autopush-common/src/errors.rs Outdated Show resolved Hide resolved
@jrconlin
Merge branch 'master' of github.com:mozilla-services/autopush-rs into…
dabcd26 
… feat/1886-sentry
@jrconlin
f r's
864dc6f 
* Redo the errors to use `ApcErrorKind` to capture backtrace.
* Add clippy ignore for the large return value.
pjenvey
pjenvey requested changes Jan 26, 2023
View reviewed changes
Copy link
Member

@pjenvey pjenvey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding backtraces. There's a few more commented out lines of code, otherwise looks good.

autopush-common/src/db/commands.rs Outdated Show resolved Hide resolved
autopush-common/src/db/commands.rs Outdated Show resolved Hide resolved
autopush-common/src/db/commands.rs Outdated Show resolved Hide resolved
autopush-common/src/lib.rs Outdated Show resolved Hide resolved
autopush/src/client.rs Outdated Show resolved Hide resolved
@jrconlin jrconlin requested review from pjenvey and removed request for ethowitz January 26, 2023 23:45
@jrconlin jrconlin merged commit 008e3e8 into master Jan 27, 2023
@jrconlin jrconlin deleted the feat/1886-sentry branch January 27, 2023 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pjenvey pjenvey pjenvey approved these changes

@taddes taddes Awaiting requested review from taddes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jrconlin @pjenvey