Skip to content

fix(api): fail OpenAI session provider errors #115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
cursor wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix(api): fail OpenAI session provider errors #115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions api/src/routes/tools/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3060,6 +3060,13 @@ router.post("/session-message", ...toolMiddleware("session-message"), async (req

// Add user message to history
session.messages.push({ role: "user", content: message });
let userMessagePending = true;
const removePendingUserMessage = (): void => {
if (userMessagePending && session.messages[session.messages.length - 1]?.role === "user") {
session.messages.pop();
}
userMessagePending = false;
};

// Keep conversation history bounded (last 50 messages)
if (session.messages.length > 50) {
Expand All @@ -3084,6 +3091,7 @@ router.post("/session-message", ...toolMiddleware("session-message"), async (req

if (CLAUDE_MODELS.includes(model)) {
if (!getAnthropic()) {
removePendingUserMessage();
res.status(503).json({ ok: false, error: "service_unavailable", message: "Anthropic API key not configured", request_id: reqId() });
return;
}
Expand All @@ -3097,6 +3105,7 @@ router.post("/session-message", ...toolMiddleware("session-message"), async (req
} else if (GPT_MODELS.includes(model)) {
const openaiKey = process.env.OPENAI_API_KEY;
if (!openaiKey) {
removePendingUserMessage();
res.status(503).json({ ok: false, error: "service_unavailable", message: "OpenAI API key not configured", request_id: reqId() });
return;
}
Expand All @@ -3109,12 +3118,28 @@ router.post("/session-message", ...toolMiddleware("session-message"), async (req
headers: { "Content-Type": "application/json", Authorization: `Bearer ${openaiKey}` },
body: JSON.stringify({ model, max_tokens: 2048, messages }),
});
const data = (await resp.json()) as { choices?: Array<{ message?: { content?: string } }> };
responseText = data.choices?.[0]?.message?.content ?? "";
const data = (await resp.json().catch(() => ({}))) as {
choices?: Array<{ message?: { content?: string } }>;
error?: { message?: string };
};
if (!resp.ok) {
removePendingUserMessage();
const detail = data.error?.message ? `OpenAI API error: ${data.error.message}` : `OpenAI API returned ${resp.status}`;
res.status(resp.status === 429 ? 429 : 502).json({ ok: false, error: resp.status === 429 ? "rate_limited" : "openai_error", message: detail, request_id: reqId() });
return;
}
const choiceText = data.choices?.[0]?.message?.content;
if (typeof choiceText !== "string" || choiceText.length === 0) {
removePendingUserMessage();
res.status(502).json({ ok: false, error: "openai_error", message: "OpenAI returned an empty response", request_id: reqId() });
return;
}
responseText = choiceText;
}

// Add assistant response to history
session.messages.push({ role: "assistant", content: responseText });
userMessagePending = false;

res.json({
ok: true,
Expand All @@ -3132,7 +3157,7 @@ router.post("/session-message", ...toolMiddleware("session-message"), async (req
});
} catch (e) {
// Remove the user message we just added since the call failed
session.messages.pop();
removePendingUserMessage();
res.status(500).json({ ok: false, error: "session_message_failed", message: safeErr(e), request_id: reqId() });
}
});
Expand Down
8 changes: 8 additions & 0 deletions api/tests/session-pricing.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,5 +118,13 @@ test("upstream calls send the trimmed window, not the raw stored history", () =>
assert.match(sessionMessageRoute, /context_truncated: true/);
});

test("OpenAI provider errors do not become successful empty assistant messages", () => {
assert.match(sessionMessageRoute, /if \(!resp\.ok\)/, "OpenAI non-2xx responses must be checked");
assert.match(sessionMessageRoute, /removePendingUserMessage\(\);\s*const detail = data\.error\?\.message/s, "failed OpenAI calls must remove the pending user turn");
assert.match(sessionMessageRoute, /error: resp\.status === 429 \? "rate_limited" : "openai_error"/, "provider failure must return an error body");
assert.match(sessionMessageRoute, /typeof choiceText !== "string" \|\| choiceText\.length === 0/, "empty OpenAI choices must not be reported as success");
assert.ok(!/responseText = data\.choices\?\.\[0\]\?\.message\?\.content \?\? ""/.test(sessionMessageRoute), "must not coerce missing OpenAI output into an empty success");
});

if (failures) { console.error(`\n${failures} failure(s)`); process.exit(1); }
console.log("\nAll session-pricing tests passed.");
Loading