Refine link/embed resolution code

src/components/ComposerV2/store/index.ts

@@ -1,11 +1,12 @@
 import {type AppBskyFeedDefs, type AtpAgent} from '@atproto/api'
 import {nanoid} from 'nanoid/non-secure'
 
-import {type resolveLink} from '#/lib/api/resolve'
 import {
-  type LinkResolutionOutcome,
-  startUriResolution,
-} from '#/components/ComposerV2/store/linkResolution'
+  type ResolvedLink,
+  resolveLink as importedResolveLink,
+  type resolveLink,
+} from '#/lib/api/resolve'
+import {createPublicAgent} from '#/state/session/agent'
 import type * as types from '#/components/ComposerV2/store/types'
 import {
   startImageUpload,
@@ -18,6 +19,7 @@ import {classifyUriTarget} from '#/components/ComposerV2/store/utils/classifyUri
 import {computePostMediaSelectionsRemaining} from '#/components/ComposerV2/store/utils/computePostMediaSelectionsRemaining'
 import {createAsyncTaskRev} from '#/components/ComposerV2/store/utils/createAsyncTaskRev'
 import {filterMediaInputs} from '#/components/ComposerV2/store/utils/filterMediaInputs'
+import {parseResolveLinkError} from '#/components/ComposerV2/store/utils/parseResolveLinkError'
 
 type Listener = () => void
 
@@ -304,6 +306,7 @@ export function createThreadStore(options: {
     const post = state.posts[postId]
     if (!post) return
 
+    const resolve = resolveLinkOverride ?? importedResolveLink
     const target = classifyUriTarget(uri)
 
     if (target === 'quote') {
@@ -318,12 +321,10 @@ export function createThreadStore(options: {
         s.isDirty = true
         return s
       })
-      startUriResolution({
-        postId,
-        uri,
-        resolveLink: resolveLinkOverride,
-        onResolve: handleQuoteResolution(rev, uri),
-      })
+      resolve(createPublicAgent(), uri).then(
+        link => applyQuoteResolved(rev, postId, uri, link),
+        err => applyQuoteFailed(rev, postId, uri, err),
+      )
       return
     }
 
@@ -345,106 +346,114 @@ export function createThreadStore(options: {
       s.isDirty = true
       return s
     })
-    startUriResolution({
-      postId,
-      uri,
-      resolveLink: resolveLinkOverride,
-      onResolve: handleEmbedResolution(rev, uri),
+    resolve(createPublicAgent(), uri).then(
+      link => applyEmbedResolved(rev, postId, uri, link),
+      err => applyEmbedFailed(rev, postId, uri, err),
+    )
+  }
+
+  function applyQuoteResolved(
+    rev: number,
+    postId: string,
+    uri: string,
+    link: ResolvedLink,
+  ) {
+    if (destroyed) return
+    if (!quoteRev.isCurrentFor(postId, rev)) return
+    // Pre-classification said this was a post URL. If resolveLink disagrees
+    // (rare, since both use the same URL patterns), surface as a generic
+    // failure in the quote slot.
+    if (link.type !== 'record' || link.kind !== 'post') {
+      applyQuoteFailed(rev, postId, uri, new Error('Could not resolve post'))
+      return
+    }
+    mutateState(s => {
+      const p = s.posts[postId]
+      if (!p) return null
+      s.posts[postId] = setPostQuote(p, {
+        state: 'resolved',
+        uri: link.record.uri,
+        cid: link.record.cid,
+        view: link.view,
+      })
+      return s
     })
   }
 
-  function handleQuoteResolution(rev: number, uri: string) {
-    return (postId: string, outcome: LinkResolutionOutcome) => {
-      if (destroyed) return
-      if (!quoteRev.isCurrentFor(postId, rev)) return
-
-      // Pre-classification said this was a post URL. If resolveLink disagrees
-      // (deleted post, embedding disabled, network error, etc.), surface as
-      // failed in the quote slot. Non-retryable failure codes (e.g.
-      // embedding-disabled) get a failed state with no `retry()`; the user
-      // has to remove the embed manually.
-      if (outcome.kind !== 'post') {
-        const code: types.LinkResolutionFailureCode =
-          outcome.embed.state === 'failed' ? outcome.embed.code : 'unknown'
-        const error =
-          outcome.embed.state === 'failed'
-            ? outcome.embed.error
-            : 'Could not resolve post'
-        const failed: types.PostEmbedQuote = {
-          state: 'failed',
-          uri,
-          error,
-          code,
-          retry:
-            code === 'embedding-disabled'
-              ? undefined
-              : () => addUri(postId, uri),
-        }
-        mutateState(s => {
-          const p = s.posts[postId]
-          if (!p) return null
-          s.posts[postId] = setPostQuote(p, failed)
-          return s
-        })
-        return
-      }
-      mutateState(s => {
-        const p = s.posts[postId]
-        if (!p) return null
-        s.posts[postId] = setPostQuote(p, {
-          state: 'resolved',
-          uri: outcome.record.uri,
-          cid: outcome.record.cid,
-          view: outcome.view,
-        })
-        return s
+  function applyQuoteFailed(
+    rev: number,
+    postId: string,
+    uri: string,
+    err: unknown,
+  ) {
+    if (destroyed) return
+    if (!quoteRev.isCurrentFor(postId, rev)) return
+    // Non-retryable failure codes (e.g. embedding-disabled) get a failed
+    // state with no `retry()`; the user has to remove the embed manually.
+    const {code, isRetryable} = parseResolveLinkError(err)
+    mutateState(s => {
+      const p = s.posts[postId]
+      if (!p) return null
+      s.posts[postId] = setPostQuote(p, {
+        state: 'failed',
+        uri,
+        error: stringifyError(err),
+        code,
+        retry: isRetryable ? () => addUri(postId, uri) : undefined,
       })
-    }
+      return s
+    })
   }
 
-  function handleEmbedResolution(rev: number, uri: string) {
-    return (postId: string, outcome: LinkResolutionOutcome) => {
-      if (destroyed) return
-      if (!embedRev.isCurrentFor(postId, rev)) return
-
-      // Pre-classification said this was a non-post URL. If resolveLink
-      // surprises us with a post outcome, treat as failure rather than
-      // silently moving slots.
-      if (outcome.kind === 'post') {
-        const failed: types.PostEmbed = {
-          state: 'failed',
-          uri,
-          error: 'Unexpected post outcome for non-post URL',
-          code: 'unknown',
-          retry: () => addUri(postId, uri),
-        }
-        mutateState(s => {
-          const p = s.posts[postId]
-          if (!p) return null
-          s.posts[postId] = setPostEmbed(p, failed)
-          return s
-        })
-        return
-      }
-
-      const embed = outcome.embed
-      const stored: types.PostEmbed =
-        embed.state === 'failed'
-          ? {
-              ...embed,
-              retry:
-                embed.code === 'embedding-disabled'
-                  ? undefined
-                  : () => addUri(postId, embed.uri),
-            }
-          : embed
-      mutateState(s => {
-        const p = s.posts[postId]
-        if (!p) return null
-        s.posts[postId] = setPostEmbed(p, stored)
-        return s
-      })
+  function applyEmbedResolved(
+    rev: number,
+    postId: string,
+    uri: string,
+    link: ResolvedLink,
+  ) {
+    if (destroyed) return
+    if (!embedRev.isCurrentFor(postId, rev)) return
+    // Pre-classification said this was a non-post URL. If resolveLink
+    // surprises us with a post outcome, treat as failure rather than
+    // silently moving slots.
+    if (link.type === 'record' && link.kind === 'post') {
+      applyEmbedFailed(
+        rev,
+        postId,
+        uri,
+        new Error('Unexpected post outcome for non-post URL'),
+      )
+      return
     }
+    mutateState(s => {
+      const p = s.posts[postId]
+      if (!p) return null
+      s.posts[postId] = setPostEmbed(p, resolvedLinkToEmbed(link))
+      return s
+    })
+  }
+
+  function applyEmbedFailed(
+    rev: number,
+    postId: string,
+    uri: string,
+    err: unknown,
+  ) {
+    if (destroyed) return
+    if (!embedRev.isCurrentFor(postId, rev)) return
+    const {code, isRetryable} = parseResolveLinkError(err)
+    mutateState(s => {
+      const p = s.posts[postId]
+      if (!p) return null
+      s.posts[postId] = setPostEmbed(p, {
+        state: 'failed',
+        uri,
+        error: stringifyError(err),
+        code,
+        retry: isRetryable ? () => addUri(postId, uri) : undefined,
+      })
+      return s
+    })
   }
 
   function removeEmbed(postId: string) {
@@ -582,6 +591,36 @@ export function createThreadStore(options: {
     return {...post, quote}
   }
 
+  /**
+   * Map a non-post ResolvedLink into the PostEmbed shape stored on the post.
+   * Callers handle the post-record case separately (those go to quote).
+   */
+  function resolvedLinkToEmbed(link: ResolvedLink): types.PostEmbed {
+    if (link.type === 'external') {
+      return {
+        state: 'external',
+        uri: link.uri,
+        title: link.title,
+        description: link.description,
+        thumb: link.thumb,
+      }
+    }
+    switch (link.kind) {
+      case 'feed':
+        return {state: 'feed', record: link.record, view: link.view}
+      case 'list':
+        return {state: 'list', record: link.record, view: link.view}
+      case 'starter-pack':
+        return {state: 'starter-pack', record: link.record, view: link.view}
+      case 'post':
+        throw new Error('post records should route to quote, not embed')
+    }
+  }
+
+  function stringifyError(err: unknown): string {
+    return String((err && (err as Error).message) ?? err)
+  }
+
   return {
     actions: {
       setPostText,

src/components/ComposerV2/store/linkResolution.ts

@@ -1,146 +0,0 @@
-/**
- * Link metadata resolution worker for the ComposerV2 store.
- *
- * Runs `resolveLink` (which classifies a URI as a post / feed / list /
- * starter-pack record or as an external link card) and reports the outcome
- * back via the `onResolve` callback. The store routes `kind: 'post'` outcomes
- * to the post's `quote` field and everything else to the `embed` field.
- *
- * No cancellation surface: cancellation is handled at the store level by
- * incrementing a per-post generation counter and ignoring stale callbacks.
- * The `__resolveLink` test seam (passed through from createThreadStore)
- * lets tests inject a controllable promise.
- */
-import {
-  type AppBskyFeedDefs,
-  type AppBskyGraphDefs,
-  type ComAtprotoRepoStrongRef,
-} from '@atproto/api'
-
-import {
-  EmbeddingDisabledError,
-  type ResolvedLink,
-  type resolveLink as defaultResolveLink,
-} from '#/lib/api/resolve'
-import {resolveLink as importedResolveLink} from '#/lib/api/resolve'
-import {type ComposerImage} from '#/state/gallery'
-import {createPublicAgent} from '#/state/session/agent'
-import {type LinkResolutionFailureCode} from './types'
-
-/**
- * What the worker (or a test) reports back about a resolved URI. `pending`
- * is not part of this type because the worker only emits terminal outcomes;
- * the synchronous pending state is set by the store itself before the
- * worker runs.
- */
-export type EmbedResolution =
-  | {
-      state: 'failed'
-      uri: string
-      error: string
-      code: LinkResolutionFailureCode
-    }
-  | {
-      state: 'external'
-      uri: string
-      title: string
-      description: string
-      thumb: ComposerImage | undefined
-    }
-  | {
-      state: 'feed'
-      record: ComAtprotoRepoStrongRef.Main
-      view: AppBskyFeedDefs.GeneratorView
-    }
-  | {
-      state: 'list'
-      record: ComAtprotoRepoStrongRef.Main
-      view: AppBskyGraphDefs.ListView
-    }
-  | {
-      state: 'starter-pack'
-      record: ComAtprotoRepoStrongRef.Main
-      view: AppBskyGraphDefs.StarterPackView
-    }
-
-/**
- * Worker output for a single URI. The store routes `kind: 'post'` to the
- * post's `quote` field and everything else to the `embed` field.
- */
-export type LinkResolutionOutcome =
-  | {
-      kind: 'post'
-      record: ComAtprotoRepoStrongRef.Main
-      view: AppBskyFeedDefs.PostView
-    }
-  | {kind: 'embed'; embed: EmbedResolution}
-
-export type StartUriResolutionOptions = {
-  postId: string
-  uri: string
-  /** Test seam; defaults to the imported resolveLink. */
-  resolveLink?: typeof defaultResolveLink
-  onResolve: (postId: string, outcome: LinkResolutionOutcome) => void
-}
-
-export function startUriResolution(opts: StartUriResolutionOptions): void {
-  const fn = opts.resolveLink ?? importedResolveLink
-  fn(createPublicAgent(), opts.uri).then(
-    link => opts.onResolve(opts.postId, mapResolvedLink(link)),
-    err =>
-      opts.onResolve(opts.postId, {
-        kind: 'embed',
-        embed: {
-          state: 'failed',
-          uri: opts.uri,
-          error: String((err && (err as Error).message) ?? err),
-          code: parseErrorCode(err),
-        },
-      }),
-  )
-}
-
-/**
- * Classify a thrown error into a stable failure code that drives UI
- * behavior. Today we only special-case EmbeddingDisabledError (which
- * `resolveLink` throws when fetching a post the author has marked
- * non-embeddable); everything else falls into 'unknown' and is retryable.
- */
-export function parseErrorCode(err: unknown): LinkResolutionFailureCode {
-  if (err instanceof EmbeddingDisabledError) return 'embedding-disabled'
-  return 'unknown'
-}
-
-function mapResolvedLink(link: ResolvedLink): LinkResolutionOutcome {
-  if (link.type === 'external') {
-    return {
-      kind: 'embed',
-      embed: {
-        state: 'external',
-        uri: link.uri,
-        title: link.title,
-        description: link.description,
-        thumb: link.thumb,
-      },
-    }
-  }
-  if (link.kind === 'post') {
-    return {kind: 'post', record: link.record, view: link.view}
-  }
-  if (link.kind === 'feed') {
-    return {
-      kind: 'embed',
-      embed: {state: 'feed', record: link.record, view: link.view},
-    }
-  }
-  if (link.kind === 'list') {
-    return {
-      kind: 'embed',
-      embed: {state: 'list', record: link.record, view: link.view},
-    }
-  }
-  return {
-    kind: 'embed',
-    embed: {state: 'starter-pack', record: link.record, view: link.view},
-  }
-}

src/components/ComposerV2/store/utils/parseResolveLinkError.ts

@@ -0,0 +1,26 @@
+import {EmbeddingDisabledError} from '#/lib/api/resolve'
+import {type LinkResolutionFailureCode} from '#/components/ComposerV2/store/types'
+
+export type ParsedResolveLinkError = {
+  code: LinkResolutionFailureCode
+  /**
+   * Whether the failure is worth retrying. Today only EmbeddingDisabledError
+   * is permanent (the post's author has forbidden quoting); anything else
+   * is a transient or unknown failure and the caller should attach a
+   * `retry()` to the failed state.
+   */
+  isRetryable: boolean
+}
+
+/**
+ * Classify a thrown error from `resolveLink` into a stable failure code
+ * and a retryability flag. Centralizes the retry-policy decision so the
+ * store doesn't repeat the `code === 'embedding-disabled'` check at every
+ * failure call site.
+ */
+export function parseResolveLinkError(err: unknown): ParsedResolveLinkError {
+  if (err instanceof EmbeddingDisabledError) {
+    return {code: 'embedding-disabled', isRetryable: false}
+  }
+  return {code: 'unknown', isRetryable: true}
+}