fix: handle batch response errors in thread view (#91)

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: scottlovegrove

package-lock.json

@@ -47,7 +47,7 @@
         "dist"
     ],
     "dependencies": {
-        "@doist/twist-sdk": "2.1.0",
+        "@doist/twist-sdk": "2.1.1",
         "@pnpm/tabtab": "0.5.4",
         "chalk": "5.6.2",
         "commander": "14.0.3",

package.json

@@ -5,7 +5,10 @@ const apiMocks = vi.hoisted(() => ({
     getTwistClient: vi.fn(),
 }))
 
-vi.mock('../lib/api.js', () => apiMocks)
+vi.mock('../lib/api.js', async (importOriginal) => ({
+    ...(await importOriginal<typeof import('../lib/api.js')>()),
+    getTwistClient: apiMocks.getTwistClient,
+}))
 
 vi.mock('../lib/public-channels.js', () => ({
     assertChannelIsPublic: vi.fn(),
@@ -76,7 +79,10 @@ function createClient({
                 if (options?.batch) return { kind: 'comments' }
                 return Promise.resolve(comments)
             }),
-            getComment: vi.fn(),
+            getComment: vi.fn((_id: number, options?: { batch?: boolean }) => {
+                if (options?.batch) return { kind: 'comment', id: _id }
+                return Promise.resolve(undefined)
+            }),
         },
         channels: {
             getChannel: vi.fn((_id: number, options?: { batch?: boolean }) => {
@@ -97,11 +103,17 @@ function createClient({
         },
         batch: vi.fn(async (...requests: Array<{ kind: string; id?: number; userId?: number }>) =>
             requests.map((request) => {
-                if (request.kind === 'thread') return { data: thread }
-                if (request.kind === 'comments') return { data: comments }
-                if (request.kind === 'channel') return { data: channel }
+                if (request.kind === 'thread') return { code: 200, data: thread }
+                if (request.kind === 'comments') return { code: 200, data: comments }
+                if (request.kind === 'comment')
+                    return {
+                        code: 200,
+                        data: comments.find((c) => c.id === request.id) ?? comments[0],
+                    }
+                if (request.kind === 'channel') return { code: 200, data: channel }
                 if (request.kind === 'user' && request.userId) {
                     return {
+                        code: 200,
                         data: users[request.userId] ?? {
                             id: request.userId,
                             name: `user:${request.userId}`,
@@ -303,3 +315,49 @@ describe('thread view --unread', () => {
         consoleSpy.mockRestore()
     })
 })
+
+describe('thread view with failed batch response', () => {
+    beforeEach(() => {
+        vi.resetAllMocks()
+    })
+
+    it('throws a clear error when comment batch response fails', async () => {
+        const client = createClient({
+            users: { 1: { id: 1, name: 'Alice' } },
+        })
+        // Override batch to return a 404 for the comment
+        client.batch.mockResolvedValueOnce([
+            { code: 200, data: createThread(500) },
+            { code: 404, data: null as never },
+        ])
+        apiMocks.getTwistClient.mockResolvedValue(client)
+
+        const program = createProgram()
+        const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
+
+        await expect(
+            program.parseAsync(['node', 'tw', 'thread', 'view', '500', '--comment', '99999']),
+        ).rejects.toThrow('Failed to fetch comment 99999.')
+
+        consoleSpy.mockRestore()
+    })
+
+    it('throws a clear error when thread batch response fails', async () => {
+        const client = createClient()
+        // Override batch to return a 404 for the thread
+        client.batch.mockResolvedValueOnce([
+            { code: 404, data: null as never },
+            { code: 200, data: [] },
+        ])
+        apiMocks.getTwistClient.mockResolvedValue(client)
+
+        const program = createProgram()
+        const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
+
+        await expect(program.parseAsync(['node', 'tw', 'thread', 'view', '500'])).rejects.toThrow(
+            'Failed to fetch thread.',
+        )
+
+        consoleSpy.mockRestore()
+    })
+})

src/__tests__/thread.test.ts

@@ -1,6 +1,6 @@
 import type { TwistApi } from '@doist/twist-sdk'
 import chalk from 'chalk'
-import { getTwistClient } from '../../lib/api.js'
+import { assertBatchData, getTwistClient } from '../../lib/api.js'
 import { formatRelativeDate } from '../../lib/dates.js'
 import { renderMarkdown } from '../../lib/markdown.js'
 import type { PaginatedViewOptions } from '../../lib/options.js'
@@ -26,8 +26,8 @@ async function viewSingleComment(
         client.comments.getComment(commentId, { batch: true }),
     )
 
-    const thread = threadResponse.data
-    const comment = commentResponse.data
+    const thread = assertBatchData(threadResponse, 'thread')
+    const comment = assertBatchData(commentResponse, `comment ${commentId}`)
 
     const userIds = new Set([thread.creator, comment.creator])
     const userCalls = [...userIds].map((id) =>
@@ -41,8 +41,10 @@ async function viewSingleComment(
         ...userCalls,
     )
 
-    const channel = channelResponse.data
-    const userMap = new Map(userResponses.map((r) => [r.data.id, r.data.name]))
+    const channel = assertBatchData(channelResponse, 'channel')
+    const userMap = new Map(
+        userResponses.filter((r) => r.data != null).map((r) => [r.data.id, r.data.name]),
+    )
 
     if (options.json) {
         const output = {
@@ -102,8 +104,8 @@ export async function viewThread(ref: string, options: ViewOptions): Promise<voi
         ),
     )
 
-    const thread = threadResponse.data
-    const comments = commentsResponse.data
+    const thread = assertBatchData(threadResponse, 'thread')
+    const comments = assertBatchData(commentsResponse, 'comments')
 
     await assertChannelIsPublic(thread.channelId, thread.workspaceId)
 
@@ -149,8 +151,10 @@ export async function viewThread(ref: string, options: ViewOptions): Promise<voi
         ...userCalls,
     )
 
-    const channel = channelResponse.data
-    const userMap = new Map(userResponses.map((r) => [r.data.id, r.data.name]))
+    const channel = assertBatchData(channelResponse, 'channel')
+    const userMap = new Map(
+        userResponses.filter((r) => r.data != null).map((r) => [r.data.id, r.data.name]),
+    )
 
     if (options.json) {
         const output = {

src/commands/thread/view.ts

@@ -261,4 +261,17 @@ export function clearUserCache(): void {
     sessionUserCache = null
 }
 
+/**
+ * Validates a batch response and returns the data, throwing on errors.
+ * Also handles the case where the SDK fails to validate the response schema
+ * (e.g. when the batch API wraps entities in a key like `{comment: {...}}`).
+ * In that case, the raw transformed data is returned — check for expected fields.
+ */
+export function assertBatchData<T>(response: { code: number; data: T }, label: string): T {
+    if (response.code >= 400 || response.data == null) {
+        throw new Error(`Failed to fetch ${label}.`)
+    }
+    return response.data
+}
+
 export type { Workspace, WorkspaceUser, User }