lee/puter
1
0
Fork 0
mirror of https://github.com/HeyPuter/puter.git synced 2026-09-25 06:36:00 +00:00
Code Issues Actions 9 Packages Projects Releases Wiki Activity

fix: expose upstream ai errors as not 500s (#3123)

Browse Source
This commit is contained in:
Daniel Salazar
2026-05-17 12:01:22 -07:00
committed by GitHub
parent 41e7228a9f
commit a568ca0f99
7 changed files with 339 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/backend/controllers/drivers/DriverController.ts
+65 -6
View File
@@ -20,7 +20,7 @@
import type { Request, Response } from 'express';
import { Context } from '../../core/context.js';
import { Controller } from '../../core/http/decorators.js';
import { HttpError } from '../../core/http/HttpError.js';
import { HttpError, isHttpError } from '../../core/http/HttpError.js';
import {
acquireDriverConcurrent,
checkDriverRateLimit,
@@ -129,6 +129,63 @@ const XD_HTML = `<!DOCTYPE html>
* additional drivers end up in that bag before this controller is
* instantiated, so they show up here automatically.
*/
/**
* Catch-all upstream-error translator for the driver boundary.
*
* Drivers that wrap a third-party SDK (OpenAI, Anthropic, etc.) often
* let the SDK's own error class bubble — those carry an HTTP `.status`
* but are plain `Error` subclasses, not `HttpError`s, so they would
* otherwise hit the global error handler as unexpected 500s and page
* PagerDuty. Repackage them with `upstream_*` legacy codes so the
* alarm gate (server.ts) treats them as upstream failures and only
* pages on the two we actually care about (rate-limit / auth).
*
* `HttpError`s thrown by drivers pass through untouched.
*/
const translateProviderError = (err: unknown): unknown => {
if (isHttpError(err)) return err;
if (!err || typeof err !== 'object') return err;
const e = err as {
status?: number;
statusCode?: number;
message?: string;
error?: { code?: string; type?: string; message?: string };
code?: string;
};
const status = e.status ?? e.statusCode;
if (typeof status !== 'number') return err;
const msg = e.error?.message ?? e.message ?? 'Upstream provider error';
const upstreamCode = e.error?.code ?? e.code;
const fields = { upstreamStatus: status, upstreamCode };
if (status === 429) {
return new HttpError(429, msg, {
legacyCode: 'upstream_rate_limited',
fields,
});
}
if (status === 401 || status === 403) {
return new HttpError(500, msg, {
legacyCode: 'upstream_auth_failed',
fields,
});
}
if (status >= 500) {
return new HttpError(400, 'AI provider unavailable', {
legacyCode: 'upstream_provider_unavailable',
fields,
});
}
if (status >= 400) {
return new HttpError(400, msg, {
legacyCode: 'upstream_bad_request',
fields,
});
}
return err;
};
@Controller('/drivers')
export class DriverController extends PuterController {
/** iface → Map<driverName, driverInstance> */
@@ -327,11 +384,13 @@ export class DriverController extends PuterController {
Context.set('driverName', requestedDriver);
// Drivers read actor/context via the Context API — no drilled args.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const result = await (fn as (...x: unknown[]) => any).call(
driver,
args,
);
let result;
try {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
result = await (fn as (...x: unknown[]) => any).call(driver, args);
} catch (e) {
throw translateProviderError(e);
}
if (isDriverStreamResult(result)) {
res.setHeader('Content-Type', result.content_type);
src/backend/core/http/middleware/errorHandler.ts
+69
View File
@@ -80,6 +80,11 @@ export const createErrorHandler = (
return;
}
const translated = translateKnownClientError(err);
if (translated) {
err = translated;
}
if (isHttpError(err)) {
opts.onError?.(err, req);
if (err.statusCode === 402 || err.statusCode === 413) {
@@ -102,6 +107,70 @@ export const createErrorHandler = (
};
};
/**
* Recognise framework-level errors that are caused by the client and
* re-shape them as `HttpError`s with `client_*` legacy codes so the
* alarm gate (server.ts) treats them as user-caused 4xx and does not
* page on them.
*
* Sources covered:
* - `URIError` from `decodeURIComponent` in the express router (raised
* by path-traversal scanners hitting `%c0%ae` etc.)
* - body-parser `entity.parse.failed` (malformed JSON in request body)
* - body-parser `request.aborted` / `ECONNABORTED` (client closed
* socket mid-upload)
* - Anything else that already opted into `expose: true` with a
* numeric `statusCode` is mapped to `client_bad_request` so it
* surfaces with the declared status instead of becoming a 500.
*/
const translateKnownClientError = (err: unknown): HttpError | null => {
if (err instanceof URIError) {
return new HttpError(400, 'Bad request URL', {
legacyCode: 'client_bad_url',
});
}
if (!err || typeof err !== 'object') return null;
const e = err as {
type?: string;
code?: string;
statusCode?: number;
status?: number;
expose?: boolean;
message?: string;
};
if (e.type === 'request.aborted' || e.code === 'ECONNABORTED') {
return new HttpError(400, 'Request aborted', {
legacyCode: 'client_aborted',
});
}
if (e.type === 'entity.parse.failed') {
return new HttpError(400, 'Malformed JSON in request body', {
legacyCode: 'client_bad_json',
});
}
// Generic body-parser / http-errors convention: anything tagged
// `expose: true` with a real 4xx statusCode is by definition meant
// to be returned to the client, not paged on. Honour the declared
// status; tag with a known code so the alarm gate skips it.
const declaredStatus = e.statusCode ?? e.status;
if (
e.expose === true &&
typeof declaredStatus === 'number' &&
declaredStatus >= 400 &&
declaredStatus < 500
) {
return new HttpError(declaredStatus, e.message ?? 'Bad request', {
legacyCode: 'client_bad_request',
});
}
return null;
};
const serializeHttpError = (err: HttpError): Record<string, unknown> => {
const payload: Record<string, unknown> = {
error: err.message,
src/backend/drivers/ai-chat/ChatCompletionDriver.ts
+137 -18
View File
@@ -60,6 +60,136 @@ import { EventMap } from '../../clients/event/types.js';
const MAX_FALLBACKS = 4; // includes first attempt
type ProviderAttempt = {
model: string;
provider: string;
status?: number;
code?: string;
error: string;
};
/**
* Capture what an upstream provider gave us so the classifier downstream
* can decide a user-facing status code instead of always returning 500.
*
* OpenAI-SDK-based providers throw `APIError` with `.status` and a
* structured `.error` body — pull both. For arbitrary errors we fall
* back to the message and a status sniff so providers that throw plain
* `Error("... 503 ...")` strings still classify correctly.
*/
const toAttempt = (
modelId: string,
providerId: string,
err: unknown,
): ProviderAttempt => {
const e = err as {
status?: number;
statusCode?: number;
code?: string;
error?: { code?: string; type?: string; message?: string };
message?: string;
};
const message = e?.message ?? (typeof err === 'string' ? err : String(err));
let status = e?.status ?? e?.statusCode;
if (status === undefined) {
const m = message.match(/\b(4\d\d|5\d\d)\b/);
if (m) status = Number(m[1]);
}
return {
model: modelId,
provider: providerId,
status,
code: e?.error?.code ?? e?.code,
error: message,
};
};
const isRateLimit = (a: ProviderAttempt) =>
a.status === 429 ||
/rate[\s_-]?limit|too many requests|quota/i.test(a.error);
const isAuthFailure = (a: ProviderAttempt) =>
a.status === 401 ||
a.status === 403 ||
/unauthorized|forbidden|invalid api key/i.test(a.error);
const isUpstream5xx = (a: ProviderAttempt) =>
(a.status !== undefined && a.status >= 500) ||
/provider returned error|internal server error|service unavailable|bad gateway/i.test(
a.error,
);
/**
* Map an exhausted fallback chain to a single user-facing HttpError.
*
* Per-class rules (see also alarm gate in server.ts):
* - all rate-limited → 429 `upstream_rate_limited` (paged: forced alert)
* - all auth failures → 500 `upstream_auth_failed` (paged: our config)
* - all upstream 5xx → 400 `upstream_provider_unavailable` (no page)
* - all upstream 4xx (other) → 400 `upstream_bad_request` (no page)
* - mixed → 400 `upstream_failed` (no page)
*/
const classifyAttempts = (attempts: ProviderAttempt[]): HttpError => {
const fields = { attempts };
if (attempts.length === 0) {
return new HttpError(500, 'No providers attempted', {
legacyCode: 'internal_error',
fields,
});
}
if (attempts.every(isRateLimit)) {
return new HttpError(429, 'AI provider rate limit exceeded', {
legacyCode: 'upstream_rate_limited',
fields,
});
}
if (attempts.every(isAuthFailure)) {
return new HttpError(500, 'AI provider authentication failed', {
legacyCode: 'upstream_auth_failed',
fields,
});
}
if (attempts.every(isUpstream5xx)) {
return new HttpError(400, 'AI provider unavailable', {
legacyCode: 'upstream_provider_unavailable',
fields,
});
}
if (
attempts.every(
(a) => a.status !== undefined && a.status >= 400 && a.status < 500,
)
) {
return new HttpError(400, attempts[0].error, {
legacyCode: 'upstream_bad_request',
fields,
});
}
// Mixed failures where at least one attempt is clearly upstream
// (had an HTTP status from the SDK) means "AI providers couldn't
// satisfy the request" — expose, don't page.
const isUpstreamSignal = (a: ProviderAttempt) =>
a.status !== undefined ||
isRateLimit(a) ||
isAuthFailure(a) ||
isUpstream5xx(a);
if (attempts.some(isUpstreamSignal)) {
return new HttpError(400, 'All AI providers failed', {
legacyCode: 'upstream_failed',
fields,
});
}
// Nothing identifiable as an upstream issue — treat as our bug
// and let the global alarm fire so we actually find out.
return new HttpError(500, 'All providers failed', {
legacyCode: 'internal_error',
fields,
});
};
/**
* Driver implementing the `puter-chat-completion` interface.
*
@@ -290,8 +420,7 @@ export class ChatCompletionDriver extends PuterDriver {
);
}
const attempts: { model: string; provider: string; error: string }[] =
[];
const attempts: ProviderAttempt[] = [];
let res: IChatCompleteResult | undefined;
try {
@@ -301,17 +430,12 @@ export class ChatCompletionDriver extends PuterDriver {
provider: model.provider,
});
} catch (e) {
const error = e as Error;
attempts.push({
model: model.id,
provider: model.provider!,
error: error?.message ?? String(e),
});
attempts.push(toAttempt(model.id, model.provider!, e));
// Fallback loop
const tried = [model.id];
const triedProviders = [model.provider!];
let lastError: Error | null = error;
let lastError: Error | null = e as Error;
while (lastError && tried.length < MAX_FALLBACKS) {
const fallback = this.#findFallback(
@@ -348,20 +472,15 @@ export class ChatCompletionDriver extends PuterDriver {
lastError = null;
} catch (fbErr) {
lastError = fbErr as Error;
attempts.push({
model: fallback.id,
provider: fallback.provider!,
error: lastError?.message ?? String(fbErr),
});
attempts.push(
toAttempt(fallback.id, fallback.provider!, fbErr),
);
}
}
}
if (!res) {
throw new HttpError(500, 'All providers failed', {
legacyCode: 'internal_error',
fields: { attempts },
});
throw classifyAttempts(attempts);
}
const username = actor.user?.username;
src/backend/drivers/ai-tts/providers/elevenlabs/ElevenLabsTTSProvider.test.ts
+5 -2
View File
@@ -420,7 +420,7 @@ describe('ElevenLabsTTSProvider.synthesize metering', () => {
// ── Error paths ─────────────────────────────────────────────────────
describe('ElevenLabsTTSProvider.synthesize error paths', () => {
it('wraps non-OK upstream responses as HttpError 502', async () => {
it('wraps non-OK upstream 4xx responses as HttpError 400 with upstream_bad_request', async () => {
const provider = makeProvider();
fetchSpy.mockResolvedValueOnce(
new Response('{"error":"bad voice"}', {
@@ -431,7 +431,10 @@ describe('ElevenLabsTTSProvider.synthesize error paths', () => {
await expect(
withTestActor(() => provider.synthesize({ text: 'hi' })),
).rejects.toMatchObject({ statusCode: 502 });
).rejects.toMatchObject({
statusCode: 400,
legacyCode: 'upstream_bad_request',
});
expect(incrementUsageSpy).not.toHaveBeenCalled();
});
src/backend/drivers/ai-tts/providers/elevenlabs/ElevenLabsTTSProvider.ts
+36 -4
View File
@@ -100,12 +100,44 @@ export class ElevenLabsTTSProvider extends TTSProvider {
status: response.status,
detail,
});
// Map upstream status to an `upstream_*` HttpError so the alarm
// gate skips it. Anything 4xx from ElevenLabs (voice_not_found,
// invalid model, bad payload, auth) is a user-caused error from
// our perspective — expose as 400. 5xx is an outage on their
// side — also expose as 400 (`upstream_provider_unavailable`)
// since the user can't act on it but it's not our bug.
const upstreamCode =
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(detail as any)?.detail?.code ?? (detail as any)?.code;
const upstreamMessage =
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(detail as any)?.detail?.message ?? (detail as any)?.message;
const legacyCode =
response.status >= 500
? 'upstream_provider_unavailable'
: response.status === 401 || response.status === 403
? 'upstream_auth_failed'
: response.status === 429
? 'upstream_rate_limited'
: 'upstream_bad_request';
const exposedStatus =
legacyCode === 'upstream_rate_limited'
? 429
: legacyCode === 'upstream_auth_failed'
? 500
: 400;
throw new HttpError(
502,
`ElevenLabs request failed (status ${response.status})`,
exposedStatus,
upstreamMessage ??
`ElevenLabs request failed (status ${response.status})`,
{
legacyCode: 'internal_error',
fields: { provider: 'elevenlabs', status: response.status },
legacyCode,
fields: {
provider: 'elevenlabs',
upstreamStatus: response.status,
upstreamCode,
},
},
);
}
src/backend/server.ts
+22 -1
View File
@@ -684,9 +684,30 @@ export class PuterServer {
// route + error signature so a hot loop of the same
// crash lands as a single alarm with N occurrences
// instead of N pages.
//
// FORCED_ALERT_CODES override the 5xx-only rule: a
// status < 500 still pages if its legacyCode is in
// the set. Use this for things we want to know about
// even though we expose them as 4xx to users (e.g.
// sustained upstream provider rate limits).
//
// SKIP_ALERT_PREFIXES override the 5xx rule the other
// direction: an error tagged as caused by an upstream
// provider or a misbehaving client gets exposed to
// the user but does not page.
const FORCED_ALERT_CODES = new Set([
'upstream_rate_limited',
'upstream_auth_failed',
]);
const SKIP_ALERT_PREFIXES = /^(upstream_|client_)/;
const isHttp = isHttpError(err);
const status = isHttp ? err.statusCode : 500;
if (status < 500) return;
const legacyCode = isHttp ? (err.legacyCode ?? '') : '';
const forced = FORCED_ALERT_CODES.has(legacyCode);
if (!forced) {
if (status < 500) return;
if (SKIP_ALERT_PREFIXES.test(legacyCode)) return;
}
const signature = isHttp
? err.legacyCode || err.code || err.message
: err instanceof Error
src/backend/services/health/ServerHealthService.ts
+5 -5
View File
@@ -253,11 +253,11 @@ export class ServerHealthService extends PuterService {
(f) => f.name === check.name,
);
if (!alreadyFailing) {
this.clients.alarm?.create(
'health-check-failure',
`Health check ${check.name} failed`,
{ error: err as Error },
);
// Intentionally do not page PagerDuty for health-check
// failures — external uptime monitors cover this and the
// internal threshold flaps under normal load. Failures
// are still logged below and still trigger self-heal
// onFail handlers.
for (const handler of check.onFailHandlers) {
try {
await handler(err);