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

feat: Capture certain network errors to Sentry #3423

Merged
merged 4 commits into from
Oct 22, 2024
Merged

feat: Capture certain network errors to Sentry #3423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
97ac6c1
remove network sentry metrics
nicholas-codecov Oct 22, 2024
08d72ac
update network error object to have optional error field
nicholas-codecov Oct 22, 2024
4e944bd
update NetworkErrorObject to conditionally send errors to sentry
nicholas-codecov Oct 22, 2024
5d34d9c
update useBundleSummary as an example
nicholas-codecov Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions src/layouts/shared/NetworkErrorBoundary/NetworkErrorBoundary.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as Sentry from '@sentry/react'
import { useQueryClient } from '@tanstack/react-query'
import cs from 'classnames'
import PropTypes from 'prop-types'
Expand All @@ -12,10 +13,6 @@ import Button from 'ui/Button'
import openUmbrella from './assets/error-open-umbrella.svg'
import upsideDownUmbrella from './assets/error-upsidedown-umbrella.svg'
import styles from './NetworkErrorBoundary.module.css'
import {
sendGraphQLErrorMetrics,
sendNetworkErrorMetrics,
} from './networkErrorMetrics'

const errorToUI = {
401: {
Expand Down Expand Up @@ -174,12 +171,26 @@ class NetworkErrorBoundary extends Component {
// if the error is not a network error, we don't do anything and
// another error boundary will take it from there
if (Object.keys(errorToUI).includes(String(error.status))) {
sendNetworkErrorMetrics(error.status)
// only capture network errors if they are not a rate limit error
// this will typically only be schema parsing errors
if (error.status !== 429 && error.dev && error.error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on making it easier for us in the case we forget to include both error.dev and error.errror and that means our errors don't get tracked? I could definitely see myself doing that. A fallback catchall fingerprint name perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When you use the NetworkErrorObject it will enforce you to add dev to the rejection object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only issue with having a catchall is that we're likely to catch errors that are out of our control, such as repo not activated, etc. which we don't want to be flooding Sentry with.

Sentry.captureMessage('Network Error', {
// group all network errors together based off of their dev message
fingerprint: error.dev,
// add a breadcrumb for with the message and error
addBreadcrumb: {
category: 'network-error',
message: error.dev,
level: 'error',
data: error.error,
},
})
}
return { hasNetworkError: true, error }
}

if (Object.keys(graphQLErrorToUI).includes(error.__typename)) {
sendGraphQLErrorMetrics(error.__typename)
Copy link
Contributor

Choose a reason for hiding this comment

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

Losing out on the metrics is a bummer. Hopefully things are getting captured on the api prometheus side

// there are no errors we want to capture for graphql errors
return { hasGraphqlError: true, error }
}

Expand Down
Loading
Loading