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

Add new client error exception #745

Merged
merged 4 commits into from
Jul 3, 2024
Merged

Conversation

savhappy
Copy link
Collaborator

@savhappy savhappy commented Jul 1, 2024

-add client error module to handle Sentry client errors

@savhappy savhappy requested a review from whatyouhide July 1, 2024 05:17
@savhappy
Copy link
Collaborator Author

savhappy commented Jul 1, 2024

This does not change anything in the logs. Should the logging be changed to match the correct message response coming from client error module?

@whatyouhide whatyouhide changed the title add new client error exception Add new client error exception Jul 1, 2024
Copy link
Collaborator

@whatyouhide whatyouhide left a comment

Choose a reason for hiding this comment

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

Left A BUNCH of comments, sorry! Fantastic work though, this is exactly what I had in mind. 🙃

lib/sentry/client.ex Outdated Show resolved Hide resolved
lib/sentry/client_error.ex Outdated Show resolved Hide resolved
lib/sentry/client_error.ex Outdated Show resolved Hide resolved
lib/sentry/client_error.ex Show resolved Hide resolved
lib/sentry/client_error.ex Outdated Show resolved Hide resolved
lib/sentry/client_error.ex Outdated Show resolved Hide resolved
test/sentry/client_error_test.exs Outdated Show resolved Hide resolved
lib/sentry/client_error.ex Show resolved Hide resolved
lib/sentry/client_error.ex Outdated Show resolved Hide resolved
lib/sentry/client_error.ex Outdated Show resolved Hide resolved
@savhappy savhappy requested a review from whatyouhide July 1, 2024 17:26
@savhappy savhappy force-pushed the sav/add-rate-limit-exception branch 3 times, most recently from 2d8a9dc to d06ff76 Compare July 1, 2024 17:39
lib/sentry.ex Outdated
Comment on lines 220 to 223
@typedoc """
An error that is returned from Sentry Client.
"""
@type error :: ClientError.t()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I’m thinking we don't need to introduce this for now since ClientError is public, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was thinking preemptive in case we introduce more error types but removed this. You're right, it's redundant.

@savhappy savhappy force-pushed the sav/add-rate-limit-exception branch from 018ee5c to 6aa7c97 Compare July 3, 2024 16:47
@savhappy savhappy requested a review from whatyouhide July 3, 2024 16:53
@whatyouhide whatyouhide merged commit c70d875 into master Jul 3, 2024
4 checks passed
@whatyouhide
Copy link
Collaborator

Yeeehaaaw

@whatyouhide whatyouhide deleted the sav/add-rate-limit-exception branch July 3, 2024 20:57
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