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

Skip reporting some client errors to Sentry #4873

Open
bjester opened this issue Jan 16, 2025 · 5 comments · May be fixed by #4881
Open

Skip reporting some client errors to Sentry #4873

bjester opened this issue Jan 16, 2025 · 5 comments · May be fixed by #4881
Assignees
Labels
DEV: dev-ops DEV: frontend help wanted Open source contributors welcome P2 - normal Priority: Nice to have TAG: dev experience

Comments

@bjester
Copy link
Member

bjester commented Jan 16, 2025

Current behavior

Studio's Axios network client is configured to intercept and log network request errors. We receive many errors that aren't very illuminating, create noise, and some we can possibly prevent from reaching Sentry because we don't care about them:

  • Request aborted: presumably this comes from XHR requests that are aborted as the user navigates to a different page
  • Redirect requests (301/302): we rarely use redirects so this may be worthwhile

Desired behavior

The network client:

  • Should ignore errors that occur during page navigation (use navigate event)
  • Should not report anything regarding redirects, unless following redirects exceeded limitations

Value add

Minimizes the noise making important Sentry errors easier to identify

Possible tradeoffs

Regarding 302 redirects, a downside would be that if a bug occurs related to them, we wouldn't have any hint like we do today.

@MisRob MisRob added the help wanted Open source contributors welcome label Jan 20, 2025
@SukhvirKooner
Copy link

Hi @bjester ,

I want to work on this Issue, Could you please assign this to me?

To proceed, could you help clarify:

  1. How can I reliably reproduce this error in a local or staging environment?
  2. Should I entirely suppress these errors?
  3. Is there an existing policy or best practice for managing such errors in Sentry?

Let me know if there’s anything specific you'd like me to focus on while addressing this!

@bjester
Copy link
Member Author

bjester commented Jan 21, 2025

@SukhvirKooner I've assigned you

  1. We cannot provide you access to Sentry, so reproducing this will be a manual process. The Request aborted error should be self-explanatory. If you have specific questions about that, please do ask. For the 302, we receive error reports of that from calls to get_node_diff but it's probably more effort than it's worth to reproduce that path. I suggest manually editing some API endpoint to return a 302 and verify the code path in the issue's linked code does not attempt to log the error.
  2. Yes
  3. Please take a look at the linked code in the issue. There are already examples of suppressing errors

@SukhvirKooner
Copy link

SukhvirKooner commented Jan 25, 2025

Hi ,
Thank you for assigning me this issue. I was unavailable for the past few days, but now I have started working on it. I appreciate the guidance provided, and I’ll be sure to reach out if I have any questions.

@MisRob
Copy link
Member

MisRob commented Jan 27, 2025

No problem, thanks @SukhvirKooner

@SukhvirKooner
Copy link

PR Update: Suppression of Request Abort and Redirect Request Errors

I have addressed both concerns outlined in the issue:

  1. Request Aborted Errors:

    • Implemented a robust suppression mechanism using AbortController and navigation state flags (isNavigating).
    • This ensures that request errors triggered during page navigation are ignored.
  2. Redirect Request Errors (301/302):

    • Suppressed errors for unnecessary redirect requests, reducing noise in Sentry.

These changes minimize irrelevant error logs, making it easier to spot critical issues.
Legitimate errors are still logged and reported to Sentry as expected.

A PR has been raised for review. Looking forward to your feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DEV: dev-ops DEV: frontend help wanted Open source contributors welcome P2 - normal Priority: Nice to have TAG: dev experience
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants