Skip to content

Commit 9dffe43

Browse files
committed
fix(api): guard response.text() in error paths and label chars
Addresses Cursor bugbot feedback on PR #1233. 1. `result.text?.()` in the `!result.ok` branches of `queryApiSafeText` and `sendApiRequest` was unguarded. If `text()` threw, the exception propagated past the clean `{ ok: false, ... }` return and broke the error-handling contract. Wrap both call sites in a shared `tryReadResponseText` helper that swallows the failure and returns `undefined`. 2. The truncation suffix reported `body.length` as "bytes", but `String.prototype.length` / `String.prototype.slice` count UTF-16 code units, not bytes. For non-ASCII response bodies the label was misleading. Rename to "chars" so the counter matches the actual measurement.
1 parent a6ee497 commit 9dffe43

File tree

3 files changed

+19
-4
lines changed

3 files changed

+19
-4
lines changed

packages/cli/src/utils/debug.mts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,12 @@ function buildApiDebugDetails(
129129
}
130130
if (requestInfo.responseBody !== undefined) {
131131
const body = requestInfo.responseBody
132+
// `.length` / `.slice` operate on UTF-16 code units, not bytes, so
133+
// the counter and truncation are both reported in "chars" to stay
134+
// consistent with what we actually measured.
132135
details['responseBody'] =
133136
body.length > RESPONSE_BODY_TRUNCATE_LENGTH
134-
? `${body.slice(0, RESPONSE_BODY_TRUNCATE_LENGTH)}… (truncated, ${body.length} bytes)`
137+
? `${body.slice(0, RESPONSE_BODY_TRUNCATE_LENGTH)}… (truncated, ${body.length} chars)`
135138
: body
136139
}
137140
return details

packages/cli/src/utils/socket/api.mts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,18 @@ export async function socketHttpRequest(
8282
return await httpRequest(url, options)
8383
}
8484

85+
// Safe wrapper for `response.text()` in error-handling code paths.
86+
// `text()` can throw (e.g. already consumed, malformed body), which
87+
// would blow past the `ok: false` CResult return and break the
88+
// error-handling contract of callers like `queryApiSafeText`.
89+
function tryReadResponseText(result: HttpResponse): string | undefined {
90+
try {
91+
return result.text?.()
92+
} catch {
93+
return undefined
94+
}
95+
}
96+
8597
export type CommandRequirements = {
8698
permissions?: string[] | undefined
8799
quota?: number | undefined
@@ -485,7 +497,7 @@ export async function queryApiSafeText(
485497
requestedAt,
486498
headers: { Authorization: '[REDACTED]' },
487499
responseHeaders: result.headers,
488-
responseBody: result.text?.(),
500+
responseBody: tryReadResponseText(result),
489501
})
490502
// Log required permissions for 403 errors when in a command context.
491503
if (commandPath && status === 403) {
@@ -674,7 +686,7 @@ export async function sendApiRequest<T>(
674686
'Content-Type': 'application/json',
675687
},
676688
responseHeaders: result.headers,
677-
responseBody: result.text?.(),
689+
responseBody: tryReadResponseText(result),
678690
})
679691
// Log required permissions for 403 errors when in a command context.
680692
if (commandPath && status === 403) {

packages/cli/test/unit/utils/debug.test.mts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ describe('debug utilities', () => {
258258
debugApiResponse('/api/big', 500, undefined, requestInfo)
259259

260260
const calledWith = mockDebugDir.mock.calls[0]?.[0]
261-
expect(calledWith.responseBody).toMatch(/ \(truncated, 5000 bytes\)$/)
261+
expect(calledWith.responseBody).toMatch(/ \(truncated, 5000 chars\)$/)
262262
expect((calledWith.responseBody as string).length).toBeLessThan(
263263
bigBody.length,
264264
)

0 commit comments

Comments
 (0)