mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-07 02:59:05 +00:00
fix(dashboard): route dev API calls on Accept and fail fast on non-JSON 2xx (#44528)
* fix(dashboard): route dev API calls on Accept and fail fast on non-JSON 2xx The next dev rewrite that sends API calls to the proxy keyed on Content-Type: application/json, which openapi-fetch rightly omits on a bodyless GET, so GET /lens fell through to the Lens page and returned HTML. Route on Accept: application/json instead, send it from both HTTP clients, turn a non-JSON 2xx into a non-retryable ApiError in the typed client, and stop react-query from retrying ApiError below 500. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(dashboard): accept a JSON body served without a JSON content type Test fakes and some servers hand back JSON as text/plain, so the typed client only rejects a 2xx whose body does not parse as JSON. The system_one request test expects the Accept header the legacy client now sends. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Yujong Lee <yujong@berri.ai> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
1d52985d03
commit
5a5fd92d39
8 changed files with 101 additions and 9 deletions
|
|
@ -13,9 +13,11 @@ const nextConfig = {
|
|||
async rewrites() {
|
||||
return {
|
||||
beforeFiles: [
|
||||
// Every dashboard HTTP client sends Accept: application/json; page loads and RSC
|
||||
// fetches do not. That is what keeps GET /lens (API) apart from /lens (page) in dev.
|
||||
{
|
||||
source: "/:path*",
|
||||
has: [{ type: "header", key: "content-type", value: "application/json.*" }],
|
||||
has: [{ type: "header", key: "accept", value: "application/json.*" }],
|
||||
destination: `${devProxyUrl}/:path*`,
|
||||
},
|
||||
{ source: "/ui/:path*", destination: "/:path*" },
|
||||
|
|
|
|||
|
|
@ -62,6 +62,7 @@ describe("makeSystemOneRequest", () => {
|
|||
const expectedRequest: Partial<RequestInit> = {
|
||||
method: "POST",
|
||||
headers: {
|
||||
Accept: "application/json",
|
||||
"Content-Type": "application/json",
|
||||
Authorization: "Bearer session-key",
|
||||
},
|
||||
|
|
|
|||
|
|
@ -16,6 +16,10 @@ describe("root query retry policy", () => {
|
|||
expect(await attempts(new ApiError("forbidden", 403, null))).toBe(1);
|
||||
});
|
||||
|
||||
it("does not retry a malformed success response, which a retry cannot fix", async () => {
|
||||
expect(await attempts(new ApiError("Expected JSON from /lens but the server returned text/html", 200, ""))).toBe(1);
|
||||
});
|
||||
|
||||
it("retries server and network errors", async () => {
|
||||
expect(await attempts(new ApiError("down", 503, null))).toBe(4);
|
||||
expect(await attempts(new TypeError("Failed to fetch"))).toBe(4);
|
||||
|
|
|
|||
|
|
@ -5,8 +5,10 @@ import { ApiError } from "@/lib/http/client";
|
|||
|
||||
const MAX_RETRIES = 3;
|
||||
|
||||
const isRetryable = (error: unknown): boolean => !(error instanceof ApiError) || error.status >= 500;
|
||||
|
||||
export const shouldRetry = (failureCount: number, error: unknown): boolean =>
|
||||
!(error instanceof ApiError && error.status >= 400 && error.status < 500) && failureCount < MAX_RETRIES;
|
||||
isRetryable(error) && failureCount < MAX_RETRIES;
|
||||
|
||||
const queryClient = new QueryClient({ defaultOptions: { queries: { retry: shouldRetry } } });
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
// @vitest-environment node
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { fetchClient } from "./api";
|
||||
import { ApiError } from "./client";
|
||||
import { shouldRetry } from "@/contexts/ReactQueryProvider";
|
||||
import {
|
||||
registerAuthHeaderNameGetter,
|
||||
registerAuthTokenGetter,
|
||||
|
|
@ -70,6 +72,61 @@ describe("typed api client middleware", () => {
|
|||
expect(requests[0].headers.get("x-litellm-key")).toBe("Bearer explicit-token");
|
||||
});
|
||||
|
||||
it("asks for JSON on a bodyless GET so the next dev rewrite routes it to the proxy, not the page", async () => {
|
||||
const { fetch, requests } = capturingFetch(jsonResponse(200, { data: [] }));
|
||||
|
||||
await fetchClient.GET("/lens", { fetch });
|
||||
|
||||
expect(requests[0].headers.get("Accept")).toBe("application/json");
|
||||
expect(requests[0].headers.get("Content-Type")).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps an Accept header the caller set", async () => {
|
||||
const { fetch, requests } = capturingFetch(jsonResponse(200, { data: [] }));
|
||||
|
||||
await fetchClient.GET("/lens", { fetch, headers: { Accept: "text/event-stream" } });
|
||||
|
||||
expect(requests[0].headers.get("Accept")).toBe("text/event-stream");
|
||||
});
|
||||
|
||||
it("reports a non-JSON success body as an ApiError naming the path that react-query will not retry", async () => {
|
||||
const onError = vi.fn();
|
||||
registerErrorHandler(onError);
|
||||
const html = new Response("<!DOCTYPE html><html></html>", {
|
||||
status: 200,
|
||||
headers: { "Content-Type": "text/html" },
|
||||
});
|
||||
const { fetch } = capturingFetch(html);
|
||||
|
||||
const error = await fetchClient.GET("/lens", { fetch }).catch((e: unknown) => e);
|
||||
|
||||
expect(error).toBeInstanceOf(ApiError);
|
||||
expect((error as ApiError).message).toBe("Expected JSON from /lens but the server returned text/html");
|
||||
expect((error as ApiError).status).toBe(200);
|
||||
expect(onError).toHaveBeenCalledWith("Expected JSON from /lens but the server returned text/html");
|
||||
expect(shouldRetry(0, error)).toBe(false);
|
||||
});
|
||||
|
||||
it("accepts a JSON success body whose content type carries a charset", async () => {
|
||||
const response = new Response(JSON.stringify({ data: [] }), {
|
||||
status: 200,
|
||||
headers: { "Content-Type": "application/json; charset=utf-8" },
|
||||
});
|
||||
const { fetch } = capturingFetch(response);
|
||||
|
||||
const { data } = await fetchClient.GET("/model_group/info", { fetch });
|
||||
|
||||
expect(data).toEqual({ data: [] });
|
||||
});
|
||||
|
||||
it("accepts a JSON body served without a JSON content type", async () => {
|
||||
const { fetch } = capturingFetch(new Response(JSON.stringify({ data: [] }), { status: 200 }));
|
||||
|
||||
const { data } = await fetchClient.GET("/model_group/info", { fetch });
|
||||
|
||||
expect(data).toEqual({ data: [] });
|
||||
});
|
||||
|
||||
it("omits the auth header when no token is set", async () => {
|
||||
const { fetch, requests } = capturingFetch(jsonResponse(200, { data: [] }));
|
||||
|
||||
|
|
|
|||
|
|
@ -13,15 +13,39 @@ const BaseAwareRequest = function (url: string, init?: RequestInit): Request {
|
|||
return new globalThis.Request(target, init);
|
||||
} as unknown as typeof Request;
|
||||
|
||||
const isJsonMediaType = (contentType: string): boolean => /[/+]json\b/i.test(contentType);
|
||||
|
||||
const carriesJson = async (response: Response): Promise<boolean> => {
|
||||
const contentType = response.headers.get("content-type");
|
||||
if (contentType !== null && isJsonMediaType(contentType)) return true;
|
||||
const text = await response.clone().text();
|
||||
if (!text) return true;
|
||||
try {
|
||||
JSON.parse(text);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
};
|
||||
|
||||
const middleware: Middleware = {
|
||||
onRequest({ request }) {
|
||||
if (!request.headers.has("Accept")) {
|
||||
request.headers.set("Accept", "application/json");
|
||||
}
|
||||
const token = getAuthToken();
|
||||
if (token && !request.headers.has(getAuthHeaderName())) {
|
||||
request.headers.set(getAuthHeaderName(), `Bearer ${token}`);
|
||||
}
|
||||
},
|
||||
async onResponse({ response }) {
|
||||
if (response.ok) return response;
|
||||
async onResponse({ request, response }) {
|
||||
if (response.ok) {
|
||||
if (await carriesJson(response)) return response;
|
||||
const contentType = response.headers.get("content-type") ?? "an unknown content type";
|
||||
const message = `Expected JSON from ${new URL(request.url).pathname} but the server returned ${contentType}`;
|
||||
reportError(message);
|
||||
throw new ApiError(message, response.status, await response.clone().text());
|
||||
}
|
||||
const raw = await response.clone().text();
|
||||
let body: unknown = raw;
|
||||
let message: string;
|
||||
|
|
@ -43,9 +67,10 @@ const middleware: Middleware = {
|
|||
*
|
||||
* The base URL is injected, not fixed at import: every request is built against
|
||||
* whatever registerBaseUrlGetter supplies at call time (a split-origin proxy or
|
||||
* worker URL), falling back to the current origin. The middleware injects the
|
||||
* auth header and maps non-2xx responses to ApiError so query functions can just
|
||||
* read `.data`.
|
||||
* worker URL), falling back to the current origin. The middleware sends
|
||||
* `Accept: application/json` (the next dev rewrite routes on it), injects the
|
||||
* auth header, and maps non-2xx responses and non-JSON success bodies to
|
||||
* ApiError so query functions can just read `.data`.
|
||||
*/
|
||||
export const fetchClient = createFetchClient<paths>({
|
||||
Request: BaseAwareRequest,
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ describe("createApiClient", () => {
|
|||
expect(url).toBe("https://proxy.example/models?team=t1&page=2");
|
||||
expect(init).toMatchObject({ method: "GET" });
|
||||
expect(init.headers).toEqual({
|
||||
Accept: "application/json",
|
||||
"Content-Type": "application/json",
|
||||
"x-litellm-key": "Bearer sk-123",
|
||||
});
|
||||
|
|
@ -120,7 +121,7 @@ describe("createApiClient", () => {
|
|||
await client.get("/public/info");
|
||||
|
||||
const [, init] = fetchImpl.mock.calls[0];
|
||||
expect(init.headers).toEqual({ "Content-Type": "application/json" });
|
||||
expect(init.headers).toEqual({ Accept: "application/json", "Content-Type": "application/json" });
|
||||
});
|
||||
|
||||
it("getBlob returns the response body as a Blob on success", async () => {
|
||||
|
|
|
|||
|
|
@ -156,7 +156,7 @@ export function createApiClient(config: ApiClientConfig): ApiClient {
|
|||
|
||||
const url = appendQuery(`${getBaseUrl()}${path}`, query);
|
||||
|
||||
const headers: Record<string, string> = {};
|
||||
const headers: Record<string, string> = { Accept: "application/json" };
|
||||
if (rawBody === undefined) {
|
||||
headers["Content-Type"] = "application/json";
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue