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

Sentry filter #318

Merged
merged 4 commits into from
Sep 7, 2022
Merged

Sentry filter #318

merged 4 commits into from
Sep 7, 2022

Conversation

sideninja
Copy link
Contributor

This PR implements Sentry error filtering as described on official docs. It currently blacklist two types of errros: GraphQL errors and Network errors which were mostly spam since they were not really related to the client issues.


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the standards mentioned here.
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@vercel
Copy link

vercel bot commented Sep 7, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
flow-playground ✅ Ready (Inspect) Visit Preview Sep 7, 2022 at 4:41PM (UTC)

@sideninja sideninja mentioned this pull request Sep 7, 2022
6 tasks
@vercel vercel bot temporarily deployed to Preview September 7, 2022 09:26 Inactive
src/index.tsx Outdated
const error = hint.originalException;

const ignoreErrors = [
/GraphQL/i, // filter out graphql errors
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it also filter out real exceptions if they contain the GraphQL string?
This question doesn't block merge, but will need to be addressed in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe there will be any honestly. We can if this will be a problem have a unique prepended string like [Playground:GraphQL] if needed.

src/index.tsx Outdated
// filter out blacklisted errors
if (error && error.message) {
for (const filter of ignoreErrors) {
if (error.message.match(filter)) {
Copy link

@DylanTinianov DylanTinianov Sep 7, 2022

Choose a reason for hiding this comment

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

I don't think the matching logic is required. If you define ignoreErrors outside of the function as an initialization param it will automatically match and filter out the strings from the error messages: https://docs.sentry.io/clients/javascript/config/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I've seen that not sure why I added as a function. Maybe I was thinking we will need more than regex. Will change to that.

@vercel vercel bot temporarily deployed to Preview September 7, 2022 16:41 Inactive
@sideninja sideninja merged commit d00fd93 into staging Sep 7, 2022
@sideninja sideninja deleted the improvement/sentry-filter branch September 7, 2022 17:31
This was referenced Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants