fix(cli): report interrupted response bodies without retrying writes

Author: j15z

packages/sim-cli/src/http/client.test.ts

@@ -286,7 +286,7 @@ describe('non-JSON responses', () => {
       name: 'SimApiError',
       status,
       code: 'RESPONSE_READ_FAILED',
-      message: 'Unable to read the response: Connection closed during response',
+      message: expect.stringContaining('Response interrupted: Connection closed during response'),
     })
     expect(fetch).toHaveBeenCalledTimes(1)
   })
@@ -494,6 +494,92 @@ describe('a request that never answers', () => {
   })
 })
 
+describe('interrupted response bodies', () => {
+  it('reports a failed read without implying a write occurred', async () => {
+    vi.stubGlobal(
+      'fetch',
+      vi
+        .fn()
+        .mockResolvedValue(
+          new Response(
+            new ReadableStream({ start: (controller) => controller.error(new Error('Dropped')) })
+          )
+        )
+    )
+
+    await expect(client().request('/api/v2/workflows')).rejects.toMatchObject({
+      name: 'SimApiError',
+      status: 200,
+      code: 'RESPONSE_READ_FAILED',
+      message: expect.stringContaining('Retry the request when the connection is restored.'),
+    })
+  })
+
+  it.each(['timeout', 'cancel'])('explains a %s during a mutation response', async (reason) => {
+    const controller = new AbortController()
+    vi.stubGlobal(
+      'fetch',
+      vi.fn().mockImplementation(async () => {
+        if (reason === 'cancel') controller.abort()
+        return new Response(
+          new ReadableStream({
+            start: (stream) =>
+              stream.error(
+                new DOMException(reason, reason === 'timeout' ? 'TimeoutError' : 'AbortError')
+              ),
+          })
+        )
+      })
+    )
+
+    const failure = await client()
+      .request('/api/v2/workflows/workflow-1/operations', {
+        method: 'POST',
+        body: { operations: [] },
+        signal: controller.signal,
+      })
+      .catch((error: unknown) => error)
+
+    expect(failure).toMatchObject({
+      name: 'SimApiError',
+      status: 200,
+      code: 'RESPONSE_READ_FAILED',
+      message: expect.stringContaining(reason === 'timeout' ? 'Timed out' : 'Request cancelled'),
+    })
+    expect(failure).toMatchObject({ message: expect.stringContaining('may have completed') })
+  })
+
+  it.each([200, 500])(
+    'reports a truncated HTTP %i mutation response without retrying',
+    async (status) => {
+      const response = new Response(
+        new ReadableStream({
+          start(controller) {
+            controller.enqueue(new TextEncoder().encode('{"data":'))
+            controller.error(new Error('Connection dropped after response headers'))
+          },
+        }),
+        { status, headers: { 'content-type': 'application/json' } }
+      )
+      const fetchMock = vi.fn().mockResolvedValue(response)
+      vi.stubGlobal('fetch', fetchMock)
+
+      const failure = await client()
+        .request('/api/v2/workflows/workflow-1/operations', {
+          method: 'POST',
+          body: { operations: [] },
+        })
+        .catch((error: unknown) => error)
+
+      expect(failure).toBeInstanceOf(SimApiError)
+      expect(failure).toMatchObject({
+        message: expect.stringContaining('may have completed'),
+      })
+      expect(fetchMock).toHaveBeenCalledTimes(1)
+    }
+  )
+})
+
 describe('tracing a request', () => {
   it('traces method, url, status and duration when asked, and nothing otherwise', async () => {
     const response = () =>

packages/sim-cli/src/http/client.ts

@@ -215,18 +215,6 @@ function transportErrorMessage(error: unknown): string {
   return messages.join(': ') || 'Unknown network error'
 }
 
-async function readResponseText(response: Response): Promise<string> {
-  try {
-    return await response.text()
-  } catch (error) {
-    throw new SimApiError(
-      `Unable to read the response: ${transportErrorMessage(error)}`,
-      response.status,
-      'RESPONSE_READ_FAILED'
-    )
-  }
-}
-
 /**
  * Whether this is the refusal a workspace-scoped key gets from an operation only
  * a personal key may perform, under either code that expresses it.
@@ -571,7 +559,7 @@ export class SimClient {
 
   async request<T>(path: string, options: RequestOptions = {}): Promise<T> {
     const { response, url } = await this.send(path, options)
-    const raw = await readResponseText(response)
+    const raw = await this.readResponseText(response, url, options)
 
     if (!raw) return undefined as T
     try {
@@ -581,6 +569,31 @@ export class SimClient {
     }
   }
 
+  private async readResponseText(
+    response: Response,
+    url: string,
+    options: RequestOptions
+  ): Promise<string> {
+    try {
+      return await response.text()
+    } catch (cause) {
+      const reason = options.signal?.aborted
+        ? 'Request cancelled while receiving the response.'
+        : isRequestTimeout(cause)
+          ? `Timed out while receiving the response. ${RAISE_TIMEOUT_HINT}`
+          : `Response interrupted: ${transportErrorMessage(cause)}`
+      const retryHint =
+        (options.method ?? 'GET') === 'GET'
+          ? 'Retry the request when the connection is restored.'
+          : 'The operation may have completed. Check the saved state or run status before retrying.'
+      throw new SimApiError(
+        `${url}: ${reason} ${retryHint}`,
+        response.status,
+        'RESPONSE_READ_FAILED'
+      )
+    }
+  }
+
   private async send(
     path: string,
     options: RequestOptions,
@@ -668,7 +681,7 @@ export class SimClient {
     }
 
     if (!response.ok) {
-      const raw = await readResponseText(response)
+      const raw = await this.readResponseText(response, url, options)
       const error = toApiError(url, response.status, response.headers.get('content-type'), raw)
       if (response.status === 401) {
         error.message = `${error.message} — run: sim login --profile ${this.profile.authProfile}`