trim comments

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

src/lib/network-error.ts

@@ -1,10 +1,8 @@
 const NETWORK_ERROR_PATTERNS = [
   /*
-   * Case-sensitive on purpose. `AbortError` (web `DOMException`) and the
+   * Case-sensitive: `AbortError` and `Aborted` are cancellations, but the
-   * `Aborted` message thrown by `#/lib/async/cancelable` are transport
+   * lowercase word appears in unrelated failures ("Multipart upload aborted")
-   * cancellations, but the lowercase word shows up in unrelated failures - a
+   * that must still be reported.
-   * multipart upload the service marked `aborted`, or a `TypeError` naming
-   * `abortController.abort` - which must still be reported.
    */
   /\bAbort(?:ed|Error)?\b/,
   /network request failed/i,
@@ -24,12 +22,9 @@ const NETWORK_ERROR_PATTERNS = [
 ]
 
 /**
- * `String(value)` runs code we do not control - a custom `toString`, a
+ * `String()` can throw - null-prototype objects, custom `toString`, proxies -
- * `Symbol.toPrimitive`, a `Proxy` trap - and throws outright for a
+ * so an unreadable value counts as "not a network error" rather than crashing
- * null-prototype object, so stringifying an arbitrary thrown value can itself
+ * the error path that asked.
- * throw. Returning `undefined` lets callers treat an unreadable value as "not a
- * network error" rather than letting the throw escape `logger.error()` or
- * Sentry's `beforeSend`, where it would lose the original report.
  */
 export function safeStringify(value: unknown): string | undefined {
   try {
@@ -45,9 +40,7 @@ export function safeStringify(value: unknown): string | undefined {
  * fetch, and the XRPC clients. Error causes are checked because the XRPC
  * clients wrap the platform-specific fetch error.
  *
- * Never throws: reading `cause` off a hostile value can throw just like
+ * Never throws; see {@link safeStringify}.
- * stringifying it can, and a throw here would escape whatever error path is
- * asking the question.
  */
 export function isNetworkError(value: unknown): boolean {
   try {
@@ -57,10 +50,6 @@ export function isNetworkError(value: unknown): boolean {
   }
 }
 
-/**
- * `seen` stays undefined until the first recursion into an object cause, so the
- * common case (a string or an error with no cause) allocates nothing.
- */
 function isNetworkErrorInner(
   value: unknown,
   seen: Set<object> | undefined,

src/lib/strings/errors.ts

@@ -39,11 +39,7 @@ export function cleanError(e: unknown): string {
    */
   // oxlint-disable-next-line typescript/no-base-to-string
   const str = typeof e === 'string' ? e : e.toString()
-  /*
+  // the original value, not `str`, so wrapped causes are checked
-   * Passed the original value, not `str`, so the wrapped-cause walk is
-   * reachable: the XRPC clients report the platform fetch failure as the
-   * `cause` of a generic message.
-   */
   if (isNetworkError(e)) {
     return t`Unable to connect. Please check your internet connection and try again.`
   }

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

@@ -15,10 +15,7 @@ function statusOnlyError(status: number) {
   return new XrpcResponseError(method, new Response(null, {status}), undefined)
 }
 
-/**
+/** A pipethrough `XrpcResponseError`: status rewritten, code forwarded. */
- * An `XrpcResponseError` as the PDS pipethrough produces it: the status is
- * rewritten while the upstream lexicon code is forwarded verbatim.
- */
 function payloadError(status: number, error: string, message: string) {
   const body = {error, message}
   return new XrpcResponseError(
@@ -92,10 +89,7 @@ describe('isExpectedSentryNetworkError', () => {
 
 describe('dropExpectedNetworkErrors', () => {
   test('drops automatic captures using the original exception', () => {
-    /*
+    // the serialized values carry no cause chain, only the hint has the failure
-     * The serialized values do not carry the cause chain, so the hint is the
-     * only place the transport failure is visible.
-     */
     const event = exceptionEvent(
       'XrpcInternalError',
       'Unable to fulfill XRPC request',

src/logger/sentry/network-errors.ts

@@ -4,29 +4,19 @@ import {type ErrorEvent} from '@sentry/react-native'
 import {isNetworkError, safeStringify} from '#/lib/network-error'
 
 /**
- * HTTP statuses that mean an upstream service was unavailable.
+ * Upstream-unavailable statuses. Matched on the status, not the lexicon
- *
+ * code: the PDS pipethrough rewrites the status to 502 but forwards the
- * Matched on the status rather than the lexicon code because the code is not
+ * upstream code verbatim, so a 502 can arrive named `InternalServerError`.
- * reliable here: the PDS pipethrough rewrites the status to 502 while
+ * `#/lib/xrpc-error` is deliberately not imported - it pulls `#/lexicons`
- * forwarding the upstream error code verbatim, so an upstream 500 arrives as a
+ * into the module graph evaluated before `Sentry.init()`.
- * 502 still named `InternalServerError`. The status is authoritative.
- *
- * `#/lib/xrpc-error` is deliberately not used for the `instanceof` check - it
- * imports `#/lexicons`, which would pull the whole lexicon graph into the
- * module evaluated before `Sentry.init()`.
  */
 const TRANSIENT_UPSTREAM_STATUSES = new Set([502, 503, 504])
 
 /**
- * Sentry serializes an exception as `{type: error.name, value: error.message}`,
+ * Sentry serializes `error.message`, not `toString()`, so these match the
- * so these match the message text rather than the `toString()`. Lex builds that
+ * message text: the payload message ("Upstream Timeout"), the lexicon code
- * message from the server's error payload ("Upstream Timeout") or, when the
+ * ("UpstreamFailure"), and lex's payload-less "Upstream server responded with
- * response carries no JSON payload, from the status alone ("Upstream server
+ * a 504 error". Word boundaries keep camelCase identifiers from matching.
- * responded with a 504 error").
- *
- * The optional space matches the spaced prose and the space-free lexicon code
- * ("UpstreamFailure") alike, and the word boundaries keep camelCase
- * identifiers such as `upstreamTimeoutV2` from matching.
  */
 const TRANSIENT_UPSTREAM_MESSAGES = [
   /\bupstream\s?failure\b/i,
@@ -64,11 +54,9 @@ export function isExpectedSentryNetworkError(value: unknown): boolean {
 /**
  * Global backstop for automatic captures that bypass the logger transport.
  *
- * Only exception events are inspected. Sentry sets `hint.originalException` to
+ * Message events are skipped: Sentry sets `hint.originalException` to the
- * the message *string* for a `captureMessage`, and message events reach Sentry
+ * message string for `captureMessage`, and the only message producers are the
- * from exactly two places: the logger transport, which has already run this
+ * logger transport (already filtered) and user bug reports (never droppable).
- * check before capturing, and user bug reports, which must never be dropped
- * for happening to contain a word like "aborted" in the report slug.
  */
 export function dropExpectedNetworkErrors(
   event: ErrorEvent,

src/logger/transports/sentry.ts

@@ -3,20 +3,14 @@ import {isExpectedSentryNetworkError} from '#/logger/sentry/network-errors'
 import {LogLevel, type Metadata, type Transport} from '#/logger/types'
 import {prepareMetadata} from '#/logger/util'
 
-/**
+/** Keys that have historically carried the underlying failure as a plain string. */
- * Metadata keys that have historically carried the underlying failure as a
- * plain string.
- */
 const CHECKED_METADATA_KEYS = ['safeMessage', 'message', 'error']
 
 /**
  * Whether this log records a network failure Sentry should not be told about.
- *
+ * Call sites do not agree on a metadata key, so any `Error` value counts under
- * The failure is often passed in metadata rather than as the message, and call
+ * any key; other types only count under `CHECKED_METADATA_KEYS` - scanning all
- * sites do not consistently use the same key, so any `Error` value counts no
+ * metadata lets an unrelated string (a URL with "abort") suppress a real error.
- * matter which key holds it. Values of other types only count under the keys
- * above: metadata is arbitrary, and scanning all of it lets an unrelated string
- * - a stream URL containing "abort", say - suppress a real error.
  */
 function isExpectedNetworkFailure(
   message: string | Error,
@@ -45,11 +39,7 @@ export const sentryTransport: Transport = (
   // Skip debug messages entirely for now - esb
   if (level === LogLevel.Debug) return
 
-  /*
+  // ambient __metadata__ is kept out of the scan but stays in the attached data
-   * `__metadata__` is ambient context rather than something the call site
-   * passed, so it is destructured out of the scanned metadata and folded back
-   * in here to keep the attached data unchanged.
-   */
   const meta = {
     __context__: context,
     ...prepareMetadata(__metadata__ ? {__metadata__, ...metadata} : metadata),