filter network errors globally in sentry

src/lib/network-error.test.ts

@@ -0,0 +1,39 @@
+import {describe, expect, test} from '@jest/globals'
+
+import {isNetworkError} from '#/lib/network-error'
+
+describe('isNetworkError', () => {
+  test.each([
+    'Network request failed',
+    'TypeError: Failed to fetch',
+    'Error: fetch failed: java.net.UnknownHostException',
+    'The Internet connection appears to be offline',
+    'The network connection was lost',
+    'A server with the specified hostname could not be found',
+    'TypeError: Network request timed out',
+  ])('detects %s', message => {
+    expect(isNetworkError(new Error(message))).toBe(true)
+  })
+
+  test('checks wrapped error causes', () => {
+    const error = new Error('Unable to fulfill XRPC request', {
+      cause: new Error('fetch failed: connection closed'),
+    })
+
+    expect(isNetworkError(error)).toBe(true)
+  })
+
+  test('does not treat arbitrary fetch-handler failures as network errors', () => {
+    expect(
+      isNetworkError(
+        new Error(
+          'Unexpected fetchHandler() error: URL.canParse is not a function',
+        ),
+      ),
+    ).toBe(false)
+  })
+
+  test('does not match words ending in load', () => {
+    expect(isNetworkError(new Error('Multipart upload failed'))).toBe(false)
+  })
+})

src/lib/network-error.ts

@@ -0,0 +1,39 @@
+const NETWORK_ERROR_PATTERNS = [
+  /\babort(?:ed|error)?\b/i,
+  /network request failed/i,
+  /failed to fetch/i,
+  /fetch failed/i,
+  /\bload failed\b/i,
+  /upstream service unreachable/i,
+  /networkerror when attempting to fetch resource/i,
+  /internet connection appears to be offline/i,
+  /network connection was lost/i,
+  /unknownhostexception/i,
+  /unable to resolve host/i,
+  /server with the specified hostname could not be found/i,
+  /network request timed out/i,
+  /connectexception: failed to connect/i,
+  /sslhandshakeexception: connection closed/i,
+]
+
+/**
+ * Detects transport failures across the error strings produced by web, native
+ * fetch, and the XRPC clients. Error causes are checked because the XRPC
+ * clients wrap the platform-specific fetch error.
+ */
+export function isNetworkError(value: unknown): boolean {
+  return isNetworkErrorInner(value, new Set())
+}
+
+function isNetworkErrorInner(value: unknown, seen: Set<object>): boolean {
+  if (NETWORK_ERROR_PATTERNS.some(pattern => pattern.test(String(value)))) {
+    return true
+  }
+
+  if (typeof value !== 'object' || value === null || seen.has(value)) {
+    return false
+  }
+
+  seen.add(value)
+  return 'cause' in value && isNetworkErrorInner(value.cause, seen)
+}

src/lib/strings/errors.ts

@@ -1,8 +1,11 @@
 import {LexError} from '@atproto/lex'
 import {t} from '@lingui/core/macro'
 
+import {isNetworkError} from '#/lib/network-error'
 import {isXrpcError} from '#/lib/xrpc-error'
 
+export {isNetworkError} from '#/lib/network-error'
+
 /**
  * The text to show the user when no special case applies.
  *
@@ -79,26 +82,6 @@ export function cleanError(e: unknown): string {
   return toDisplayString(e, str)
 }
 
-const NETWORK_ERRORS = [
-  'Abort',
-  'Network request failed',
-  'Failed to fetch',
-  'fetch failed',
-  'Load failed',
-  'Upstream service unreachable',
-  'NetworkError when attempting to fetch resource',
-]
-
-export function isNetworkError(e: unknown) {
-  const str = String(e)
-  for (const err of NETWORK_ERRORS) {
-    if (str.includes(err)) {
-      return true
-    }
-  }
-  return false
-}
-
 /**
  * The PDS answers an app-password-scope rejection with the lexicon code
  * `InvalidToken` and a message of 'Bad token scope' or 'Bad token method'

src/logger/__tests__/logger.test.ts

@@ -301,6 +301,15 @@ describe('general functionality', () => {
       timestamp,
     )
 
+    // call sites do not consistently use the same metadata key
+    sentryTransport(
+      LogLevel.Error,
+      Logger.Context.Default,
+      'request failed',
+      {underlyingError: new Error('fetch failed: connection closed')},
+      timestamp,
+    )
+
     jest.runAllTimers()
     expect(Sentry.captureMessage).not.toHaveBeenCalled()
 
@@ -312,6 +321,15 @@ describe('general functionality', () => {
       {},
       timestamp,
     )
+
+    // network error in metadata with an Error object message
+    sentryTransport(
+      LogLevel.Error,
+      Logger.Context.Default,
+      new Error('request failed'),
+      {underlyingError: new Error('fetch failed: connection closed')},
+      timestamp,
+    )
     expect(Sentry.captureException).not.toHaveBeenCalled()
   })
 

src/logger/sentry/network-errors.test.ts

@@ -0,0 +1,102 @@
+import {type Procedure, XrpcFetchError, XrpcResponseError} from '@atproto/lex'
+import {describe, expect, test} from '@jest/globals'
+import {type ErrorEvent} from '@sentry/react-native'
+
+import {
+  dropExpectedNetworkErrors,
+  isExpectedSentryNetworkError,
+} from '#/logger/sentry/network-errors'
+
+const method = {
+  nsid: 'app.bsky.test.getTest',
+  type: 'query',
+} as unknown as Procedure
+
+function xrpcResponseError(status: number) {
+  return new XrpcResponseError(method, new Response(null, {status}), undefined)
+}
+
+describe('isExpectedSentryNetworkError', () => {
+  test.each([502, 503, 504])('detects transient upstream status %s', status => {
+    expect(isExpectedSentryNetworkError(xrpcResponseError(status))).toBe(true)
+  })
+
+  test.each([429, 500])('keeps actionable HTTP status %s', status => {
+    expect(isExpectedSentryNetworkError(xrpcResponseError(status))).toBe(false)
+  })
+
+  test('checks an XRPC fetch error cause', () => {
+    expect(
+      isExpectedSentryNetworkError(
+        new XrpcFetchError(method, new Error('fetch failed: connection reset')),
+      ),
+    ).toBe(true)
+  })
+
+  test('keeps implementation errors wrapped by XRPC fetch errors', () => {
+    expect(
+      isExpectedSentryNetworkError(
+        new XrpcFetchError(
+          method,
+          new TypeError('URL.canParse is not a function'),
+        ),
+      ),
+    ).toBe(false)
+  })
+})
+
+describe('dropExpectedNetworkErrors', () => {
+  test('drops automatic captures using the original exception', () => {
+    const event = {type: undefined} satisfies ErrorEvent
+
+    expect(
+      dropExpectedNetworkErrors(event, {
+        originalException: new Error('fetch failed: connection closed'),
+      }),
+    ).toBeNull()
+  })
+
+  test('checks every exception in a linked error chain', () => {
+    const event = {
+      type: undefined,
+      exception: {
+        values: [
+          {type: 'Error', value: 'Network request failed'},
+          {
+            type: 'XrpcInternalError',
+            value: 'Unable to fulfill XRPC request',
+          },
+        ],
+      },
+    } satisfies ErrorEvent
+
+    expect(dropExpectedNetworkErrors(event, {})).toBeNull()
+  })
+
+  test('drops serialized transient upstream errors', () => {
+    const event = {
+      type: undefined,
+      exception: {
+        values: [{type: 'XrpcResponseError', value: 'Upstream Timeout'}],
+      },
+    } satisfies ErrorEvent
+
+    expect(dropExpectedNetworkErrors(event, {})).toBeNull()
+  })
+
+  test('keeps non-network events', () => {
+    const event = {
+      type: undefined,
+      exception: {
+        values: [
+          {
+            type: 'XrpcFetchError',
+            value: 'URL.canParse is not a function',
+          },
+        ],
+      },
+    } satisfies ErrorEvent
+
+    expect(dropExpectedNetworkErrors(event, {})).toBe(event)
+  })
+})

src/logger/sentry/network-errors.ts

@@ -0,0 +1,56 @@
+import {XrpcResponseError} from '@atproto/lex'
+import {type ErrorEvent} from '@sentry/react-native'
+
+import {isNetworkError} from '#/lib/network-error'
+
+const TRANSIENT_UPSTREAM_ERRORS = new Set([
+  'UpstreamFailure',
+  'NotEnoughResources',
+  'UpstreamTimeout',
+])
+
+const TRANSIENT_UPSTREAM_MESSAGES = [
+  /upstream\s*failure/i,
+  /not\s*enough\s*resources/i,
+  /upstream\s*timeout/i,
+  /operation timed out, please try again/i,
+]
+
+/**
+ * Sentry should not report expected transport failures or transient upstream
+ * availability errors. Keep this narrower than `XrpcError.shouldRetry()`:
+ * fetch handlers can wrap implementation bugs, while 429s and generic 500s
+ * can reveal actionable client or server regressions.
+ */
+export function isExpectedSentryNetworkError(value: unknown): boolean {
+  if (isNetworkError(value)) {
+    return true
+  }
+
+  if (
+    value instanceof XrpcResponseError &&
+    TRANSIENT_UPSTREAM_ERRORS.has(value.error)
+  ) {
+    return true
+  }
+
+  const message = String(value)
+  return TRANSIENT_UPSTREAM_MESSAGES.some(pattern => pattern.test(message))
+}
+
+/** Global backstop for automatic captures that bypass the logger transport. */
+export function dropExpectedNetworkErrors(
+  event: ErrorEvent,
+  hint: {originalException?: unknown},
+): ErrorEvent | null {
+  const candidates: unknown[] = [hint.originalException, event.message]
+
+  for (const exception of event.exception?.values ?? []) {
+    candidates.push(exception.value)
+    if (exception.type && exception.value) {
+      candidates.push(`${exception.type}: ${exception.value}`)
+    }
+  }
+
+  return candidates.some(isExpectedSentryNetworkError) ? null : event
+}

src/logger/sentry/setup/index.ts

@@ -1,5 +1,6 @@
 import {getGlobalScope, init} from '@sentry/react-native'
 
+import {dropExpectedNetworkErrors} from '#/logger/sentry/network-errors'
 import * as env from '#/env'
 
 init({
@@ -10,6 +11,7 @@ init({
   environment: env.ENV,
   dist: env.BUNDLE_IDENTIFIER,
   release: env.RELEASE_VERSION,
+  beforeSend: dropExpectedNetworkErrors,
   ignoreErrors: [
     /*
      * Unknown internals errors

src/logger/transports/sentry.ts

@@ -1,5 +1,5 @@
-import {isNetworkError} from '#/lib/strings/errors'
 import {Sentry} from '#/logger/sentry/lib'
+import {isExpectedSentryNetworkError} from '#/logger/sentry/network-errors'
 import {LogLevel, type Transport} from '#/logger/types'
 import {prepareMetadata} from '#/logger/util'
 
@@ -47,14 +47,14 @@ export const sentryTransport: Transport = (
       timestamp: timestamp / 1000, // Sentry expects seconds
     })
 
-    // We don't want to send any network errors to sentry. The underlying
-    // cause is often passed via metadata rather than the message itself, so
-    // check the common metadata keys too.
+    /*
+     * Keep the breadcrumb, but don't send expected network failures as events.
+     * The underlying cause is often passed in metadata rather than the message
+     * itself, and call sites do not consistently use the same metadata key.
+     */
     if (
-      isNetworkError(message) ||
-      isNetworkError(metadata.safeMessage) ||
-      isNetworkError(metadata.message) ||
-      isNetworkError(metadata.error)
+      isExpectedSentryNetworkError(message) ||
+      Object.values(metadata).some(isExpectedSentryNetworkError)
     ) {
       return
     }
@@ -74,8 +74,10 @@ export const sentryTransport: Transport = (
       })
     }
   } else {
-    // We don't want to send any network errors to sentry
-    if (isNetworkError(message)) {
+    if (
+      isExpectedSentryNetworkError(message) ||
+      Object.values(metadata).some(isExpectedSentryNetworkError)
+    ) {
       return
     }