Merge remote-tracking branch 'origin/main' into spence/enrich-hls-sentry-errors

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'],
+    })
+  })
+})

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<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,
+    },
+  }
+}

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)

src/lib/media/picker.tsx

@@ -17,11 +17,11 @@ export async function openCamera(customOpts: ImagePickerOptions) {
   }
   const res = await launchCameraAsync(opts)
 
-  if (!res || !res.assets) {
+  if (res.canceled) {
-    throw new Error('Camera was closed before taking a photo')
+    return
   }
 
-  const asset = res?.assets[0]
+  const asset = res.assets[0]
 
   return {
     path: asset.uri,

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', () => {

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} : {}),
     })
   }
 }

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.

src/view/com/composer/photos/OpenCameraBtn.tsx

@@ -32,6 +32,9 @@ export function OpenCameraBtn({disabled, onAdd}: OpenCameraBtnProps) {
       const img = await openCamera({
         aspect: [1, 1],
       })
+      if (!img) {
+        return
+      }
 
       // If we don't have permissions it's fine, we just wont save it. The post itself will still have access to
       // the image even without these permissions

src/view/com/feeds/ComposerPrompt.tsx

@@ -115,6 +115,9 @@ export function ComposerPrompt() {
       const image = await openCamera({
         mediaTypes: 'images',
       })
+      if (!image) {
+        return
+      }
 
       const imageUris = [
         {

src/view/com/notifications/NotificationFeedItem.tsx

@@ -250,7 +250,6 @@ let NotificationFeedItem = ({
           t.atoms.text,
           a.font_semi_bold,
           a.text_md,
-          a.leading_tight,
           web({direction: 'ltr', unicodeBidi: 'isolate'}),
         ]}
         to={firstAuthor.href}
@@ -260,18 +259,8 @@ let NotificationFeedItem = ({
         {forceLTR(firstAuthorName)}
         <ProfileBadges
           profile={firstAuthor.profile}
-          size="md"
+          size="sm"
-          style={[
+          style={[a.px_2xs, {transform: [{translateY: 1}]}]}
-            a.relative,
-            {
-              // weird stuff here
-              paddingTop: platform({android: 2}),
-              marginBottom: platform({ios: -6}),
-              top: platform({web: 2}),
-              paddingLeft: 3,
-              paddingRight: 2,
-            },
-          ]}
         />
       </InlineLinkText>
     </ProfileHoverCard>
@@ -315,7 +304,7 @@ let NotificationFeedItem = ({
     notificationContent = hasMultipleAuthors ? (
       <Trans>
         {firstAuthorLink} and{' '}
-        <Text style={[a.text_md, a.font_semi_bold, a.leading_snug]}>
+        <Text style={[a.text_md, a.font_semi_bold]}>
           <Plural
             value={additionalAuthorsCount}
             one={`${formattedAuthorsCount} other`}
@@ -337,7 +326,7 @@ let NotificationFeedItem = ({
     notificationContent = hasMultipleAuthors ? (
       <Trans>
         {firstAuthorLink} and{' '}
-        <Text style={[a.text_md, a.font_semi_bold, a.leading_snug]}>
+        <Text style={[a.text_md, a.font_semi_bold]}>
           <Plural
             value={additionalAuthorsCount}
             one={`${formattedAuthorsCount} other`}
@@ -377,7 +366,7 @@ let NotificationFeedItem = ({
       notificationContent = hasMultipleAuthors ? (
         <Trans>
           {firstAuthorLink} and{' '}
-          <Text style={[a.text_md, a.font_semi_bold, a.leading_snug]}>
+          <Text style={[a.text_md, a.font_semi_bold]}>
             <Plural
               value={additionalAuthorsCount}
               one={`${formattedAuthorsCount} other`}
@@ -409,7 +398,7 @@ let NotificationFeedItem = ({
     notificationContent = hasMultipleAuthors ? (
       <Trans>
         {firstAuthorLink} and{' '}
-        <Text style={[a.text_md, a.font_semi_bold, a.leading_snug]}>
+        <Text style={[a.text_md, a.font_semi_bold]}>
           <Plural
             value={additionalAuthorsCount}
             one={`${formattedAuthorsCount} other`}
@@ -431,7 +420,7 @@ let NotificationFeedItem = ({
     notificationContent = hasMultipleAuthors ? (
       <Trans>
         {firstAuthorLink} and{' '}
-        <Text style={[a.text_md, a.font_semi_bold, a.leading_snug]}>
+        <Text style={[a.text_md, a.font_semi_bold]}>
           <Plural
             value={additionalAuthorsCount}
             one={`${formattedAuthorsCount} other`}
@@ -458,7 +447,7 @@ let NotificationFeedItem = ({
     notificationContent = hasMultipleAuthors ? (
       <Trans>
         {firstAuthorLink} and{' '}
-        <Text style={[a.text_md, a.font_semi_bold, a.leading_snug]}>
+        <Text style={[a.text_md, a.font_semi_bold]}>
           <Plural
             value={additionalAuthorsCount}
             one={`${formattedAuthorsCount} other`}
@@ -481,7 +470,7 @@ let NotificationFeedItem = ({
     notificationContent = hasMultipleAuthors ? (
       <Trans>
         {firstAuthorLink} and{' '}
-        <Text style={[a.text_md, a.font_semi_bold, a.leading_snug]}>
+        <Text style={[a.text_md, a.font_semi_bold]}>
           <Plural
             value={additionalAuthorsCount}
             one={`${formattedAuthorsCount} other`}
@@ -506,7 +495,7 @@ let NotificationFeedItem = ({
     notificationContent = hasMultipleAuthors ? (
       <Trans>
         {firstAuthorLink} and{' '}
-        <Text style={[a.text_md, a.font_semi_bold, a.leading_snug]}>
+        <Text style={[a.text_md, a.font_semi_bold]}>
           <Plural
             value={additionalAuthorsCount}
             one={`${formattedAuthorsCount} other`}
@@ -528,7 +517,7 @@ let NotificationFeedItem = ({
     notificationContent = hasMultipleAuthors ? (
       <Trans>
         {firstAuthorLink} and{' '}
-        <Text style={[a.text_md, a.font_semi_bold, a.leading_snug]}>
+        <Text style={[a.text_md, a.font_semi_bold]}>
           <Plural
             value={additionalAuthorsCount}
             one={`${formattedAuthorsCount} other`}
@@ -558,7 +547,7 @@ let NotificationFeedItem = ({
     notificationContent = hasMultipleAuthors ? (
       <Trans>
         New posts from {firstAuthorLink} and{' '}
-        <Text style={[a.text_md, a.font_semi_bold, a.leading_snug]}>
+        <Text style={[a.text_md, a.font_semi_bold]}>
           <Plural
             value={additionalAuthorsCount}
             one={`${formattedAuthorsCount} other`}
@@ -662,7 +651,6 @@ let NotificationFeedItem = ({
                   {paddingTop: 6},
                   a.self_start,
                   a.text_md,
-                  a.leading_snug,
                 ]}
                 accessibilityHint=""
                 accessibilityLabel={a11yLabel}>
@@ -1171,7 +1159,7 @@ function AdditionalPostText({post}: {post?: AppBskyFeedDefs.PostView}) {
         {text?.length > 0 && (
           <Text
             emoji
-            style={[a.text_sm, a.leading_snug, t.atoms.text_contrast_medium]}
+            style={[a.text_sm, t.atoms.text_contrast_medium]}
             numberOfLines={MAX_POST_LINES}>
             {text}
           </Text>

src/view/com/util/UserAvatar.tsx

@@ -391,14 +391,14 @@ let EditableUserAvatar = ({
       return
     }
 
-    onSelectNewAvatar(
+    const image = await openCamera({
-      await compressIfNeeded(
+      aspect: [1, 1],
-        await openCamera({
+    })
-          aspect: [1, 1],
+    if (!image) {
-        }),
+      return
-        IMAGE_SIZE_CONFIG_2K_1MB,
+    }
-      ),
+
-    )
+    onSelectNewAvatar(await compressIfNeeded(image, IMAGE_SIZE_CONFIG_2K_1MB))
   }, [onSelectNewAvatar, requestCameraAccessIfNeeded])
 
   const onOpenLibrary = useCallback(async () => {

src/view/com/util/UserBanner.tsx

@@ -58,14 +58,14 @@ export function UserBanner({
     if (!(await requestCameraAccessIfNeeded())) {
       return
     }
-    onSelectNewBanner?.(
+    const image = await openCamera({
-      await compressIfNeeded(
+      aspect: [3, 1],
-        await openCamera({
+    })
-          aspect: [3, 1],
+    if (!image) {
-        }),
+      return
-        IMAGE_SIZE_CONFIG_2K_1MB,
+    }
-      ),
+
-    )
+    onSelectNewBanner?.(await compressIfNeeded(image, IMAGE_SIZE_CONFIG_2K_1MB))
   }, [onSelectNewBanner, requestCameraAccessIfNeeded])
 
   const onOpenLibrary = useCallback(async () => {