Skip to content

Commit a47f21f

Browse files
fix(openai-native): keep provider error text out of the abort-race telemetry capture
The abort-race capture put the SDK's own message into ApiProviderError, and PostHogTelemetryClient forwards error.message to PostHog, so a 400 body that echoes request content (an address, a prompt fragment) could reach the telemetry backend. Report a fixed message plus the numeric status in errorCode instead - the client already turns errorCode into a structured property - and add a regression test whose provider error carries sensitive text. The propagated object is unchanged, so retry, backoff and the UI still read the provider's own error, and the capture still happens exactly once before the throw.
1 parent 865ae0d commit a47f21f

3 files changed

Lines changed: 70 additions & 2 deletions

File tree

‎src/api/providers/__tests__/openai-native.spec.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1542,6 +1542,53 @@ describe("OpenAiNativeHandler", () => {
15421542
expect(mockCaptureException.mock.calls[0]?.[0]).toMatchObject({ operation: "createMessage" })
15431543
})
15441544

1545+
it("keeps the provider's own error text out of the abort-race telemetry capture", async () => {
1546+
// A 400 body can echo request content — an address, a prompt fragment — and the
1547+
// telemetry client forwards error.message to PostHog, so an abort-raced failure
1548+
// must be reported as fixed text plus the numeric status, never as its own text.
1549+
const mockFetch = vitest.fn()
1550+
global.fetch = mockFetch as typeof fetch
1551+
const controller = new AbortController()
1552+
const sensitiveText = "400 Invalid request: someone@example.com sent 120000 tokens"
1553+
const leaky = Object.assign(new Error(sensitiveText), { status: 400 })
1554+
mockResponsesCreate.mockImplementation((_body: unknown, options: { signal?: AbortSignal }) => {
1555+
return new Promise((_resolve, reject) => {
1556+
if (options?.signal?.aborted) {
1557+
reject(leaky)
1558+
return
1559+
}
1560+
options?.signal?.addEventListener("abort", () => reject(leaky), { once: true })
1561+
})
1562+
})
1563+
1564+
const collected = collectStream(
1565+
handler.createMessage(
1566+
systemPrompt,
1567+
messages,
1568+
makeCreateMessageMetadata({ abortSignal: controller.signal }),
1569+
),
1570+
)
1571+
await tick()
1572+
controller.abort()
1573+
1574+
const error = await collected.then(
1575+
() => undefined,
1576+
(e: unknown) => e,
1577+
)
1578+
// The caller still receives the provider's own object, provider text included.
1579+
expect(error).toBe(leaky)
1580+
expect(mockFetch).not.toHaveBeenCalled()
1581+
expect(mockCaptureException).toHaveBeenCalledTimes(1)
1582+
const [captured] = mockCaptureException.mock.calls[0] ?? []
1583+
const capturedError = captured instanceof ApiProviderError ? captured : undefined
1584+
expect(capturedError).toBeDefined()
1585+
expect(capturedError?.message).toBe(
1586+
"OpenAI Native createMessage request failed with a status-bearing error after the caller aborted",
1587+
)
1588+
expect(capturedError?.message).not.toContain("someone@example.com")
1589+
expect(capturedError?.errorCode).toBe(400)
1590+
})
1591+
15451592
it("should keep a streaming request's controller when completePrompt uses its own signal", async () => {
15461593
// completePrompt is a request-local signal path: it must not take over or clear the
15471594
// controller owned by an in-flight createMessage on the same handler, and aborting

‎src/api/providers/openai-native.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import { isMcpTool } from "../../utils/mcp-name"
3636
import { sanitizeOpenAiCallId } from "../../utils/tool-id"
3737
import { createAbortError, isRequestAborted, mergeAbortSignalAndTimeout } from "./utils/abort-signal"
3838
import { createTimeoutError, getRequestTimeoutMs } from "./utils/request-timeout"
39-
import { hasNumericHttpStatus } from "./utils/error-handler"
39+
import { getNumericHttpStatus, hasNumericHttpStatus } from "./utils/error-handler"
4040

4141
export type OpenAiNativeModel = ReturnType<OpenAiNativeHandler["getModel"]>
4242

@@ -581,12 +581,18 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio
581581
// visible in provider telemetry, and capturing must not rewrite or
582582
// replace the propagated error.
583583
const failedModel = this.getModel()
584+
// The provider's own error text stays out of telemetry: a 400 body can echo
585+
// request content (an email address, a prompt fragment), and the telemetry
586+
// client sends error.message straight to PostHog, so this path reports a fixed
587+
// message plus the numeric status in errorCode, which the client turns into a
588+
// structured property. The propagated object still carries the provider text.
584589
TelemetryService.instance.captureException(
585590
new ApiProviderError(
586-
sdkErr instanceof Error ? sdkErr.message : String(sdkErr),
591+
`${this.providerName} createMessage request failed with a status-bearing error after the caller aborted`,
587592
this.providerName,
588593
failedModel.id,
589594
"createMessage",
595+
getNumericHttpStatus(sdkErr),
590596
),
591597
)
592598
throw sdkErr

‎src/api/providers/utils/error-handler.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,21 @@ export function hasNumericHttpStatus(error: unknown): boolean {
5454
return false
5555
}
5656

57+
/**
58+
* The numeric HTTP status carried by an error, or `undefined` when it carries none.
59+
* Same field order as hasNumericHttpStatus — `status` (most SDKs expose it that way)
60+
* before `statusCode` (the AI SDK's APICallError shape) — and only finite numbers,
61+
* so a null or stringy status never becomes a retry header or a telemetry property.
62+
*/
63+
export function getNumericHttpStatus(error: unknown): number | undefined {
64+
if (!error || typeof error !== "object") return undefined
65+
const candidate = error as { status?: unknown; statusCode?: unknown }
66+
// Number.isFinite does not coerce, so the value it accepted has to be handed back
67+
// with a cast: there is no narrower form for an already-validated unknown number.
68+
if (Number.isFinite(candidate.status)) return candidate.status as number
69+
return Number.isFinite(candidate.statusCode) ? (candidate.statusCode as number) : undefined
70+
}
71+
5772
export function handleProviderError(
5873
error: unknown,
5974
providerName: string,

0 commit comments

Comments
 (0)