diff --git a/.github/fern/fern.config.json b/.github/fern/fern.config.json index 6197fdb90b..0ab413211b 100644 --- a/.github/fern/fern.config.json +++ b/.github/fern/fern.config.json @@ -1,4 +1,4 @@ { "organization": "truefoundry", - "version": "5.143.4" + "version": "5.171.3" } diff --git a/.github/fern/generators.yml b/.github/fern/generators.yml index 1ae2a2b1dd..e533f26ffe 100644 --- a/.github/fern/generators.yml +++ b/.github/fern/generators.yml @@ -37,7 +37,7 @@ groups: ts-sdk: generators: - name: fernapi/fern-typescript-node-sdk - version: 3.98.3 + version: 3.107.6 output: location: local-file-system path: ../../packages/trueforge-sdk @@ -104,7 +104,7 @@ groups: python-sdk: generators: - name: fernapi/fern-python-sdk - version: 5.34.2 + version: 5.41.7 output: location: local-file-system path: ../../python/trueforge_sdk diff --git a/packages/trueforge-sdk/src/api/resources/agents/client/Client.ts b/packages/trueforge-sdk/src/api/resources/agents/client/Client.ts index 5a3ff0a293..cf312740d2 100644 --- a/packages/trueforge-sdk/src/api/resources/agents/client/Client.ts +++ b/packages/trueforge-sdk/src/api/resources/agents/client/Client.ts @@ -297,7 +297,7 @@ export class AgentsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/agents/${core.url.encodePathParam(agent_id)}`, + `api/v1/agents/${core.url.encodePathParam(agent_id, errors.TrueForgeError)}`, ), method: "GET", headers: _headers, @@ -391,7 +391,7 @@ export class AgentsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/agents/${core.url.encodePathParam(agent_id)}`, + `api/v1/agents/${core.url.encodePathParam(agent_id, errors.TrueForgeError)}`, ), method: "PUT", headers: _headers, @@ -508,7 +508,7 @@ export class AgentsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/agents/${core.url.encodePathParam(agent_id)}`, + `api/v1/agents/${core.url.encodePathParam(agent_id, errors.TrueForgeError)}`, ), method: "DELETE", headers: _headers, diff --git a/packages/trueforge-sdk/src/api/resources/internal/resources/agents/client/Client.ts b/packages/trueforge-sdk/src/api/resources/internal/resources/agents/client/Client.ts index b9a69f038b..70fe0184aa 100644 --- a/packages/trueforge-sdk/src/api/resources/internal/resources/agents/client/Client.ts +++ b/packages/trueforge-sdk/src/api/resources/internal/resources/agents/client/Client.ts @@ -63,7 +63,7 @@ export class AgentsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/agents/${core.url.encodePathParam(agent_id)}/code-snippets`, + `api/v1/agents/${core.url.encodePathParam(agent_id, errors.TrueForgeError)}/code-snippets`, ), method: "GET", headers: _headers, diff --git a/packages/trueforge-sdk/src/api/resources/mcpServers/client/Client.ts b/packages/trueforge-sdk/src/api/resources/mcpServers/client/Client.ts index c4c879f000..cedc5203de 100644 --- a/packages/trueforge-sdk/src/api/resources/mcpServers/client/Client.ts +++ b/packages/trueforge-sdk/src/api/resources/mcpServers/client/Client.ts @@ -137,7 +137,7 @@ export class McpServersClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/mcp-servers/${core.url.encodePathParam(name)}`, + `api/v1/mcp-servers/${core.url.encodePathParam(name, errors.TrueForgeError)}`, ), method: "GET", headers: _headers, @@ -242,7 +242,7 @@ export class McpServersClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/mcp-servers/${core.url.encodePathParam(name)}/authorize`, + `api/v1/mcp-servers/${core.url.encodePathParam(name, errors.TrueForgeError)}/authorize`, ), method: "GET", headers: _headers, @@ -378,7 +378,7 @@ export class McpServersClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/mcp-servers/${core.url.encodePathParam(name)}/authorize`, + `api/v1/mcp-servers/${core.url.encodePathParam(name, errors.TrueForgeError)}/authorize`, ), method: "DELETE", headers: _headers, @@ -469,7 +469,7 @@ export class McpServersClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/mcp-servers/${core.url.encodePathParam(name)}/tools`, + `api/v1/mcp-servers/${core.url.encodePathParam(name, errors.TrueForgeError)}/tools`, ), method: "GET", headers: _headers, diff --git a/packages/trueforge-sdk/src/api/resources/sandboxEnvironments/client/Client.ts b/packages/trueforge-sdk/src/api/resources/sandboxEnvironments/client/Client.ts index 55a1d6464e..70671cae17 100644 --- a/packages/trueforge-sdk/src/api/resources/sandboxEnvironments/client/Client.ts +++ b/packages/trueforge-sdk/src/api/resources/sandboxEnvironments/client/Client.ts @@ -502,7 +502,7 @@ export class SandboxEnvironmentsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sandbox-environments/${core.url.encodePathParam(name)}`, + `api/v1/sandbox-environments/${core.url.encodePathParam(name, errors.TrueForgeError)}`, ), method: "GET", headers: _headers, @@ -606,7 +606,7 @@ export class SandboxEnvironmentsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sandbox-environments/${core.url.encodePathParam(name)}`, + `api/v1/sandbox-environments/${core.url.encodePathParam(name, errors.TrueForgeError)}`, ), method: "DELETE", headers: _headers, diff --git a/packages/trueforge-sdk/src/api/resources/schedules/client/Client.ts b/packages/trueforge-sdk/src/api/resources/schedules/client/Client.ts index 1199f6e37f..14c34c3825 100644 --- a/packages/trueforge-sdk/src/api/resources/schedules/client/Client.ts +++ b/packages/trueforge-sdk/src/api/resources/schedules/client/Client.ts @@ -442,7 +442,7 @@ export class SchedulesClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/schedules/${core.url.encodePathParam(schedule_id)}`, + `api/v1/schedules/${core.url.encodePathParam(schedule_id, errors.TrueForgeError)}`, ), method: "GET", headers: _headers, @@ -553,7 +553,7 @@ export class SchedulesClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/schedules/${core.url.encodePathParam(schedule_id)}`, + `api/v1/schedules/${core.url.encodePathParam(schedule_id, errors.TrueForgeError)}`, ), method: "PUT", headers: _headers, @@ -686,7 +686,7 @@ export class SchedulesClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/schedules/${core.url.encodePathParam(schedule_id)}`, + `api/v1/schedules/${core.url.encodePathParam(schedule_id, errors.TrueForgeError)}`, ), method: "DELETE", headers: _headers, @@ -791,7 +791,7 @@ export class SchedulesClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/schedules/${core.url.encodePathParam(schedule_id)}/runs`, + `api/v1/schedules/${core.url.encodePathParam(schedule_id, errors.TrueForgeError)}/runs`, ), method: "GET", headers: _headers, diff --git a/packages/trueforge-sdk/src/api/resources/sessions/client/Client.ts b/packages/trueforge-sdk/src/api/resources/sessions/client/Client.ts index 2faf08eefe..4074748441 100644 --- a/packages/trueforge-sdk/src/api/resources/sessions/client/Client.ts +++ b/packages/trueforge-sdk/src/api/resources/sessions/client/Client.ts @@ -316,7 +316,7 @@ export class SessionsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sessions/${core.url.encodePathParam(session_id)}`, + `api/v1/sessions/${core.url.encodePathParam(session_id, errors.TrueForgeError)}`, ), method: "GET", headers: _headers, @@ -407,7 +407,7 @@ export class SessionsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sessions/${core.url.encodePathParam(session_id)}`, + `api/v1/sessions/${core.url.encodePathParam(session_id, errors.TrueForgeError)}`, ), method: "DELETE", headers: _headers, @@ -492,7 +492,7 @@ export class SessionsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sessions/${core.url.encodePathParam(session_id)}`, + `api/v1/sessions/${core.url.encodePathParam(session_id, errors.TrueForgeError)}`, ), method: "PATCH", headers: _headers, @@ -629,7 +629,7 @@ export class SessionsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sessions/${core.url.encodePathParam(session_id)}/cancel`, + `api/v1/sessions/${core.url.encodePathParam(session_id, errors.TrueForgeError)}/cancel`, ), method: "POST", headers: _headers, @@ -757,7 +757,7 @@ export class SessionsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sessions/${core.url.encodePathParam(session_id)}/events`, + `api/v1/sessions/${core.url.encodePathParam(session_id, errors.TrueForgeError)}/events`, ), method: "GET", headers: _headers, @@ -889,7 +889,7 @@ export class SessionsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sessions/${core.url.encodePathParam(session_id)}/turns`, + `api/v1/sessions/${core.url.encodePathParam(session_id, errors.TrueForgeError)}/turns`, ), method: "GET", headers: _headers, @@ -1011,7 +1011,7 @@ export class SessionsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sessions/${core.url.encodePathParam(session_id)}/turns`, + `api/v1/sessions/${core.url.encodePathParam(session_id, errors.TrueForgeError)}/turns`, ), method: "POST", headers: _headers, @@ -1202,7 +1202,7 @@ export class SessionsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sessions/${core.url.encodePathParam(session_id)}/turns`, + `api/v1/sessions/${core.url.encodePathParam(session_id, errors.TrueForgeError)}/turns`, ), method: "POST", headers: _headers, @@ -1374,7 +1374,7 @@ export class SessionsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sessions/${core.url.encodePathParam(session_id)}/turns/${core.url.encodePathParam(turn_id)}`, + `api/v1/sessions/${core.url.encodePathParam(session_id, errors.TrueForgeError)}/turns/${core.url.encodePathParam(turn_id, errors.TrueForgeError)}`, ), method: "GET", headers: _headers, @@ -1483,7 +1483,7 @@ export class SessionsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sessions/${core.url.encodePathParam(session_id)}/turns/${core.url.encodePathParam(turn_id)}/download-sandbox-file`, + `api/v1/sessions/${core.url.encodePathParam(session_id, errors.TrueForgeError)}/turns/${core.url.encodePathParam(turn_id, errors.TrueForgeError)}/download-sandbox-file`, ), method: "GET", headers: _headers, @@ -1650,7 +1650,7 @@ export class SessionsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sessions/${core.url.encodePathParam(session_id)}/turns/${core.url.encodePathParam(turn_id)}/events`, + `api/v1/sessions/${core.url.encodePathParam(session_id, errors.TrueForgeError)}/turns/${core.url.encodePathParam(turn_id, errors.TrueForgeError)}/events`, ), method: "GET", headers: _headers, @@ -1791,7 +1791,7 @@ export class SessionsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sessions/${core.url.encodePathParam(session_id)}/turns/${core.url.encodePathParam(turn_id)}/events`, + `api/v1/sessions/${core.url.encodePathParam(session_id, errors.TrueForgeError)}/turns/${core.url.encodePathParam(turn_id, errors.TrueForgeError)}/events`, ), method: "POST", headers: _headers, @@ -1923,7 +1923,7 @@ export class SessionsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sessions/${core.url.encodePathParam(session_id)}/turns/${core.url.encodePathParam(turn_id)}/subscribe`, + `api/v1/sessions/${core.url.encodePathParam(session_id, errors.TrueForgeError)}/turns/${core.url.encodePathParam(turn_id, errors.TrueForgeError)}/subscribe`, ), method: "GET", headers: _headers, @@ -1944,7 +1944,7 @@ export class SessionsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/sessions/${core.url.encodePathParam(session_id)}/turns/${core.url.encodePathParam(turn_id)}/subscribe`, + `api/v1/sessions/${core.url.encodePathParam(session_id, errors.TrueForgeError)}/turns/${core.url.encodePathParam(turn_id, errors.TrueForgeError)}/subscribe`, ), method: "GET", headers: { ..._headers, "Last-Event-ID": lastEventId }, diff --git a/packages/trueforge-sdk/src/api/resources/settings/resources/mcpServers/client/Client.ts b/packages/trueforge-sdk/src/api/resources/settings/resources/mcpServers/client/Client.ts index fb8bc100b4..1fa78b0c3d 100644 --- a/packages/trueforge-sdk/src/api/resources/settings/resources/mcpServers/client/Client.ts +++ b/packages/trueforge-sdk/src/api/resources/settings/resources/mcpServers/client/Client.ts @@ -411,7 +411,7 @@ export class McpServersClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/settings/mcp-servers/${core.url.encodePathParam(name)}`, + `api/v1/settings/mcp-servers/${core.url.encodePathParam(name, errors.TrueForgeError)}`, ), method: "GET", headers: _headers, @@ -501,7 +501,7 @@ export class McpServersClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/settings/mcp-servers/${core.url.encodePathParam(name)}`, + `api/v1/settings/mcp-servers/${core.url.encodePathParam(name, errors.TrueForgeError)}`, ), method: "DELETE", headers: _headers, diff --git a/packages/trueforge-sdk/src/api/resources/settings/resources/modelProviders/client/Client.ts b/packages/trueforge-sdk/src/api/resources/settings/resources/modelProviders/client/Client.ts index 3bfcf80f7f..510f1ada4e 100644 --- a/packages/trueforge-sdk/src/api/resources/settings/resources/modelProviders/client/Client.ts +++ b/packages/trueforge-sdk/src/api/resources/settings/resources/modelProviders/client/Client.ts @@ -426,7 +426,7 @@ export class ModelProvidersClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/settings/model-providers/${core.url.encodePathParam(name)}`, + `api/v1/settings/model-providers/${core.url.encodePathParam(name, errors.TrueForgeError)}`, ), method: "DELETE", headers: _headers, diff --git a/packages/trueforge-sdk/src/api/resources/settings/resources/skills/client/Client.ts b/packages/trueforge-sdk/src/api/resources/settings/resources/skills/client/Client.ts index d35819957f..c7d787b86f 100644 --- a/packages/trueforge-sdk/src/api/resources/settings/resources/skills/client/Client.ts +++ b/packages/trueforge-sdk/src/api/resources/settings/resources/skills/client/Client.ts @@ -389,7 +389,7 @@ export class SkillsClient { url: core.url.join( (await core.Supplier.get(this._options.baseUrl)) ?? (await core.Supplier.get(this._options.environment)), - `api/v1/settings/skills/${core.url.encodePathParam(name)}`, + `api/v1/settings/skills/${core.url.encodePathParam(name, errors.TrueForgeError)}`, ), method: "DELETE", headers: _headers, diff --git a/packages/trueforge-sdk/src/api/types/AvailableModel.ts b/packages/trueforge-sdk/src/api/types/AvailableModel.ts index 5703b5f0e0..01698be6ff 100644 --- a/packages/trueforge-sdk/src/api/types/AvailableModel.ts +++ b/packages/trueforge-sdk/src/api/types/AvailableModel.ts @@ -9,6 +9,6 @@ export interface AvailableModel { name: string; properties: TrueForge.ModelProperties; provider: TrueForge.AvailableModelProvider; - /** Warning for not so capable models. */ + /** Set when reasoning effort or context length is not configured; may affect TrueForge performance. */ warning?: string; } diff --git a/packages/trueforge-sdk/src/core/fetcher/Fetcher.ts b/packages/trueforge-sdk/src/core/fetcher/Fetcher.ts index cd5c5793d6..b8456abedb 100644 --- a/packages/trueforge-sdk/src/core/fetcher/Fetcher.ts +++ b/packages/trueforge-sdk/src/core/fetcher/Fetcher.ts @@ -7,12 +7,13 @@ import { EndpointSupplier } from "./EndpointSupplier.js"; import { getErrorResponseBody } from "./getErrorResponseBody.js"; import { getFetchFn } from "./getFetchFn.js"; import { getRequestBody } from "./getRequestBody.js"; -import { getResponseBody } from "./getResponseBody.js"; +import { getResponseBody, isResponseBodyError } from "./getResponseBody.js"; import { Headers } from "./Headers.js"; -import { makeRequest } from "./makeRequest.js"; +import { clearResponseTimeout, makeRequest } from "./makeRequest.js"; import { abortRawResponse, toRawResponse, unknownRawResponse } from "./RawResponse.js"; import { redactUrl, SENSITIVE_QUERY_PARAMS } from "./redactUrl.js"; import { requestWithRetries } from "./requestWithRetries.js"; +import { TIMEOUT } from "./signals.js"; export type FetchFunction = (args: Fetcher.Args) => Promise>; @@ -152,6 +153,11 @@ async function getHeaders(args: Fetcher.Args): Promise { return newHeaders; } +function isJsonContentType(contentType: string | null): boolean { + const mediaType = contentType?.split(";")[0]?.trim().toLowerCase() ?? ""; + return mediaType === "application/json" || mediaType === "text/json" || mediaType.endsWith("+json"); +} + export async function fetcherImpl(args: Fetcher.Args): Promise> { let url = args.url; if (args.queryString != null && args.queryString.length > 0) { @@ -178,10 +184,18 @@ export async function fetcherImpl(args: Fetcher.Args): Promise - makeRequest( + async () => { + // A retry means the previous attempt is over; stop its timer now rather than at the end. + for (const previousResponse of attemptResponses) { + clearResponseTimeout(previousResponse); + } + const attemptResponse = await makeRequest( fetchFn, url, args.method, @@ -192,8 +206,13 @@ export async function fetcherImpl(args: Fetcher.Args): Promise= 200 && response.status < 400) { @@ -207,6 +226,18 @@ export async function fetcherImpl(args: Fetcher.Args): Promise(args: Fetcher.Args): Promise(args: Fetcher.Args): Promise { let contentType = response.headers.get("Content-Type")?.toLowerCase(); if (contentType == null || contentType.length === 0) { - return getResponseBody(response); + const body = await getResponseBody(response); + if (isResponseBodyError(body)) { + return body.error.reason === "non-json" ? body.error.rawBody : undefined; + } + return body; } if (contentType.indexOf(";") !== -1) { @@ -16,14 +32,11 @@ export async function getErrorResponseBody(response: Response): Promise case "application/ld+json": case "application/problem+json": case "application/vnd.api+json": - case "text/json": { - const text = await response.text(); - return text.length > 0 ? fromJson(text) : undefined; - } + case "text/json": + return parseJsonErrorBody(await response.text()); default: if (contentType.startsWith("application/vnd.") && contentType.endsWith("+json")) { - const text = await response.text(); - return text.length > 0 ? fromJson(text) : undefined; + return parseJsonErrorBody(await response.text()); } // Fallback to plain text if content type is not recognized diff --git a/packages/trueforge-sdk/src/core/fetcher/getResponseBody.ts b/packages/trueforge-sdk/src/core/fetcher/getResponseBody.ts index 2e831e4956..e0b736335a 100644 --- a/packages/trueforge-sdk/src/core/fetcher/getResponseBody.ts +++ b/packages/trueforge-sdk/src/core/fetcher/getResponseBody.ts @@ -11,6 +11,27 @@ function retainResponse(target: object, response: Response): void { }); } +export type ResponseBodyError = { + ok: false; + error: { reason: "non-json"; statusCode: number; rawBody: string } | { reason: "body-is-null"; statusCode: number }; +}; + +const responseBodyErrors = new WeakSet(); + +function responseBodyError(error: ResponseBodyError["error"]): ResponseBodyError { + const record: ResponseBodyError = { ok: false, error }; + responseBodyErrors.add(record); + return record; +} + +/** + * Returns true when `value` is a failure record created by `getResponseBody` (for example malformed JSON), + * as opposed to a parsed JSON body that happens to have the same shape. + */ +export function isResponseBodyError(value: unknown): value is ResponseBodyError { + return typeof value === "object" && value != null && responseBodyErrors.has(value); +} + export async function getResponseBody(response: Response, responseType?: string): Promise { switch (responseType) { case "binary-response": @@ -21,25 +42,19 @@ export async function getResponseBody(response: Response, responseType?: string) return await response.arrayBuffer(); case "sse": if (response.body == null) { - return { - ok: false, - error: { - reason: "body-is-null", - statusCode: response.status, - }, - }; + return responseBodyError({ + reason: "body-is-null", + statusCode: response.status, + }); } retainResponse(response.body, response); return response.body; case "streaming": if (response.body == null) { - return { - ok: false, - error: { - reason: "body-is-null", - statusCode: response.status, - }, - }; + return responseBodyError({ + reason: "body-is-null", + statusCode: response.status, + }); } retainResponse(response.body, response); @@ -56,14 +71,11 @@ export async function getResponseBody(response: Response, responseType?: string) const responseBody = fromJson(text); return responseBody; } catch (_err) { - return { - ok: false, - error: { - reason: "non-json", - statusCode: response.status, - rawBody: text, - }, - }; + return responseBodyError({ + reason: "non-json", + statusCode: response.status, + rawBody: text, + }); } } return undefined; diff --git a/packages/trueforge-sdk/src/core/fetcher/makePassthroughRequest.ts b/packages/trueforge-sdk/src/core/fetcher/makePassthroughRequest.ts index e8dceda938..140f1acbef 100644 --- a/packages/trueforge-sdk/src/core/fetcher/makePassthroughRequest.ts +++ b/packages/trueforge-sdk/src/core/fetcher/makePassthroughRequest.ts @@ -6,6 +6,7 @@ import { makeRequest } from "./makeRequest.js"; import { redactUrl } from "./redactUrl.js"; import { requestWithRetries } from "./requestWithRetries.js"; import { Supplier } from "./Supplier.js"; +import { TIMEOUT } from "./signals.js"; export declare namespace PassthroughRequest { /** @@ -163,22 +164,32 @@ export async function makePassthroughRequest( }); } - const response = await requestWithRetries( - async () => - makeRequest( - fetchFn, - fullUrl, - method, - mergedHeaders, - body ?? undefined, - timeoutMs, - abortSignal, - effectiveInit?.credentials === "include", - undefined, // duplex - false, // disableCache - ), - maxRetries, - ); + let response: Response; + try { + response = await requestWithRetries( + async () => + makeRequest( + fetchFn, + fullUrl, + method, + mergedHeaders, + body ?? undefined, + timeoutMs, + abortSignal, + effectiveInit?.credentials === "include", + undefined, // duplex + false, // disableCache + ), + maxRetries, + abortSignal, + ); + } catch (error) { + // Match `fetch`: a timeout rejects with an Error named "TimeoutError", not the bare abort reason. + if (error === TIMEOUT) { + throw createTimeoutError(); + } + throw error; + } if (logger.isDebug()) { logger.debug("Passthrough HTTP request completed", { @@ -191,6 +202,12 @@ export async function makePassthroughRequest( return response; } +function createTimeoutError(): Error { + const error = new Error("The request timed out."); + error.name = "TimeoutError"; + return error; +} + /** * Returns true when the resolved request URL points at the same origin as the * configured base URL. Relative paths are always joined onto the base URL, so diff --git a/packages/trueforge-sdk/src/core/fetcher/makeRequest.ts b/packages/trueforge-sdk/src/core/fetcher/makeRequest.ts index 360a86df40..62ff9fcc7e 100644 --- a/packages/trueforge-sdk/src/core/fetcher/makeRequest.ts +++ b/packages/trueforge-sdk/src/core/fetcher/makeRequest.ts @@ -1,4 +1,4 @@ -import { anySignal, getTimeoutSignal } from "./signals.js"; +import { anySignal, getTimeoutSignal, TIMEOUT } from "./signals.js"; /** * Cached result of checking whether the current runtime supports @@ -26,6 +26,20 @@ export function resetCacheNoStoreSupported(): void { _cacheNoStoreSupported = undefined; } +const responseTimeouts = new WeakMap>(); + +/** + * Clears the timeout that `makeRequest` kept running (with `keepTimeoutUntilBodyRead`) so that it also + * covered reading the response body. + */ +export function clearResponseTimeout(response: Response): void { + const timeoutId = responseTimeouts.get(response); + if (timeoutId != null) { + clearTimeout(timeoutId); + responseTimeouts.delete(response); + } +} + export const makeRequest = async ( fetchFn: (url: string, init: RequestInit) => Promise, url: string, @@ -37,13 +51,16 @@ export const makeRequest = async ( withCredentials?: boolean, duplex?: "half", disableCache?: boolean, + keepTimeoutUntilBodyRead?: boolean, ): Promise => { const signals: AbortSignal[] = []; let timeoutAbortId: ReturnType | undefined; + let timeoutSignal: AbortSignal | undefined; if (timeoutMs != null) { const { signal, abortId } = getTimeoutSignal(timeoutMs); timeoutAbortId = abortId; + timeoutSignal = signal; signals.push(signal); } @@ -51,19 +68,35 @@ export const makeRequest = async ( signals.push(abortSignal); } const newSignals = anySignal(signals); - const response = await fetchFn(url, { - method: method, - headers, - body: requestBody, - signal: newSignals, - credentials: withCredentials ? "include" : undefined, - // @ts-ignore - duplex, - ...(disableCache && isCacheNoStoreSupported() ? { cache: "no-store" as RequestCache } : {}), - }); + let response: Response; + try { + response = await fetchFn(url, { + method: method, + headers, + body: requestBody, + signal: newSignals, + credentials: withCredentials ? "include" : undefined, + // @ts-ignore + duplex, + ...(disableCache && isCacheNoStoreSupported() ? { cache: "no-store" as RequestCache } : {}), + }); + } catch (error) { + if (timeoutAbortId != null) { + clearTimeout(timeoutAbortId); + } + // Some runtimes (e.g. Node 18) reject with their own error instead of the abort reason. + if (timeoutSignal?.aborted && !abortSignal?.aborted) { + throw TIMEOUT; + } + throw error; + } if (timeoutAbortId != null) { - clearTimeout(timeoutAbortId); + if (keepTimeoutUntilBodyRead) { + responseTimeouts.set(response, timeoutAbortId); + } else { + clearTimeout(timeoutAbortId); + } } return response; diff --git a/packages/trueforge-sdk/src/core/fetcher/requestWithRetries.ts b/packages/trueforge-sdk/src/core/fetcher/requestWithRetries.ts index 5e66b9330e..eb7c273e28 100644 --- a/packages/trueforge-sdk/src/core/fetcher/requestWithRetries.ts +++ b/packages/trueforge-sdk/src/core/fetcher/requestWithRetries.ts @@ -48,9 +48,32 @@ function getRetryDelayFromHeaders(response: Response, retryAttempt: number): num return addSymmetricJitter(Math.min(INITIAL_RETRY_DELAY * 2 ** retryAttempt, MAX_RETRY_DELAY)); } +/** + * Waits `ms` milliseconds, or rejects as soon as `abortSignal` aborts. The timer and + * the abort listener are always removed, so a cancelled wait never keeps the process alive. + */ +function sleep(ms: number, abortSignal: AbortSignal | undefined): Promise { + return new Promise((resolve, reject) => { + if (abortSignal?.aborted) { + reject(abortSignal.reason); + return; + } + const onAbort = () => { + clearTimeout(timeoutId); + reject(abortSignal?.reason); + }; + const timeoutId = setTimeout(() => { + abortSignal?.removeEventListener("abort", onAbort); + resolve(); + }, ms); + abortSignal?.addEventListener("abort", onAbort, { once: true }); + }); +} + export async function requestWithRetries( requestFn: () => Promise, maxRetries: number = DEFAULT_MAX_RETRIES, + abortSignal?: AbortSignal, ): Promise { let response: Response = await requestFn(); @@ -58,7 +81,7 @@ export async function requestWithRetries( if (isRetryableStatusCode(response.status)) { const delay = getRetryDelayFromHeaders(response, i); - await new Promise((resolve) => setTimeout(resolve, delay)); + await sleep(delay, abortSignal); response = await requestFn(); } else { break; diff --git a/packages/trueforge-sdk/src/core/fetcher/signals.ts b/packages/trueforge-sdk/src/core/fetcher/signals.ts index ba74c4d02b..925527b695 100644 --- a/packages/trueforge-sdk/src/core/fetcher/signals.ts +++ b/packages/trueforge-sdk/src/core/fetcher/signals.ts @@ -1,4 +1,4 @@ -const TIMEOUT = "timeout"; +export const TIMEOUT = "timeout"; export function getTimeoutSignal(timeoutMs: number): { signal: AbortSignal; abortId: ReturnType } { const controller = new AbortController(); diff --git a/packages/trueforge-sdk/src/core/stream/Stream.ts b/packages/trueforge-sdk/src/core/stream/Stream.ts index c8acd44310..bb2286bd8e 100644 --- a/packages/trueforge-sdk/src/core/stream/Stream.ts +++ b/packages/trueforge-sdk/src/core/stream/Stream.ts @@ -46,6 +46,8 @@ export declare namespace Stream { type: "sse"; streamTerminator?: string; eventDiscriminator?: string; + /** Event names whose union variant wraps the `data:` payload as `{ , data }`. */ + envelopeEvents?: string[]; resumable?: boolean; } } @@ -78,6 +80,7 @@ export class Stream implements AsyncIterable { private messageTerminator: string; private streamTerminator: string | undefined; private eventDiscriminator: string | undefined; + private envelopeEvents: Set | undefined; private resumable: boolean; private reconnectionEnabled: boolean; private maxReconnectionAttempts: number; @@ -105,6 +108,7 @@ export class Stream implements AsyncIterable { this.messageTerminator = "\n"; this.streamTerminator = eventShape.streamTerminator; this.eventDiscriminator = eventShape.eventDiscriminator; + this.envelopeEvents = eventShape.envelopeEvents != null ? new Set(eventShape.envelopeEvents) : undefined; this.resumable = eventShape.resumable ?? false; } else { this.messageTerminator = eventShape.messageTerminator; @@ -448,6 +452,9 @@ export class Stream implements AsyncIterable { if (this.eventDiscriminator == null || eventType == null) { return parsed; } + if (this.envelopeEvents?.has(eventType)) { + return { [this.eventDiscriminator]: eventType, data: parsed }; + } if (parsed == null || typeof parsed !== "object" || Array.isArray(parsed)) { return parsed; } diff --git a/packages/trueforge-sdk/src/core/url/encodePathParam.ts b/packages/trueforge-sdk/src/core/url/encodePathParam.ts index 19b9012442..052f1dfc81 100644 --- a/packages/trueforge-sdk/src/core/url/encodePathParam.ts +++ b/packages/trueforge-sdk/src/core/url/encodePathParam.ts @@ -1,4 +1,7 @@ -export function encodePathParam(param: unknown): string { +export function encodePathParam( + param: unknown, + InvalidPathParameterError?: new (args: { message: string }) => Error, +): string { if (param === null) { return "null"; } @@ -14,5 +17,11 @@ export function encodePathParam(param: unknown): string { param = String(param); break; } - return encodeURIComponent(param as string | number | boolean); + const encoded = encodeURIComponent(param as string | number | boolean); + // "." and ".." are dot segments: URL resolution would remove them and send the request to a different path. + if (encoded === "." || encoded === "..") { + const message = `Invalid path parameter "${encoded}": "." and ".." are not allowed because they change the request path.`; + throw InvalidPathParameterError != null ? new InvalidPathParameterError({ message }) : new Error(message); + } + return encoded; } diff --git a/packages/trueforge-sdk/src/core/utils/setObjectProperty.ts b/packages/trueforge-sdk/src/core/utils/setObjectProperty.ts index 5528af2486..9daac39e84 100644 --- a/packages/trueforge-sdk/src/core/utils/setObjectProperty.ts +++ b/packages/trueforge-sdk/src/core/utils/setObjectProperty.ts @@ -1,12 +1,13 @@ /** - * Sets the value at path of object. If a portion of path doesn’t exist it’s created. This is - * inspired by Lodash's set function, but is simplified to accommodate our use case. - * For more details, see https://lodash.com/docs/4.17.15#set. + * Returns a copy of object with the value at path set. If a portion of path doesn’t exist it’s created. + * Each object along path is shallow-copied, so the original object (and any nested objects it + * shares with the copy) is never modified. This is inspired by Lodash's set function, but is + * simplified to accommodate our use case. For more details, see https://lodash.com/docs/4.17.15#set. * - * @param object The object to modify. + * @param object The object to copy. * @param path The path of the property to set. * @param value The value to set. - * @return Returns object. + * @return Returns the updated copy of object. */ export function setObjectProperty(object: T, path: string, value: any): T { if (object == null) { @@ -19,15 +20,19 @@ export function setObjectProperty(object: T, path: string, val return object; } - let current: Record = object; + const result: Record = { ...object }; + let current: Record = result; for (let i = 0; i < keys.length - 1; i++) { const key = keys[i]; if (key == null) { // Unreachable. continue; } - if (!current[key] || typeof current[key] !== "object") { - current[key] = {}; + const next = current[key]; + if (Array.isArray(next)) { + current[key] = [...next]; + } else { + current[key] = next != null && typeof next === "object" ? { ...next } : {}; } current = current[key] as Record; } @@ -35,9 +40,9 @@ export function setObjectProperty(object: T, path: string, val const lastKey = keys[keys.length - 1]; if (lastKey == null) { // Unreachable. - return object; + return result as T; } current[lastKey] = value; - return object; + return result as T; } diff --git a/packages/trueforge-sdk/src/errors/TrueForgeError.ts b/packages/trueforge-sdk/src/errors/TrueForgeError.ts index 4a17471c66..baee93eb72 100644 --- a/packages/trueforge-sdk/src/errors/TrueForgeError.ts +++ b/packages/trueforge-sdk/src/errors/TrueForgeError.ts @@ -38,7 +38,21 @@ export class TrueForgeError extends Error { } public get requestId(): string | undefined { - return this.rawResponse?.headers?.get("x-request-id") ?? undefined; + const headers = this.rawResponse?.headers; + if (headers == null) { + return undefined; + } + const requestId = headers.get("X-Request-Id"); + if (requestId != null) { + return requestId; + } + let fallback: string | undefined; + headers.forEach((value, name) => { + if (fallback === undefined && /request[-_]?id$/i.test(name)) { + fallback = value; + } + }); + return fallback; } } diff --git a/packages/trueforge-sdk/tests/unit/fetcher/Fetcher.test.ts b/packages/trueforge-sdk/tests/unit/fetcher/Fetcher.test.ts index 6c17624228..ba00149406 100644 --- a/packages/trueforge-sdk/tests/unit/fetcher/Fetcher.test.ts +++ b/packages/trueforge-sdk/tests/unit/fetcher/Fetcher.test.ts @@ -259,4 +259,197 @@ describe("Test fetcherImpl", () => { expect(body.bodyUsed).toBe(true); } }); + + it("should return a non-json error instead of a body when a successful response has malformed JSON", async () => { + global.fetch = vi.fn().mockResolvedValue( + new Response('{"broken":', { + status: 200, + headers: { "Content-Type": "application/json", "X-Request-Id": "req-200" }, + }), + ); + + const result = await fetcherImpl({ url: "https://example.com/items", method: "GET", maxRetries: 0 }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toEqual({ reason: "non-json", statusCode: 200, rawBody: '{"broken":' }); + expect(result.rawResponse.status).toBe(200); + expect(result.rawResponse.headers.get("X-Request-Id")).toBe("req-200"); + } + }); + + it("should still resolve when a successful response has a plain-text body", async () => { + global.fetch = vi + .fn() + .mockResolvedValue(new Response("OK", { status: 200, headers: { "Content-Type": "text/plain" } })); + + const result = await fetcherImpl({ url: "https://example.com/items", method: "DELETE", maxRetries: 0 }); + + expect(result.ok).toBe(true); + }); + + it("should keep the status and raw body when a JSON error response is malformed", async () => { + global.fetch = vi.fn().mockResolvedValue( + new Response("Bad Gateway", { + status: 502, + headers: { "Content-Type": "application/json", "X-Request-Id": "req-502" }, + }), + ); + + const result = await fetcherImpl({ url: "https://example.com/items", method: "GET", maxRetries: 0 }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toEqual({ reason: "status-code", statusCode: 502, body: "Bad Gateway" }); + expect(result.rawResponse.headers.get("X-Request-Id")).toBe("req-502"); + } + }); + + it("should return the raw text for a malformed error body without a content type", async () => { + global.fetch = vi.fn().mockResolvedValue(new Response(new TextEncoder().encode('{"broken":'), { status: 500 })); + + const result = await fetcherImpl({ url: "https://example.com/items", method: "GET", maxRetries: 0 }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toEqual({ reason: "status-code", statusCode: 500, body: '{"broken":' }); + } + }); + + it("should still parse valid JSON error bodies and keep plain-text error bodies", async () => { + const fetchMock = vi.fn(); + fetchMock + .mockResolvedValueOnce( + new Response(JSON.stringify({ error_code: "INVALID" }), { + status: 400, + headers: { "Content-Type": "application/json" }, + }), + ) + .mockResolvedValueOnce(new Response("Service Unavailable", { status: 503 })); + global.fetch = fetchMock; + + const json = await fetcherImpl({ url: "https://example.com/items", method: "GET", maxRetries: 0 }); + const text = await fetcherImpl({ url: "https://example.com/items", method: "GET", maxRetries: 0 }); + + expect(json.ok || json.error).toEqual({ + reason: "status-code", + statusCode: 400, + body: { error_code: "INVALID" }, + }); + expect(text.ok || text.error).toEqual({ reason: "status-code", statusCode: 503, body: "Service Unavailable" }); + }); + + it("should time out while reading a slow JSON body", async () => { + global.fetch = vi.fn().mockImplementation(async (_url: string, init: RequestInit) => { + const body = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('{"data":')); + init.signal?.addEventListener("abort", () => controller.error(init.signal?.reason)); + }, + }); + return new Response(body, { status: 200, headers: { "Content-Type": "application/json" } }); + }); + + const result = await fetcherImpl({ + url: "https://example.com/slow", + method: "GET", + maxRetries: 0, + timeoutMs: 20, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error.reason).toBe("timeout"); + } + }); + + it("should report a timeout when fetch rejects with the timeout reason", async () => { + global.fetch = vi.fn().mockImplementation( + (_url: string, init: RequestInit) => + new Promise((_resolve, reject) => { + init.signal?.addEventListener("abort", () => reject(init.signal?.reason)); + }), + ); + + const result = await fetcherImpl({ + url: "https://example.com/slow", + method: "GET", + maxRetries: 0, + timeoutMs: 20, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error.reason).toBe("timeout"); + } + }); + + it("should not leave timers running after a request succeeds or fails", async () => { + vi.useFakeTimers(); + try { + global.fetch = vi + .fn() + .mockResolvedValueOnce( + new Response(JSON.stringify({ data: "test" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ) + .mockRejectedValueOnce(new TypeError("fetch failed")); + + const success = await fetcherImpl({ + url: "https://example.com", + method: "GET", + maxRetries: 0, + timeoutMs: 1000, + }); + expect(success.ok).toBe(true); + expect(vi.getTimerCount()).toBe(0); + + const failure = await fetcherImpl({ + url: "https://example.com", + method: "GET", + maxRetries: 0, + timeoutMs: 1000, + }); + expect(failure.ok).toBe(false); + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + + it("should clear the previous attempt's timeout before a retry is sent", async () => { + vi.useFakeTimers(); + try { + const timersSeenByAttempt: number[] = []; + let attempt = 0; + global.fetch = vi.fn().mockImplementation(async () => { + timersSeenByAttempt.push(vi.getTimerCount()); + attempt += 1; + return attempt === 1 + ? new Response("busy", { status: 503, headers: { "Retry-After": "1" } }) + : new Response(JSON.stringify({ data: "test" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }); + + const promise = fetcherImpl({ + url: "https://example.com", + method: "GET", + maxRetries: 1, + timeoutMs: 60_000, + }); + await vi.advanceTimersByTimeAsync(2_000); + const result = await promise; + + expect(result.ok).toBe(true); + // The retry only sees its own timeout timer, not the first attempt's. + expect(timersSeenByAttempt).toEqual([1, 1]); + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } + }); }); diff --git a/packages/trueforge-sdk/tests/unit/fetcher/getResponseBody.test.ts b/packages/trueforge-sdk/tests/unit/fetcher/getResponseBody.test.ts index 64ed7461ec..1d4fa2ac5e 100644 --- a/packages/trueforge-sdk/tests/unit/fetcher/getResponseBody.test.ts +++ b/packages/trueforge-sdk/tests/unit/fetcher/getResponseBody.test.ts @@ -1,4 +1,4 @@ -import { getResponseBody } from "../../../src/core/fetcher/getResponseBody"; +import { getResponseBody, isResponseBodyError } from "../../../src/core/fetcher/getResponseBody"; import { RUNTIME } from "../../../src/core/runtime"; @@ -120,4 +120,16 @@ describe("Test getResponseBody", () => { const descriptor = Object.getOwnPropertyDescriptor(result, "__fern_response_ref"); expect(descriptor?.enumerable).toBe(false); }); + + it("should mark malformed JSON as a response body error", async () => { + const result = await getResponseBody(new Response("invalid json")); + expect(isResponseBodyError(result)).toBe(true); + }); + + it("should not treat a JSON body shaped like a failure record as a response body error", async () => { + const lookalike = { ok: false, error: { reason: "non-json", statusCode: 200, rawBody: "x" } }; + const result = await getResponseBody(new Response(JSON.stringify(lookalike))); + expect(result).toEqual(lookalike); + expect(isResponseBodyError(result)).toBe(false); + }); }); diff --git a/packages/trueforge-sdk/tests/unit/fetcher/makePassthroughRequest.test.ts b/packages/trueforge-sdk/tests/unit/fetcher/makePassthroughRequest.test.ts index 07cb846739..802411f92e 100644 --- a/packages/trueforge-sdk/tests/unit/fetcher/makePassthroughRequest.test.ts +++ b/packages/trueforge-sdk/tests/unit/fetcher/makePassthroughRequest.test.ts @@ -342,6 +342,44 @@ describe("makePassthroughRequest", () => { vi.restoreAllMocks(); }); + + it("should reject with an Error named TimeoutError when the request times out", async () => { + mockFetch.mockImplementation( + (_url: string, init: RequestInit) => + new Promise((_resolve, reject) => { + init.signal?.addEventListener("abort", () => reject(init.signal?.reason)); + }), + ); + + const error = await makePassthroughRequest("https://api.example.com", undefined, { + timeoutInSeconds: 0.01, + maxRetries: 0, + fetch: mockFetch, + }).catch((e: unknown) => e); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).name).toBe("TimeoutError"); + }); + + it("should keep the caller's abort reason when the caller aborts", async () => { + mockFetch.mockImplementation( + (_url: string, init: RequestInit) => + new Promise((_resolve, reject) => { + init.signal?.addEventListener("abort", () => reject(init.signal?.reason)); + }), + ); + const controller = new AbortController(); + + const promise = makePassthroughRequest( + "https://api.example.com", + undefined, + { timeoutInSeconds: 10, maxRetries: 0, fetch: mockFetch }, + { abortSignal: controller.signal }, + ).catch((e: unknown) => e); + controller.abort("cancelled"); + + expect(await promise).toBe("cancelled"); + }); }); describe("abort signal", () => { diff --git a/packages/trueforge-sdk/tests/unit/fetcher/makeRequest.test.ts b/packages/trueforge-sdk/tests/unit/fetcher/makeRequest.test.ts index bde194554d..1a7e539a0f 100644 --- a/packages/trueforge-sdk/tests/unit/fetcher/makeRequest.test.ts +++ b/packages/trueforge-sdk/tests/unit/fetcher/makeRequest.test.ts @@ -1,5 +1,6 @@ import type { Mock } from "vitest"; import { + clearResponseTimeout, isCacheNoStoreSupported, makeRequest, resetCacheNoStoreSupported, @@ -155,4 +156,61 @@ describe("Test makeRequest", () => { resetCacheNoStoreSupported(); } }); + + it("should clear the timeout once headers arrive by default", async () => { + vi.useFakeTimers(); + try { + await makeRequest(mockFetch, mockGetUrl, "GET", mockHeaders, undefined, 1000); + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + + it("should keep the timeout running until clearResponseTimeout when keepTimeoutUntilBodyRead is set", async () => { + vi.useFakeTimers(); + try { + const response = await makeRequest( + mockFetch, + mockGetUrl, + "GET", + mockHeaders, + undefined, + 1000, + undefined, + undefined, + undefined, + undefined, + true, + ); + expect(vi.getTimerCount()).toBe(1); + clearResponseTimeout(response); + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + + it("should clear the timeout when fetch rejects", async () => { + vi.useFakeTimers(); + try { + mockFetch.mockRejectedValue(new TypeError("fetch failed")); + await expect(makeRequest(mockFetch, mockGetUrl, "GET", mockHeaders, undefined, 1000)).rejects.toThrow( + "fetch failed", + ); + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + + it("should reject with the timeout reason when the runtime rejects with its own error after the timeout fires", async () => { + mockFetch.mockImplementation( + (_url: string, init: RequestInit) => + new Promise((_resolve, reject) => { + init.signal?.addEventListener("abort", () => reject(new TypeError("invalid_argument"))); + }), + ); + await expect(makeRequest(mockFetch, mockGetUrl, "GET", mockHeaders, undefined, 10)).rejects.toBe("timeout"); + }); }); diff --git a/packages/trueforge-sdk/tests/unit/fetcher/requestWithRetries.test.ts b/packages/trueforge-sdk/tests/unit/fetcher/requestWithRetries.test.ts index 7c98c0abfa..a7536102ef 100644 --- a/packages/trueforge-sdk/tests/unit/fetcher/requestWithRetries.test.ts +++ b/packages/trueforge-sdk/tests/unit/fetcher/requestWithRetries.test.ts @@ -279,4 +279,45 @@ describe("requestWithRetries", () => { expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 60000); expect(response.status).toBe(200); }); + + it("should stop waiting and not retry when aborted during the backoff", async () => { + vi.restoreAllMocks(); + const controller = new AbortController(); + mockFetch.mockImplementation(async () => new Response("", { status: 429, headers: { "Retry-After": "1" } })); + + const responsePromise = requestWithRetries(() => mockFetch(), 2, controller.signal); + const assertion = expect(responsePromise).rejects.toBe("cancelled"); + await vi.advanceTimersByTimeAsync(30); + controller.abort("cancelled"); + await assertion; + + expect(mockFetch).toHaveBeenCalledTimes(1); + expect(vi.getTimerCount()).toBe(0); + }); + + it("should not wait when the signal is already aborted", async () => { + vi.restoreAllMocks(); + const controller = new AbortController(); + controller.abort("cancelled"); + mockFetch.mockImplementation(async () => new Response("", { status: 503 })); + + await expect(requestWithRetries(() => mockFetch(), 2, controller.signal)).rejects.toBe("cancelled"); + expect(mockFetch).toHaveBeenCalledTimes(1); + expect(vi.getTimerCount()).toBe(0); + }); + + it("should still retry normally when a signal is passed but never aborted", async () => { + vi.restoreAllMocks(); + const controller = new AbortController(); + mockFetch + .mockImplementationOnce(async () => new Response("", { status: 503 })) + .mockImplementationOnce(async () => new Response("", { status: 200 })); + + const responsePromise = requestWithRetries(() => mockFetch(), 2, controller.signal); + await vi.runAllTimersAsync(); + const response = await responsePromise; + + expect(response.status).toBe(200); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); }); diff --git a/packages/trueforge-sdk/tests/unit/stream/Stream.test.ts b/packages/trueforge-sdk/tests/unit/stream/Stream.test.ts index 90f7b88716..c60c4ba6b0 100644 --- a/packages/trueforge-sdk/tests/unit/stream/Stream.test.ts +++ b/packages/trueforge-sdk/tests/unit/stream/Stream.test.ts @@ -267,6 +267,27 @@ describe("Stream", () => { ]); }); + it("should wrap data in an envelope for envelope events", async () => { + const mockStream = createReadableStream([ + 'event: entity\ndata: {"id": "1"}\n\nevent: error\ndata: {"code": 3, "message": "bad input"}\n\n', + ]); + const stream = new Stream({ + stream: mockStream, + parse: async (val: unknown) => val, + eventShape: { type: "sse", eventDiscriminator: "event", envelopeEvents: ["entity"] }, + }); + + const messages: unknown[] = []; + for await (const message of stream) { + messages.push(message); + } + + expect(messages).toEqual([ + { event: "entity", data: { id: "1" } }, + { event: "error", code: 3, message: "bad input" }, + ]); + }); + it("should not inject if data already contains discriminator key", async () => { const mockStream = createReadableStream([ 'event: completion\ndata: {"type": "existing", "content": "hello"}\n\n', diff --git a/packages/trueforge-sdk/tests/unit/url/encodePathParam.test.ts b/packages/trueforge-sdk/tests/unit/url/encodePathParam.test.ts new file mode 100644 index 0000000000..4267475f42 --- /dev/null +++ b/packages/trueforge-sdk/tests/unit/url/encodePathParam.test.ts @@ -0,0 +1,46 @@ +import { encodePathParam, join } from "../../../src/core/url/index"; + +class InvalidPathParameterError extends Error { + constructor({ message }: { message: string }) { + super(message); + this.name = "InvalidPathParameterError"; + } +} + +describe("encodePathParam", () => { + it.each([ + ["usr_123", "usr_123"], + ["a/b", "a%2Fb"], + ["a b", "a%20b"], + ["a?b#c", "a%3Fb%23c"], + ["...", "..."], + [".a", ".a"], + ["a..b", "a..b"], + ["%2E%2E", "%252E%252E"], + ["", ""], + [42, "42"], + [true, "true"], + [null, "null"], + [undefined, "undefined"], + ])("encodes %j as %j", (input, expected) => { + expect(encodePathParam(input)).toBe(expected); + }); + + it.each([".", ".."])("rejects the dot segment %j", (input) => { + expect(() => encodePathParam(input)).toThrow(`Invalid path parameter "${input}"`); + }); + + it("throws the provided error class", () => { + expect(() => encodePathParam("..", InvalidPathParameterError)).toThrow(InvalidPathParameterError); + }); + + it("does not throw the provided error class for valid values", () => { + expect(encodePathParam("usr_123", InvalidPathParameterError)).toBe("usr_123"); + }); + + it("keeps encoded values in the path when joined with a base URL", () => { + expect(join("https://api.example.com/api/v2", `/users/${encodePathParam("...")}/roles`)).toBe( + "https://api.example.com/api/v2/users/.../roles", + ); + }); +}); diff --git a/packages/trueforge-sdk/tests/unit/utils/setObjectProperty.test.ts b/packages/trueforge-sdk/tests/unit/utils/setObjectProperty.test.ts index 29f8e638d4..36bf33d6f2 100644 --- a/packages/trueforge-sdk/tests/unit/utils/setObjectProperty.test.ts +++ b/packages/trueforge-sdk/tests/unit/utils/setObjectProperty.test.ts @@ -74,3 +74,39 @@ describe("Test setObjectProperty", () => { expect(result).toEqual(wantObject); }); }); + +describe("setObjectProperty does not modify its input", () => { + test("top-level property", () => { + const original = { cursor: "a", limit: 10 }; + const result = setObjectProperty(original, "cursor", "b"); + expect(result).toEqual({ cursor: "b", limit: 10 }); + expect(original).toEqual({ cursor: "a", limit: 10 }); + }); + + test("nested property", () => { + const options = { offset: 0, count: 2 }; + const original = { id: "x", options }; + const result = setObjectProperty(original, "options.offset", 2); + expect(result).toEqual({ id: "x", options: { offset: 2, count: 2 } }); + expect(original).toEqual({ id: "x", options: { offset: 0, count: 2 } }); + expect(original.options).toBe(options); + expect(result.options).not.toBe(options); + }); + + test("missing nested object", () => { + const original: { id: string; options?: { offset?: number } } = { id: "x" }; + const result = setObjectProperty(original, "options.offset", 2); + expect(result).toEqual({ id: "x", options: { offset: 2 } }); + expect(original).toEqual({ id: "x" }); + }); + + test("array along the path stays an array", () => { + const items = [{ name: "a" }]; + const original = { items }; + const result = setObjectProperty(original, "items.0", { name: "b" }); + expect(Array.isArray(result.items)).toBe(true); + expect(result.items).toEqual([{ name: "b" }]); + expect(original.items).toBe(items); + expect(items).toEqual([{ name: "a" }]); + }); +}); diff --git a/packages/trueforge/src/apis/mcpServers.ts b/packages/trueforge/src/apis/mcpServers.ts index f7f00689f5..a7e2b18d16 100644 --- a/packages/trueforge/src/apis/mcpServers.ts +++ b/packages/trueforge/src/apis/mcpServers.ts @@ -36,7 +36,7 @@ import { putMcpServerRoute, } from '../routes/mcpServerRoutes'; import { findCatalogUsageConflict } from '../runtime/catalogUsage'; -import { resolveMcpServer } from '../runtime/sessionResources'; +import { getMcpConnection } from '../runtime/sessionResources'; import type { AvailableMcpServer, ConfiguredMcpServer, @@ -467,7 +467,7 @@ export function createMcpServersRouter(deps: McpServersRouterDeps< const requestContext = deps.resolveRequestContext(c); const userRef = requestContext.subject.id; // Same url + header resolution as turn execution (store Bearer, DCR, or static headers). - const connection = await resolveMcpServer({ + const connection = await getMcpConnection({ tenant_id: requestContext.tenant_id, name, store: deps.resolveMcpServerStore(c), diff --git a/packages/trueforge/src/apis/turns.ts b/packages/trueforge/src/apis/turns.ts index c3402b361d..26995c0870 100644 --- a/packages/trueforge/src/apis/turns.ts +++ b/packages/trueforge/src/apis/turns.ts @@ -61,8 +61,8 @@ import { StreamGoneError, type EventSubscription, type EventSubscriptionRegistry import { validateSandboxFilePath } from '../runtime/sandboxFilePath'; import { buildTurnSandbox, + getMcpConnection, getModelDetails, - resolveMcpServer, resolveSandboxEnvironment, resolveSandboxProvider, } from '../runtime/sessionResources'; @@ -214,7 +214,7 @@ function createTurnResolver(deps: { }; }, mcp: async name => { - const connection = await resolveMcpServer({ + const connection = await getMcpConnection({ tenant_id, name, store: mcpServerStore, diff --git a/packages/trueforge/src/runtime/sessionResources.ts b/packages/trueforge/src/runtime/sessionResources.ts index 098d473353..9582c6e463 100644 --- a/packages/trueforge/src/runtime/sessionResources.ts +++ b/packages/trueforge/src/runtime/sessionResources.ts @@ -107,11 +107,11 @@ export async function getModelDetails({ } /** - * Resolve a configured MCP server for a turn. + * Load MCP url + headers for a configured server. Returns undefined when unregistered. * * TODO: OAuth header resolvers re-run on every RemoteMCP listTools/callTool; cache or gate later. */ -export async function resolveMcpServer({ +export async function getMcpConnection({ tenant_id, name, store, diff --git a/packages/trueforge/tests/unit/runtime/resolveMcpServer.test.ts b/packages/trueforge/tests/unit/runtime/getMcpConnection.test.ts similarity index 96% rename from packages/trueforge/tests/unit/runtime/resolveMcpServer.test.ts rename to packages/trueforge/tests/unit/runtime/getMcpConnection.test.ts index 55923635df..809b382785 100644 --- a/packages/trueforge/tests/unit/runtime/resolveMcpServer.test.ts +++ b/packages/trueforge/tests/unit/runtime/getMcpConnection.test.ts @@ -7,7 +7,7 @@ import { createSqliteDb } from '../../../src/db/sqlite/client'; import { SqliteMcpServerStore } from '../../../src/db/sqlite/mcp-server-store/SqliteMcpServerStore'; import { SqliteOAuthTokenStore } from '../../../src/db/sqlite/token-store/SqliteOAuthTokenStore'; import { mcpOAuthCallbackUrl } from '../../../src/mcp/auth/mcpOAuthHelpers'; -import { resolveMcpServer } from '../../../src/runtime/sessionResources'; +import { getMcpConnection } from '../../../src/runtime/sessionResources'; jest.mock('undici', () => { const actual = jest.requireActual('undici'); @@ -20,7 +20,7 @@ jest.mock('undici', () => { }; }); -describe('resolveMcpServer', () => { +describe('getMcpConnection', () => { let db: ReturnType; let mcpServerStore: IMcpServerWithAuthStore; let tokenStore: SqliteOAuthTokenStore; @@ -96,7 +96,7 @@ describe('resolveMcpServer', () => { }, }); - const connection = await resolveMcpServer({ + const connection = await getMcpConnection({ tenant_id: 'default', name: 'oauth-mcp', store: mcpServerStore, @@ -155,7 +155,7 @@ describe('resolveMcpServer', () => { }, }); - const connection = await resolveMcpServer({ + const connection = await getMcpConnection({ tenant_id: 'default', name: 'tokened-mcp', store: mcpServerStore, @@ -182,7 +182,7 @@ describe('resolveMcpServer', () => { }, }); - const connection = await resolveMcpServer({ + const connection = await getMcpConnection({ tenant_id: 'default', name: 'open-mcp', store: mcpServerStore, @@ -211,7 +211,7 @@ describe('resolveMcpServer', () => { }, }); - const connection = await resolveMcpServer({ + const connection = await getMcpConnection({ tenant_id: 'default', name: 'header-mcp', store: mcpServerStore, @@ -251,7 +251,7 @@ describe('resolveMcpServer', () => { }, }); - const connection = await resolveMcpServer({ + const connection = await getMcpConnection({ tenant_id: 'default', name: 'tfy-mcp', store, @@ -270,7 +270,7 @@ describe('resolveMcpServer', () => { it('returns undefined when the server is not registered', async () => { await expect( - resolveMcpServer({ + getMcpConnection({ tenant_id: 'default', name: 'missing-mcp', store: mcpServerStore, diff --git a/python/trueforge_sdk/src/trueforge_sdk/core/enum.py b/python/trueforge_sdk/src/trueforge_sdk/core/enum.py index a3d17a67b1..3b31470033 100644 --- a/python/trueforge_sdk/src/trueforge_sdk/core/enum.py +++ b/python/trueforge_sdk/src/trueforge_sdk/core/enum.py @@ -17,4 +17,5 @@ class in all supported Python versions. else: class StrEnum(str, enum.Enum): - pass + def __str__(self) -> str: + return str(self.value) diff --git a/python/trueforge_sdk/src/trueforge_sdk/core/http_client.py b/python/trueforge_sdk/src/trueforge_sdk/core/http_client.py index 3e5a786509..28c971cd15 100644 --- a/python/trueforge_sdk/src/trueforge_sdk/core/http_client.py +++ b/python/trueforge_sdk/src/trueforge_sdk/core/http_client.py @@ -66,9 +66,12 @@ def _parse_retry_after(response_headers: httpx.Headers) -> typing.Optional[float retry_after_ms = response_headers.get("retry-after-ms") if retry_after_ms is not None: try: - return int(retry_after_ms) / 1000 if retry_after_ms > 0 else 0 - except Exception: + retry_after_ms_value = float(retry_after_ms) + except ValueError: pass + else: + if 0 < retry_after_ms_value < float("inf"): + return retry_after_ms_value / 1000 retry_after = response_headers.get("retry-after") if retry_after is None: @@ -184,8 +187,33 @@ def _should_retry(response: httpx.Response) -> bool: ) +_AUTH_HEADERS: typing.FrozenSet[str] = frozenset() + + def _redact_headers(headers: typing.Dict[str, str]) -> typing.Dict[str, str]: - return {k: ("[REDACTED]" if k.lower() in _SENSITIVE_HEADERS else v) for k, v in headers.items()} + return { + k: ("[REDACTED]" if k.lower() in _SENSITIVE_HEADERS or k.lower() in _AUTH_HEADERS else v) + for k, v in headers.items() + } + + +def _merge_headers(*header_dicts: typing.Optional[typing.Dict[str, typing.Any]]) -> typing.Dict[str, typing.Any]: + """ + Merge header dicts left to right. Header names are case-insensitive, so a later + `x-api-key` replaces an earlier `X-API-Key` instead of producing two headers. + """ + merged: typing.Dict[str, typing.Any] = {} + keys_by_lower: typing.Dict[str, str] = {} + for header_dict in header_dicts: + if not header_dict: + continue + for key, value in header_dict.items(): + existing_key = keys_by_lower.get(key.lower()) + if existing_key is not None: + del merged[existing_key] + keys_by_lower[key.lower()] = key + merged[key] = value + return merged def _build_url(base_url: str, path: typing.Optional[str]) -> str: @@ -425,11 +453,11 @@ def request( _request_url = _build_url(base_url, path) _request_headers = jsonable_encoder( remove_none_from_dict( - { - **self.base_headers(), - **(headers if headers is not None else {}), - **(request_options.get("additional_headers", {}) or {} if request_options is not None else {}), - } + _merge_headers( + self.base_headers(), + headers, + request_options.get("additional_headers") if request_options is not None else None, + ) ) ) _request_headers = drop_content_type_without_body( @@ -463,7 +491,12 @@ def request( files=request_files, timeout=timeout, ) - except (httpx.ConnectError, httpx.RemoteProtocolError): + except (httpx.ConnectError, httpx.RemoteProtocolError) as error: + # A protocol error may occur after the server applied the request, even while reading a 200 body. + if isinstance(error, httpx.RemoteProtocolError) and not ( + request_options is not None and request_options.get("retry_remote_protocol_errors", False) + ): + raise if retries < max_retries: time.sleep(_retry_timeout_from_retries(retries=retries)) return self.request( @@ -594,11 +627,11 @@ def stream( _request_url = _build_url(base_url, path) _request_headers = jsonable_encoder( remove_none_from_dict( - { - **self.base_headers(), - **(headers if headers is not None else {}), - **(request_options.get("additional_headers", {}) if request_options is not None else {}), - } + _merge_headers( + self.base_headers(), + headers, + request_options.get("additional_headers") if request_options is not None else None, + ) ) ) _request_headers = drop_content_type_without_body( @@ -735,11 +768,11 @@ async def request( _request_url = _build_url(base_url, path) _request_headers = jsonable_encoder( remove_none_from_dict( - { - **_headers, - **(headers if headers is not None else {}), - **(request_options.get("additional_headers", {}) or {} if request_options is not None else {}), - } + _merge_headers( + _headers, + headers, + request_options.get("additional_headers") if request_options is not None else None, + ) ) ) _request_headers = drop_content_type_without_body( @@ -773,7 +806,12 @@ async def request( files=request_files, timeout=timeout, ) - except (httpx.ConnectError, httpx.RemoteProtocolError): + except (httpx.ConnectError, httpx.RemoteProtocolError) as error: + # A protocol error may occur after the server applied the request, even while reading a 200 body. + if isinstance(error, httpx.RemoteProtocolError) and not ( + request_options is not None and request_options.get("retry_remote_protocol_errors", False) + ): + raise if retries < max_retries: await asyncio.sleep(_retry_timeout_from_retries(retries=retries)) return await self.request( @@ -907,11 +945,11 @@ async def stream( _request_url = _build_url(base_url, path) _request_headers = jsonable_encoder( remove_none_from_dict( - { - **_headers, - **(headers if headers is not None else {}), - **(request_options.get("additional_headers", {}) if request_options is not None else {}), - } + _merge_headers( + _headers, + headers, + request_options.get("additional_headers") if request_options is not None else None, + ) ) ) _request_headers = drop_content_type_without_body( diff --git a/python/trueforge_sdk/src/trueforge_sdk/core/jsonable_encoder.py b/python/trueforge_sdk/src/trueforge_sdk/core/jsonable_encoder.py index f638cc9a42..2608ce6bd8 100644 --- a/python/trueforge_sdk/src/trueforge_sdk/core/jsonable_encoder.py +++ b/python/trueforge_sdk/src/trueforge_sdk/core/jsonable_encoder.py @@ -15,7 +15,7 @@ from pathlib import PurePath from types import GeneratorType from typing import Any, Callable, Dict, List, Optional, Set, Union -from urllib.parse import quote +from urllib.parse import quote, unquote import pydantic from .datetime_utils import serialize_datetime @@ -118,7 +118,11 @@ def encode_path_param(obj: Any) -> str: """ if isinstance(obj, bool): return "true" if obj else "false" - return str(jsonable_encoder(obj)) + value = str(jsonable_encoder(obj)) + # The value is not percent-encoded here, so check every segment it would add to the path. + for segment in unquote(value).split("/"): + _reject_dot_segment(segment) + return value def quote_path_param(obj: Any) -> str: @@ -130,4 +134,12 @@ def quote_path_param(obj: Any) -> str: """ if isinstance(obj, bool): return "true" if obj else "false" - return quote(str(jsonable_encoder(obj)), safe="") + value = str(jsonable_encoder(obj)) + _reject_dot_segment(value) + return quote(value, safe="") + + +def _reject_dot_segment(value: str) -> None: + # "." and ".." are dot-segments that HTTP clients resolve, which would change the request path. + if value in (".", ".."): + raise ValueError(f'Invalid path parameter value "{value}": "." and ".." are not allowed.') diff --git a/python/trueforge_sdk/src/trueforge_sdk/core/pagination.py b/python/trueforge_sdk/src/trueforge_sdk/core/pagination.py index 760b08995f..a4bc1947ef 100644 --- a/python/trueforge_sdk/src/trueforge_sdk/core/pagination.py +++ b/python/trueforge_sdk/src/trueforge_sdk/core/pagination.py @@ -3,7 +3,20 @@ from __future__ import annotations from dataclasses import dataclass -from typing import AsyncIterator, Awaitable, Callable, Generic, Iterator, List, Optional, TypeVar +from typing import ( + Any, + AsyncIterator, + Awaitable, + Callable, + Generic, + Iterator, + List, + Mapping, + Optional, + Sequence, + TypeVar, + get_args, +) # Generic to represent the underlying type of the results within a page T = TypeVar("T") @@ -80,3 +93,73 @@ async def iter_pages(self) -> AsyncIterator[AsyncPager[T, R]]: async def next_page(self) -> Optional[AsyncPager[T, R]]: return await self.get_next() if self.get_next is not None else None + + +def get_nested_page_value(container: Any, path: Sequence[str], default: Any = None) -> Any: + """ + Read a nested paging value (e.g. `options.offset`) from a request parameter that may be + a dict, a model, None, or omitted (`...`). Returns `default` when any segment is missing. + """ + value = container + for key in path: + if value is None or value is ...: + return default + value = value.get(key) if isinstance(value, Mapping) else getattr(value, key, None) + return default if value is None or value is ... else value + + +def with_nested_page_value(container: Any, path: Sequence[str], value: Any, container_type: Any = None) -> Any: + """ + Return a copy of a request parameter with the nested paging value at `path` replaced, + creating omitted containers and keeping every other field. When `container_type` (or the + annotation of the parent model's field) is a model, created containers are built through that + model so field aliases (e.g. `page_offset` -> `pageOffset`) apply on the wire. + """ + if len(path) == 0: + return value + key, rest = path[0], path[1:] + if container is None or container is ...: + return _build_container(container_type, {key: with_nested_page_value(None, rest, value)}) + if isinstance(container, Mapping): + return {**container, key: with_nested_page_value(container.get(key), rest, value)} + updated = with_nested_page_value( + getattr(container, key, None), rest, value, _get_field_annotation(type(container), key) + ) + if hasattr(container, "model_copy"): + return container.model_copy(update={key: updated}) + return container.copy(update={key: updated}) + + +def _build_container(container_type: Any, data: Mapping[str, Any]) -> Any: + model = _find_model_class(container_type) + if model is None: + return data + try: + if hasattr(model, "model_validate"): + return model.model_validate(data) + return model.parse_obj(data) + except (TypeError, ValueError): + return data + + +def _find_model_class(annotation: Any) -> Any: + models = [] + pending = [annotation] + while pending: + candidate = pending.pop() + if isinstance(candidate, type): + if hasattr(candidate, "model_validate") or hasattr(candidate, "parse_obj"): + models.append(candidate) + else: + pending.extend(get_args(candidate)) + return models[0] if len(models) == 1 else None + + +def _get_field_annotation(model: Any, key: str) -> Any: + fields = getattr(model, "model_fields", None) + if isinstance(fields, Mapping): + return fields[key].annotation if key in fields else None + legacy_fields = getattr(model, "__fields__", None) + if isinstance(legacy_fields, Mapping) and key in legacy_fields: + return legacy_fields[key].outer_type_ + return None diff --git a/python/trueforge_sdk/src/trueforge_sdk/core/pydantic_utilities.py b/python/trueforge_sdk/src/trueforge_sdk/core/pydantic_utilities.py index 70816b99fc..4e63e0fb3e 100644 --- a/python/trueforge_sdk/src/trueforge_sdk/core/pydantic_utilities.py +++ b/python/trueforge_sdk/src/trueforge_sdk/core/pydantic_utilities.py @@ -14,6 +14,7 @@ Callable, ClassVar, Dict, + FrozenSet, List, Mapping, Optional, @@ -37,8 +38,8 @@ IS_PYDANTIC_V2 = pydantic.VERSION.startswith("2.") if IS_PYDANTIC_V2: - _datetime_adapter = pydantic.TypeAdapter(dt.datetime) # type: ignore[attr-defined] - _date_adapter = pydantic.TypeAdapter(dt.date) # type: ignore[attr-defined] + _datetime_adapter = pydantic.TypeAdapter(dt.datetime) # type: ignore[attr-defined,var-annotated] + _date_adapter = pydantic.TypeAdapter(dt.date) # type: ignore[attr-defined,var-annotated] def parse_datetime(value: Any) -> dt.datetime: # type: ignore[misc] if isinstance(value, dt.datetime): @@ -129,7 +130,7 @@ def _decimal_encoder(dec_value: Any) -> Any: from pydantic.typing import is_union as is_union # type: ignore[no-redef] from .datetime_utils import serialize_datetime -from .serialization import convert_and_respect_annotation_metadata +from .serialization import convert_and_respect_annotation_metadata, get_field_to_alias_mapping from typing_extensions import TypeAlias T = TypeVar("T") @@ -237,6 +238,59 @@ def _coerce_keys_to_aliases(model: type, data: Any) -> Any: return rewritten +_declared_keys_cache: "weakref.WeakKeyDictionary[type, Tuple[FrozenSet[str], FrozenSet[str]]]" = ( + weakref.WeakKeyDictionary() +) + + +def _get_declared_keys(model: type) -> Tuple[FrozenSet[str], FrozenSet[str]]: + """ + Keys that refer to a declared field (field names, Pydantic aliases and FieldMetadata aliases), together with + their casefolded forms. Computed once per model class. + """ + cached = _declared_keys_cache.get(model) + if cached is None: + fields: Mapping[str, Any] = ( + getattr(model, "model_fields", {}) if IS_PYDANTIC_V2 else getattr(model, "__fields__", {}) + ) + keys: Set[str] = set(fields) + for field in fields.values(): + alias = getattr(field, "alias", None) + if isinstance(alias, str): + keys.add(alias) + keys.update(get_field_to_alias_mapping(model).values()) + cached = (frozenset(keys), frozenset(key.casefold() for key in keys)) + _declared_keys_cache[model] = cached + return cached + + +def _allows_extra_fields(model: type) -> bool: + if IS_PYDANTIC_V2: + return getattr(model, "model_config", {}).get("extra") == "allow" + extra = getattr(getattr(model, "__config__", None), "extra", None) + return getattr(extra, "value", extra) == "allow" + + +def _drop_extras_colliding_with_fields(model: type, data: Any) -> Any: + """ + Drop unknown keys that match a declared field case-insensitively (e.g. `VALUE` next to `value`). Kept as + extras, they would be sent again after the declared field, and a case-insensitive decoder such as Go's + encoding/json would read them instead of it. + """ + if not isinstance(data, Mapping) or not _allows_extra_fields(model): + return data + + declared_keys, declared_casefolded = _get_declared_keys(model) + colliding = { + key + for key in data + if isinstance(key, str) and key not in declared_keys and key.casefold() in declared_casefolded + } + if not colliding: + return data + return {key: value for key, value in data.items() if key not in colliding} + + def parse_obj_as(type_: Type[T], object_: Any) -> T: # convert_and_respect_annotation_metadata is required for TypedDict aliasing. # @@ -279,7 +333,7 @@ class UniversalBaseModel(pydantic.BaseModel): @pydantic.model_validator(mode="before") # type: ignore[attr-defined] @classmethod def _coerce_field_names_to_aliases(cls, data: Any) -> Any: - return _coerce_keys_to_aliases(cls, data) + return _coerce_keys_to_aliases(cls, _drop_extras_colliding_with_fields(cls, data)) @pydantic.model_serializer(mode="plain", when_used="json") # type: ignore[attr-defined] def serialize_model(self) -> Any: # type: ignore[name-defined] @@ -295,7 +349,7 @@ class Config: @pydantic.root_validator(pre=True) def _coerce_field_names_to_aliases(cls, values: Any) -> Any: - return _coerce_keys_to_aliases(cls, values) # type: ignore[arg-type] + return _coerce_keys_to_aliases(cls, _drop_extras_colliding_with_fields(cls, values)) # type: ignore[arg-type] @classmethod def model_construct(cls: Type["Model"], _fields_set: Optional[Set[str]] = None, **values: Any) -> "Model": diff --git a/python/trueforge_sdk/src/trueforge_sdk/core/request_options.py b/python/trueforge_sdk/src/trueforge_sdk/core/request_options.py index caa6f669b3..bd6bc01d6f 100644 --- a/python/trueforge_sdk/src/trueforge_sdk/core/request_options.py +++ b/python/trueforge_sdk/src/trueforge_sdk/core/request_options.py @@ -20,6 +20,8 @@ class RequestOptions(typing.TypedDict, total=False): - max_retries: int. The max number of retries to attempt if the API call fails. + - retry_remote_protocol_errors: bool. Retry server disconnects and incomplete responses. Defaults to False, since the server may already have applied the request. Enable only when replaying the request is safe; bounded by max_retries. + - additional_headers: typing.Dict[str, typing.Any]. A dictionary containing additional parameters to spread into the request's header dict - additional_query_parameters: typing.Dict[str, typing.Any]. A dictionary containing additional parameters to spread into the request's query parameters dict @@ -32,6 +34,7 @@ class RequestOptions(typing.TypedDict, total=False): timeout: NotRequired[float] timeout_in_seconds: NotRequired[int] max_retries: NotRequired[int] + retry_remote_protocol_errors: NotRequired[bool] additional_headers: NotRequired[typing.Dict[str, typing.Any]] additional_query_parameters: NotRequired[typing.Dict[str, typing.Any]] additional_body_parameters: NotRequired[typing.Dict[str, typing.Any]] diff --git a/python/trueforge_sdk/src/trueforge_sdk/core/unchecked_base_model.py b/python/trueforge_sdk/src/trueforge_sdk/core/unchecked_base_model.py index c5deec1f5d..a82155aab3 100644 --- a/python/trueforge_sdk/src/trueforge_sdk/core/unchecked_base_model.py +++ b/python/trueforge_sdk/src/trueforge_sdk/core/unchecked_base_model.py @@ -127,9 +127,16 @@ def construct( extras = {} pydantic_alias_fields = [field.alias for field in fields.values()] internal_alias_fields = list(field_aliases.values()) + declared_keys_casefolded = { + key.casefold() for key in (*fields, *pydantic_alias_fields, *internal_alias_fields) if isinstance(key, str) + } for key, value in values.items(): # If the key is not a field by name, nor an alias to a field, then it's extra if (key not in pydantic_alias_fields and key not in internal_alias_fields) and key not in fields: + # Drop extras that match a declared field case-insensitively (e.g. `VALUE` next to `value`): they + # would be sent again after the field, and case-insensitive decoders would read them instead of it. + if isinstance(key, str) and key.casefold() in declared_keys_casefolded: + continue if IS_PYDANTIC_V2: extras[key] = value else: @@ -305,6 +312,10 @@ def _convert_undiscriminated_union_type( except Exception: continue + # No member matched (e.g. a variant added after this SDK was generated, or a missing + # discriminant): keep the original data rather than dropping it. + return object_ + def _convert_union_type( type_: typing.Type[typing.Any], diff --git a/python/trueforge_sdk/src/trueforge_sdk/types/available_model.py b/python/trueforge_sdk/src/trueforge_sdk/types/available_model.py index 2f9024d0f1..b74b1a4f3d 100644 --- a/python/trueforge_sdk/src/trueforge_sdk/types/available_model.py +++ b/python/trueforge_sdk/src/trueforge_sdk/types/available_model.py @@ -24,7 +24,7 @@ class AvailableModel(UncheckedBaseModel): provider: AvailableModelProvider warning: typing.Optional[str] = pydantic.Field(default=None) """ - Warning for not so capable models. + Set when reasoning effort or context length is not configured; may affect TrueForge performance. """ if IS_PYDANTIC_V2: