classify report dialog errors
This commit is contained in:
@@ -0,0 +1,107 @@
|
||||
import {XRPCError} from '@atproto/api'
|
||||
|
||||
import {classifyReportError} from './errors'
|
||||
|
||||
describe('classifyReportError', () => {
|
||||
it('treats account takedown as an expected rejection', () => {
|
||||
const result = classifyReportError(
|
||||
new XRPCError(
|
||||
403,
|
||||
'AccountTakedown',
|
||||
'Report not accepted from takendown account',
|
||||
),
|
||||
)
|
||||
|
||||
expect(result).toMatchObject({
|
||||
kind: 'account-takedown',
|
||||
shouldReport: false,
|
||||
fingerprint: ['{{ default }}', 'report-dialog:account-takedown'],
|
||||
tags: {
|
||||
report_error_kind: 'account-takedown',
|
||||
report_error_bucket: 'account-takedown',
|
||||
report_xrpc_error: 'AccountTakedown',
|
||||
report_http_status: 403,
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
it.each([
|
||||
{
|
||||
error: new XRPCError(
|
||||
502,
|
||||
'InternalServerError',
|
||||
'Failed to perform upstream request',
|
||||
),
|
||||
bucket: 'upstream-fetch',
|
||||
},
|
||||
{
|
||||
error: new XRPCError(502, 'UpstreamFailure', 'Internal Server Error'),
|
||||
bucket: 'upstream-internal',
|
||||
},
|
||||
{
|
||||
error: new XRPCError(
|
||||
502,
|
||||
'UpstreamFailure',
|
||||
'Upstream server responded with a 502 error',
|
||||
),
|
||||
bucket: 'upstream-http-502',
|
||||
},
|
||||
{
|
||||
error: new XRPCError(
|
||||
504,
|
||||
'UpstreamTimeout',
|
||||
'Upstream server responded with a 504 error',
|
||||
),
|
||||
bucket: 'upstream-http-504',
|
||||
},
|
||||
])('classifies $bucket as unavailable', ({error, bucket}) => {
|
||||
expect(classifyReportError(error)).toMatchObject({
|
||||
kind: 'service-unavailable',
|
||||
shouldReport: true,
|
||||
fingerprint: ['{{ default }}', `report-dialog:${bucket}`],
|
||||
})
|
||||
})
|
||||
|
||||
it('classifies an invalid reason type separately', () => {
|
||||
const result = classifyReportError(
|
||||
new XRPCError(
|
||||
400,
|
||||
'InvalidRequest',
|
||||
'Invalid reason type: tools.ozone.report.defs#reasonOther',
|
||||
),
|
||||
)
|
||||
|
||||
expect(result).toMatchObject({
|
||||
kind: 'invalid-reason-type',
|
||||
shouldReport: true,
|
||||
fingerprint: ['{{ default }}', 'report-dialog:invalid-reason-type'],
|
||||
})
|
||||
})
|
||||
|
||||
it.each([400, 404])(
|
||||
'separates a non-retryable upstream %i without calling it temporary',
|
||||
status => {
|
||||
const result = classifyReportError(
|
||||
new XRPCError(
|
||||
502,
|
||||
'UpstreamFailure',
|
||||
`Upstream server responded with a ${status} error`,
|
||||
),
|
||||
)
|
||||
|
||||
expect(result).toMatchObject({
|
||||
kind: 'unexpected',
|
||||
shouldReport: true,
|
||||
fingerprint: ['{{ default }}', `report-dialog:upstream-http-${status}`],
|
||||
})
|
||||
},
|
||||
)
|
||||
|
||||
it('classifies non-XRPC errors as unexpected', () => {
|
||||
expect(classifyReportError(new Error('boom'))).toMatchObject({
|
||||
kind: 'unexpected',
|
||||
shouldReport: true,
|
||||
fingerprint: ['{{ default }}', 'report-dialog:unexpected'],
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,111 @@
|
||||
import {XRPCError} from '@atproto/api'
|
||||
|
||||
import {isRetryableHttpStatus, shouldRetryError} from '#/lib/strings/errors'
|
||||
|
||||
export type ReportErrorKind =
|
||||
| 'account-takedown'
|
||||
| 'invalid-reason-type'
|
||||
| 'service-unavailable'
|
||||
| 'unexpected'
|
||||
|
||||
export type ReportErrorClassification = {
|
||||
kind: ReportErrorKind
|
||||
shouldReport: boolean
|
||||
fingerprint: string[]
|
||||
tags: Record<string, string | number>
|
||||
}
|
||||
|
||||
export function classifyReportError(error: unknown): ReportErrorClassification {
|
||||
if (!(error instanceof XRPCError)) {
|
||||
return classification('unexpected', 'unexpected', true)
|
||||
}
|
||||
|
||||
const xrpcTags = {
|
||||
report_xrpc_error: error.error,
|
||||
report_http_status: error.status,
|
||||
}
|
||||
|
||||
if (error.error === 'AccountTakedown') {
|
||||
return classification(
|
||||
'account-takedown',
|
||||
'account-takedown',
|
||||
false,
|
||||
xrpcTags,
|
||||
)
|
||||
}
|
||||
|
||||
if (error.message.startsWith('Invalid reason type')) {
|
||||
return classification(
|
||||
'invalid-reason-type',
|
||||
'invalid-reason-type',
|
||||
true,
|
||||
xrpcTags,
|
||||
)
|
||||
}
|
||||
|
||||
if (error.message === 'Failed to perform upstream request') {
|
||||
return classification(
|
||||
'service-unavailable',
|
||||
'upstream-fetch',
|
||||
true,
|
||||
xrpcTags,
|
||||
)
|
||||
}
|
||||
|
||||
if (error.message === 'Internal Server Error') {
|
||||
return classification(
|
||||
'service-unavailable',
|
||||
'upstream-internal',
|
||||
true,
|
||||
xrpcTags,
|
||||
)
|
||||
}
|
||||
|
||||
const upstreamStatus = error.message.match(
|
||||
/^Upstream server responded with a (\d{3}) error$/,
|
||||
)?.[1]
|
||||
if (upstreamStatus) {
|
||||
return classification(
|
||||
isRetryableHttpStatus(Number(upstreamStatus))
|
||||
? 'service-unavailable'
|
||||
: 'unexpected',
|
||||
`upstream-http-${upstreamStatus}`,
|
||||
true,
|
||||
xrpcTags,
|
||||
)
|
||||
}
|
||||
|
||||
if (shouldRetryError(error)) {
|
||||
return classification(
|
||||
'service-unavailable',
|
||||
`xrpc-retryable-${error.status}`,
|
||||
true,
|
||||
xrpcTags,
|
||||
)
|
||||
}
|
||||
|
||||
return classification(
|
||||
'unexpected',
|
||||
`xrpc-other-${error.status}`,
|
||||
true,
|
||||
xrpcTags,
|
||||
)
|
||||
}
|
||||
|
||||
function classification(
|
||||
kind: ReportErrorKind,
|
||||
bucket: string,
|
||||
shouldReport: boolean,
|
||||
tags: Record<string, string | number> = {},
|
||||
): ReportErrorClassification {
|
||||
return {
|
||||
kind,
|
||||
shouldReport,
|
||||
fingerprint: ['{{ default }}', `report-dialog:${bucket}`],
|
||||
tags: {
|
||||
report_error_kind: kind,
|
||||
report_error_bucket: bucket,
|
||||
...tags,
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -45,6 +45,7 @@ import {
|
||||
SUPPORT_PAGE,
|
||||
} from './const'
|
||||
import {useCopyForSubject} from './copy'
|
||||
import {classifyReportError} from './errors'
|
||||
import {
|
||||
getNciiQualificationOutcome,
|
||||
initialState,
|
||||
@@ -244,14 +245,39 @@ function Inner(props: ReportDialogProps) {
|
||||
})
|
||||
}, 1e3)
|
||||
} catch (err) {
|
||||
const e = err as Error
|
||||
const e = err instanceof Error ? err : new Error(String(err))
|
||||
const classification = classifyReportError(e)
|
||||
const tags = {
|
||||
...classification.tags,
|
||||
report_subject_type: props.subject.type,
|
||||
report_labeler: state.selectedLabeler?.creator.did,
|
||||
report_reason: state.selectedOption?.reason,
|
||||
}
|
||||
|
||||
ax.metric('reportDialog:failure', {})
|
||||
logger.error(e, {
|
||||
source: 'ReportDialog',
|
||||
})
|
||||
|
||||
if (classification.shouldReport) {
|
||||
logger.error(e, {
|
||||
source: 'ReportDialog',
|
||||
fingerprint: classification.fingerprint,
|
||||
tags,
|
||||
})
|
||||
} else {
|
||||
logger.warn('Report rejected for taken down account', {tags})
|
||||
}
|
||||
|
||||
let error = l`Something went wrong. Please try again.`
|
||||
if (classification.kind === 'account-takedown') {
|
||||
error = l`Your account cannot submit reports while it is taken down.`
|
||||
} else if (classification.kind === 'invalid-reason-type') {
|
||||
error = l`This moderation service does not support that report reason. Please choose a different reason or moderation service.`
|
||||
} else if (classification.kind === 'service-unavailable') {
|
||||
error = l`The moderation service is temporarily unavailable. Please try again later.`
|
||||
}
|
||||
|
||||
dispatch({
|
||||
type: 'setError',
|
||||
error: l`Something went wrong. Please try again.`,
|
||||
error,
|
||||
})
|
||||
} finally {
|
||||
setIsPending(false)
|
||||
|
||||
@@ -242,6 +242,20 @@ describe('general functionality', () => {
|
||||
__context__: 'logger',
|
||||
},
|
||||
})
|
||||
|
||||
const fingerprint = ['{{ default }}', 'report-dialog:upstream-fetch']
|
||||
sentryTransport(
|
||||
LogLevel.Error,
|
||||
Logger.Context.ReportDialog,
|
||||
e,
|
||||
{fingerprint},
|
||||
timestamp,
|
||||
)
|
||||
expect(Sentry.captureException).toHaveBeenLastCalledWith(e, {
|
||||
tags: {category: 'report-dialog'},
|
||||
extra: {__context__: 'report-dialog'},
|
||||
fingerprint,
|
||||
})
|
||||
})
|
||||
|
||||
test('sentryTransport serializes errors', () => {
|
||||
|
||||
@@ -7,7 +7,7 @@ export const sentryTransport: Transport = (
|
||||
level,
|
||||
context,
|
||||
message,
|
||||
{type, tags, ...metadata},
|
||||
{type, tags, fingerprint, ...metadata},
|
||||
timestamp,
|
||||
) => {
|
||||
// Skip debug messages entirely for now - esb
|
||||
@@ -70,6 +70,7 @@ export const sentryTransport: Transport = (
|
||||
level: severity,
|
||||
tags: _tags,
|
||||
extra: meta,
|
||||
...(fingerprint ? {fingerprint} : {}),
|
||||
})
|
||||
}
|
||||
} else {
|
||||
@@ -84,6 +85,7 @@ export const sentryTransport: Transport = (
|
||||
Sentry.captureException(message, {
|
||||
tags: _tags,
|
||||
extra: meta,
|
||||
...(fingerprint ? {fingerprint} : {}),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -82,6 +82,12 @@ export type Metadata = {
|
||||
[key: string]: number | string | boolean | null | undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* Passed through to Sentry as a custom fingerprint. Include
|
||||
* `{{ default }}` to preserve Sentry's default grouping and add dimensions.
|
||||
*/
|
||||
fingerprint?: string[]
|
||||
|
||||
/**
|
||||
* Any additional data, passed through to Sentry as `extra` param on
|
||||
* exceptions, or the `data` param on breadcrumbs.
|
||||
|
||||
Reference in New Issue
Block a user