Skip to content

Commit

Permalink
feat: Rework how we capture service errors to sentry (#3436)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nicholas-codecov
nicholas-codecov authored Oct 28, 2024
1 parent db905aa commit 55c16c2
Show file tree
Hide file tree
Showing 8 changed files with 121 additions and 226 deletions.
16 changes: 0 additions & 16 deletions src/layouts/shared/NetworkErrorBoundary/NetworkErrorBoundary.jsx
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as Sentry from '@sentry/react'
import { useQueryClient } from '@tanstack/react-query'
import cs from 'classnames'
import PropTypes from 'prop-types'
Expand Down Expand Up @@ -171,21 +170,6 @@ 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))) {
// 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) {
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 }
}

Expand Down
198 changes: 0 additions & 198 deletions src/layouts/shared/NetworkErrorBoundary/NetworkErrorBoundary.test.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,6 @@ import NetworkErrorBoundary from './NetworkErrorBoundary'
vi.spyOn(console, 'error').mockImplementation(() => undefined)
vi.mock('config')

const mocks = vi.hoisted(() => ({
captureMessage: vi.fn(),
}))

vi.mock('@sentry/react', async () => {
const actual = await vi.importActual('@sentry/react')
return {
...actual,
captureMessage: mocks.captureMessage,
}
})

const queryClient = new QueryClient({
defaultOptions: { queries: { retry: false } },
})
Expand Down Expand Up @@ -194,34 +182,6 @@ describe('NetworkErrorBoundary', () => {
const returnButton = await screen.findByText('Return to previous page')
expect(returnButton).toBeInTheDocument()
})

it('calls captureMessage with the error when dev and error are provided', async () => {
const { user } = setup()
render(
<App
status={401}
detail="not authenticated"
error="cool error"
dev="401 - cool error"
/>,
{ wrapper: wrapper() }
)

const textBox = await screen.findByRole('textbox')
await user.type(textBox, 'fail')

await waitFor(() =>
expect(mocks.captureMessage).toHaveBeenCalledWith('Network Error', {
fingerprint: '401 - cool error',
addBreadcrumb: {
category: 'network-error',
data: 'cool error',
level: 'error',
message: '401 - cool error',
},
})
)
})
})

describe('when the children component has a 403 error', () => {
Expand Down Expand Up @@ -263,34 +223,6 @@ describe('NetworkErrorBoundary', () => {
const button = await screen.findByText('Return to previous page')
expect(button).toBeInTheDocument()
})

it('calls captureMessage with the error when dev and error are provided', async () => {
const { user } = setup()
render(
<App
status={403}
detail="you not admin"
error="cool error"
dev="403 - cool error"
/>,
{ wrapper: wrapper() }
)

const textBox = await screen.findByRole('textbox')
await user.type(textBox, 'fail')

await waitFor(() =>
expect(mocks.captureMessage).toHaveBeenCalledWith('Network Error', {
fingerprint: '403 - cool error',
addBreadcrumb: {
category: 'network-error',
data: 'cool error',
level: 'error',
message: '403 - cool error',
},
})
)
})
})

describe('when the children component has a 404 error', () => {
Expand Down Expand Up @@ -320,34 +252,6 @@ describe('NetworkErrorBoundary', () => {
const button = await screen.findByText('Return to previous page')
expect(button).toBeInTheDocument()
})

it('calls captureMessage with the error when dev and error are provided', async () => {
const { user } = setup()
render(
<App
status={404}
detail="not found"
error="cool error"
dev="404 - cool error"
/>,
{ wrapper: wrapper() }
)

const textBox = await screen.findByRole('textbox')
await user.type(textBox, 'fail')

await waitFor(() =>
expect(mocks.captureMessage).toHaveBeenCalledWith('Network Error', {
fingerprint: '404 - cool error',
addBreadcrumb: {
category: 'network-error',
data: 'cool error',
level: 'error',
message: '404 - cool error',
},
})
)
})
})

describe('when running in self hosted mode', () => {
Expand Down Expand Up @@ -376,34 +280,6 @@ describe('NetworkErrorBoundary', () => {
const button = await screen.findByText('Return to previous page')
expect(button).toBeInTheDocument()
})

it('calls captureMessage with the error when dev and error are provided', async () => {
const { user } = setup({ isSelfHosted: true })
render(
<App
status={404}
detail="not found"
error="cool error"
dev="404 - cool error"
/>,
{ wrapper: wrapper() }
)

const textBox = await screen.findByRole('textbox')
await user.type(textBox, 'fail')

await waitFor(() =>
expect(mocks.captureMessage).toHaveBeenCalledWith('Network Error', {
fingerprint: '404 - cool error',
addBreadcrumb: {
category: 'network-error',
data: 'cool error',
level: 'error',
message: '404 - cool error',
},
})
)
})
})
})

Expand Down Expand Up @@ -454,24 +330,6 @@ describe('NetworkErrorBoundary', () => {
// Clean up the mock
global.fetch.mockRestore()
})

it('does not call captureMessage ', async () => {
const { user } = setup()
render(
<App
status={429}
detail="rate limit exceeded"
error="cool error"
dev="429 - cool error"
/>,
{ wrapper: wrapper() }
)

const textBox = await screen.findByRole('textbox')
await user.type(textBox, 'fail')

await waitFor(() => expect(mocks.captureMessage).not.toHaveBeenCalled())
})
})

describe('when the children component has a 500 error', () => {
Expand Down Expand Up @@ -501,34 +359,6 @@ describe('NetworkErrorBoundary', () => {
const button = await screen.findByText('Return to previous page')
expect(button).toBeInTheDocument()
})

it('calls captureMessage with the error when dev and error are provided', async () => {
const { user } = setup()
render(
<App
status={500}
detail="server error"
error="cool error"
dev="500 - cool error"
/>,
{ wrapper: wrapper() }
)

const textBox = await screen.findByRole('textbox')
await user.type(textBox, 'fail')

await waitFor(() =>
expect(mocks.captureMessage).toHaveBeenCalledWith('Network Error', {
fingerprint: '500 - cool error',
addBreadcrumb: {
category: 'network-error',
data: 'cool error',
level: 'error',
message: '500 - cool error',
},
})
)
})
})

describe('when running in self-hosted mode', () => {
Expand Down Expand Up @@ -557,34 +387,6 @@ describe('NetworkErrorBoundary', () => {
const button = await screen.findByText('Return to previous page')
expect(button).toBeInTheDocument()
})

it('calls captureMessage with the error when dev and error are provided', async () => {
const { user } = setup({ isSelfHosted: true })
render(
<App
status={500}
detail="server error"
error="cool error"
dev="500 - cool error"
/>,
{ wrapper: wrapper() }
)

const textBox = await screen.findByRole('textbox')
await user.type(textBox, 'fail')

await waitFor(() =>
expect(mocks.captureMessage).toHaveBeenCalledWith('Network Error', {
fingerprint: '500 - cool error',
addBreadcrumb: {
category: 'network-error',
data: 'cool error',
level: 'error',
message: '500 - cool error',
},
})
)
})
})
})

Expand Down
6 changes: 3 additions & 3 deletions src/services/bundleAnalysis/useBundleSummary.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
useRepoOverview,
} from 'services/repo'
import Api from 'shared/api/api'
import type { NetworkErrorObject } from 'shared/api/helpers'
import { NetworkErrorObject, rejectNetworkError } from 'shared/api/helpers'
import A from 'ui/A'

const BundleDataSchema = z.object({
Expand Down Expand Up @@ -167,12 +167,12 @@ export const useBundleSummary = ({
const parsedData = RequestSchema.safeParse(res?.data)

if (!parsedData.success) {
return Promise.reject({
return rejectNetworkError({
status: 404,
data: {},
dev: 'useBundleSummary - 404 Failed to parse data',
error: parsedData.error,
} satisfies NetworkErrorObject)
})
}

const data = parsedData.data
Expand Down
2 changes: 1 addition & 1 deletion src/services/commit/useCommit.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ describe('useCommit', () => {
}),
graphql.query(`CompareTotals`, (info) => {
if (skipPolling) {
return HttpResponse.json({ data: {} })
return HttpResponse.json({ data: { owner: null } })
}
return HttpResponse.json({ data: compareDoneData })
})
Expand Down
15 changes: 8 additions & 7 deletions src/services/commit/useCommit.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
RepoOwnerNotActivatedErrorSchema,
} from 'services/repo/schemas'
import Api from 'shared/api'
import { NetworkErrorObject } from 'shared/api/helpers'
import { rejectNetworkError } from 'shared/api/helpers'
import {
ErrorCodeEnum,
UploadStateEnum,
Expand Down Expand Up @@ -356,25 +356,26 @@ export function useCommit({
const parsedRes = RequestSchema.safeParse(res?.data)

if (!parsedRes.success) {
return Promise.reject({
return rejectNetworkError({
status: 404,
data: {},
dev: 'useCommit - 404 failed to parse',
} satisfies NetworkErrorObject)
error: parsedRes.error,
})
}

const data = parsedRes.data

if (data?.owner?.repository?.__typename === 'NotFoundError') {
return Promise.reject({
return rejectNetworkError({
status: 404,
data: {},
dev: 'useCommit - 404 not found',
} satisfies NetworkErrorObject)
})
}

if (data?.owner?.repository?.__typename === 'OwnerNotActivatedError') {
return Promise.reject({
return rejectNetworkError({
status: 403,
data: {
detail: (
Expand All @@ -387,7 +388,7 @@ export function useCommit({
),
},
dev: 'useCommit - 403 owner not activated',
} satisfies NetworkErrorObject)
})
}

const commit = data?.owner?.repository?.commit
Expand Down
Loading

0 comments on commit 55c16c2

Please sign in to comment.