fix: handle ambiguous prompt transport failures
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
import { describe, expect, mock, test } from 'bun:test';
|
||||
import { beforeEach, describe, expect, mock, test } from 'bun:test';
|
||||
|
||||
type ConfigResponse = { data: Record<string, unknown> };
|
||||
|
||||
@@ -6,6 +6,15 @@ type ConfigResponse = { data: Record<string, unknown> };
|
||||
|
||||
const configResolvers: Array<(response: ConfigResponse) => void> = [];
|
||||
let configCalls = 0;
|
||||
const promptAsyncCalls: unknown[][] = [];
|
||||
const promptAsyncResults: Array<unknown> = [];
|
||||
|
||||
const promptAsyncMock = mock(async (...args: unknown[]) => {
|
||||
promptAsyncCalls.push(args);
|
||||
const next = promptAsyncResults.shift();
|
||||
if (next instanceof Error) throw next;
|
||||
return next ?? { response: new Response(null, { status: 200 }) };
|
||||
});
|
||||
|
||||
mock.module('@opencode-ai/sdk/v2', () => ({
|
||||
createOpencodeClient: mock(() => ({
|
||||
@@ -17,6 +26,9 @@ mock.module('@opencode-ai/sdk/v2', () => ({
|
||||
});
|
||||
}),
|
||||
},
|
||||
session: {
|
||||
promptAsync: promptAsyncMock,
|
||||
},
|
||||
})),
|
||||
}));
|
||||
|
||||
@@ -47,6 +59,11 @@ mock.module('@/lib/startupTrace', () => ({
|
||||
|
||||
const { opencodeClient } = await import(`./client?cache-test=${Date.now()}`);
|
||||
|
||||
beforeEach(() => {
|
||||
promptAsyncCalls.length = 0;
|
||||
promptAsyncResults.length = 0;
|
||||
});
|
||||
|
||||
describe('opencodeClient getConfig cache', () => {
|
||||
test('cleared stale in-flight requests do not repopulate cache or delete newer in-flight requests', async () => {
|
||||
const first = opencodeClient.getConfig('/workspace/project');
|
||||
@@ -72,3 +89,54 @@ describe('opencodeClient getConfig cache', () => {
|
||||
expect(configCalls).toBe(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe('opencodeClient prompt retry behavior', () => {
|
||||
const sendPrompt = (providerID = 'anthropic') => opencodeClient.sendMessage({
|
||||
id: 'ses_1',
|
||||
providerID,
|
||||
modelID: 'claude-sonnet',
|
||||
text: 'hello',
|
||||
});
|
||||
|
||||
test('does not retry 504 prompt responses because the POST may already be accepted', async () => {
|
||||
promptAsyncResults.push({ response: new Response('gateway timeout', { status: 504 }) });
|
||||
|
||||
let error: unknown = null;
|
||||
try {
|
||||
await sendPrompt('anthropic-504');
|
||||
} catch (caught) {
|
||||
error = caught;
|
||||
}
|
||||
|
||||
expect(promptAsyncCalls.length).toBe(1);
|
||||
expect(error instanceof Error ? error.message : String(error)).toContain('Failed to send message (504)');
|
||||
});
|
||||
|
||||
test('does not retry transport failures because the tunnel may have lost only the response', async () => {
|
||||
promptAsyncResults.push(new TypeError('Failed to fetch'));
|
||||
|
||||
let error: unknown = null;
|
||||
try {
|
||||
await sendPrompt('anthropic-network');
|
||||
} catch (caught) {
|
||||
error = caught;
|
||||
}
|
||||
|
||||
expect(promptAsyncCalls.length).toBe(1);
|
||||
expect(error instanceof Error ? error.message : String(error)).toContain('Failed to fetch');
|
||||
});
|
||||
|
||||
test('does not retry 503 prompt responses because proxy errors can be ambiguous too', async () => {
|
||||
promptAsyncResults.push({ response: new Response('starting', { status: 503 }) });
|
||||
|
||||
let error: unknown = null;
|
||||
try {
|
||||
await sendPrompt('anthropic-503');
|
||||
} catch (caught) {
|
||||
error = caught;
|
||||
}
|
||||
|
||||
expect(promptAsyncCalls.length).toBe(1);
|
||||
expect(error instanceof Error ? error.message : String(error)).toContain('Failed to send message (503)');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -22,8 +22,6 @@ import {
|
||||
assertProviderCircuitClosed,
|
||||
recordProviderSuccess,
|
||||
recordProviderError,
|
||||
shouldRetry,
|
||||
getRetryDelayMs,
|
||||
} from "./provider-tracker";
|
||||
|
||||
// Use relative path by default (works with both dev and nginx proxy server)
|
||||
@@ -120,12 +118,6 @@ const ascendingId = (prefix: "msg"): string => {
|
||||
return `${prefix}_${hex}${randomBase62(ID_RANDOM_LENGTH)}`;
|
||||
};
|
||||
|
||||
const isRetryableFetchError = (error: unknown): boolean => {
|
||||
if (error instanceof DOMException && error.name === 'AbortError') return true;
|
||||
if (error instanceof TypeError) return true;
|
||||
return false;
|
||||
};
|
||||
|
||||
const ensureAbsoluteBaseUrl = (candidate: string): string => {
|
||||
const normalized = typeof candidate === "string" && candidate.trim().length > 0 ? candidate.trim() : "/api";
|
||||
|
||||
@@ -764,8 +756,8 @@ class OpencodeService {
|
||||
};
|
||||
directory?: string | null;
|
||||
}): Promise<string> {
|
||||
// Reuse one client-side message ID across retries. The server accepts this
|
||||
// as the real user message ID, making ambiguous network retries idempotent.
|
||||
// Use the optimistic/client-generated ID as the real user message ID so SSE
|
||||
// can reconcile the echoed server message in-place.
|
||||
const messageId = params.messageId ?? ascendingId("msg");
|
||||
|
||||
// Build parts array using SDK types (TextPartInput | FilePartInput) plus lightweight agent parts
|
||||
@@ -848,74 +840,55 @@ class OpencodeService {
|
||||
|
||||
assertProviderCircuitClosed(params.providerID);
|
||||
|
||||
let response!: Response;
|
||||
let response: Response;
|
||||
|
||||
for (let attempt = 0; attempt < 3; attempt++) {
|
||||
try {
|
||||
const result = await this.client.session.promptAsync({
|
||||
sessionID: params.id,
|
||||
...(requestDirectory ? { directory: requestDirectory } : {}),
|
||||
model: {
|
||||
providerID: params.providerID,
|
||||
modelID: params.modelID,
|
||||
},
|
||||
agent: params.agent,
|
||||
variant: params.variant,
|
||||
messageID: messageId,
|
||||
...(params.delivery ? { delivery: params.delivery } : {}),
|
||||
...(params.format ? { format: params.format } : {}),
|
||||
parts,
|
||||
});
|
||||
if (result.response instanceof Response) {
|
||||
response = result.response;
|
||||
} else if (result.error) {
|
||||
const status = (result as SdkResult<unknown>).response?.status || 500;
|
||||
response = new Response(JSON.stringify(result.error), { status });
|
||||
} else {
|
||||
response = new Response(JSON.stringify(result.data ?? true), { status: 200 });
|
||||
}
|
||||
} catch (error) {
|
||||
if (attempt < 2 && isRetryableFetchError(error)) {
|
||||
const delay = getRetryDelayMs(attempt);
|
||||
console.warn(
|
||||
`[prompt] fetch failed for ${params.providerID}/${params.modelID} (attempt ${attempt + 1}/3), retrying in ${delay}ms`,
|
||||
(error as Error)?.message
|
||||
);
|
||||
await new Promise((resolve) => setTimeout(resolve, delay));
|
||||
continue;
|
||||
}
|
||||
recordProviderError(params.providerID);
|
||||
throw error;
|
||||
try {
|
||||
const result = await this.client.session.promptAsync({
|
||||
sessionID: params.id,
|
||||
...(requestDirectory ? { directory: requestDirectory } : {}),
|
||||
model: {
|
||||
providerID: params.providerID,
|
||||
modelID: params.modelID,
|
||||
},
|
||||
agent: params.agent,
|
||||
variant: params.variant,
|
||||
messageID: messageId,
|
||||
...(params.delivery ? { delivery: params.delivery } : {}),
|
||||
...(params.format ? { format: params.format } : {}),
|
||||
parts,
|
||||
});
|
||||
if (result.response instanceof Response) {
|
||||
response = result.response;
|
||||
} else if (result.error) {
|
||||
const status = (result as SdkResult<unknown>).response?.status || 500;
|
||||
response = new Response(JSON.stringify(result.error), { status });
|
||||
} else {
|
||||
response = new Response(JSON.stringify(result.data ?? true), { status: 200 });
|
||||
}
|
||||
|
||||
if (response.ok) {
|
||||
recordProviderSuccess(params.providerID);
|
||||
return messageId;
|
||||
}
|
||||
|
||||
if (shouldRetry(params.providerID, response.status, attempt)) {
|
||||
const delay = getRetryDelayMs(attempt);
|
||||
console.warn(
|
||||
`[prompt] ${response.status} for ${params.providerID}/${params.modelID} (attempt ${attempt + 1}/3), retrying in ${delay}ms`
|
||||
);
|
||||
await new Promise((resolve) => setTimeout(resolve, delay));
|
||||
continue;
|
||||
}
|
||||
|
||||
let detail = '';
|
||||
try {
|
||||
detail = await response.text();
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
const suffix = detail && detail.trim().length > 0 ? `: ${detail.trim()}` : '';
|
||||
const error = new Error(`Failed to send message (${response.status})${suffix}`);
|
||||
recordProviderError(params.providerID, response.status);
|
||||
} catch (error) {
|
||||
// Do not retry prompt_async after a transport failure: through a remote
|
||||
// tunnel the POST may already be running server-side even though the
|
||||
// client lost the response.
|
||||
recordProviderError(params.providerID);
|
||||
throw error;
|
||||
}
|
||||
// Defensive fallback — all loop paths return/throw, but TypeScript
|
||||
// control flow analysis cannot prove exhaustiveness without this.
|
||||
throw new Error('Failed to send message after retries');
|
||||
|
||||
if (response.ok) {
|
||||
recordProviderSuccess(params.providerID);
|
||||
return messageId;
|
||||
}
|
||||
|
||||
let detail = '';
|
||||
try {
|
||||
detail = await response.text();
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
const suffix = detail && detail.trim().length > 0 ? `: ${detail.trim()}` : '';
|
||||
const error = new Error(`Failed to send message (${response.status})${suffix}`) as Error & { status?: number };
|
||||
error.status = response.status;
|
||||
recordProviderError(params.providerID, response.status);
|
||||
throw error;
|
||||
}
|
||||
|
||||
async sendCommand(params: {
|
||||
|
||||
Reference in New Issue
Block a user