From 8bcd8bfa14deb700cd7f56138a15812cabc159d9 Mon Sep 17 00:00:00 2001 From: Samuel Newman Date: Mon, 3 Aug 2026 14:50:07 +0300 Subject: [PATCH] classify report dialog errors --- .../moderation/ReportDialog/errors.test.ts | 107 +++++++++++++++++ .../moderation/ReportDialog/errors.ts | 111 ++++++++++++++++++ .../moderation/ReportDialog/index.tsx | 36 +++++- src/logger/__tests__/logger.test.ts | 14 +++ src/logger/transports/sentry.ts | 4 +- src/logger/types.ts | 6 + 6 files changed, 272 insertions(+), 6 deletions(-) create mode 100644 src/components/moderation/ReportDialog/errors.test.ts create mode 100644 src/components/moderation/ReportDialog/errors.ts diff --git a/src/components/moderation/ReportDialog/errors.test.ts b/src/components/moderation/ReportDialog/errors.test.ts new file mode 100644 index 0000000000..4a469c5eb2 --- /dev/null +++ b/src/components/moderation/ReportDialog/errors.test.ts @@ -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'], + }) + }) +}) diff --git a/src/components/moderation/ReportDialog/errors.ts b/src/components/moderation/ReportDialog/errors.ts new file mode 100644 index 0000000000..2dc71c8d23 --- /dev/null +++ b/src/components/moderation/ReportDialog/errors.ts @@ -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 +} + +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 = {}, +): ReportErrorClassification { + return { + kind, + shouldReport, + fingerprint: ['{{ default }}', `report-dialog:${bucket}`], + tags: { + report_error_kind: kind, + report_error_bucket: bucket, + ...tags, + }, + } +} diff --git a/src/components/moderation/ReportDialog/index.tsx b/src/components/moderation/ReportDialog/index.tsx index dfb3e781a9..50c780630e 100644 --- a/src/components/moderation/ReportDialog/index.tsx +++ b/src/components/moderation/ReportDialog/index.tsx @@ -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) diff --git a/src/logger/__tests__/logger.test.ts b/src/logger/__tests__/logger.test.ts index d7bda5ad23..ed46301f6e 100644 --- a/src/logger/__tests__/logger.test.ts +++ b/src/logger/__tests__/logger.test.ts @@ -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', () => { diff --git a/src/logger/transports/sentry.ts b/src/logger/transports/sentry.ts index 1764e91541..92a7b92f67 100644 --- a/src/logger/transports/sentry.ts +++ b/src/logger/transports/sentry.ts @@ -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} : {}), }) } } diff --git a/src/logger/types.ts b/src/logger/types.ts index cc700dc58b..7d3c897477 100644 --- a/src/logger/types.ts +++ b/src/logger/types.ts @@ -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.