mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
refactor(ui): drop the accessToken ref, snapshot once at submit instead
Reading accessTokenRef.current independently in verifyPresetStillAvailable and again at the create call still left a residual gap: two separate reads separated by an await can observe two different tokens if one rotates in between, so verification and creation could still end up representing different callers. Chasing "freshest at every read" can never fully close that, since there's always a window between any two reads. The actual invariant needed is internal consistency for one submission, not maximum freshness at each step: capture accessToken once, via the plain closure that already exists, and thread that same value through both the verification fetch and the create call. Deletes the ref and its syncing effect entirely; a plain prop closure already guarantees two reads within the same function invocation return the same value, no ref needed.
This commit is contained in:
parent
8a30af69d0
commit
3ed3ee31db
2 changed files with 19 additions and 24 deletions
|
|
@ -289,17 +289,18 @@ describe("AddAutoRouterTab", () => {
|
|||
expect(mockHandleAddAutoRouterSubmit).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// submitRecommendedRouter awaits a network round trip (the submit-time re-check) before it ever
|
||||
// reaches the create call. If accessToken rotates while that await is pending, a closure-captured
|
||||
// value would carry the token that was live at click time into a create call that fires after the
|
||||
// replacement token is active. Read it through a ref updated every render instead.
|
||||
it("uses the current access token for creation even if it rotates while the submit-time re-check is in flight", async () => {
|
||||
// A single in-flight submission must stay internally consistent even if accessToken rotates
|
||||
// mid-flight: verifyPresetStillAvailable and the create call both receive the token captured
|
||||
// once when submit started, so they can never end up verifying one caller's models and creating
|
||||
// under another's identity. Reading a "latest" ref independently at each point can't fully close
|
||||
// that gap (there's always a residual window between any two reads); one snapshot, used
|
||||
// throughout, closes it completely.
|
||||
it("keeps verification and creation on the same token even if it rotates mid-submission", async () => {
|
||||
const user = userEvent.setup();
|
||||
let resolveInitialRecheck: (models: ModelGroup[]) => void = () => undefined;
|
||||
mockFetchAvailableModels
|
||||
.mockResolvedValueOnce(ALL_FAMILY_MODELS)
|
||||
.mockReturnValueOnce(new Promise<ModelGroup[]>((resolve) => (resolveInitialRecheck = resolve)))
|
||||
.mockResolvedValueOnce(ALL_FAMILY_MODELS);
|
||||
.mockReturnValueOnce(new Promise<ModelGroup[]>((resolve) => (resolveInitialRecheck = resolve)));
|
||||
|
||||
const { rerender } = renderWithProviders(
|
||||
<AddAutoRouterTab handleOk={vi.fn()} accessToken="stale-token" userRole="Admin" />,
|
||||
|
|
@ -311,6 +312,7 @@ describe("AddAutoRouterTab", () => {
|
|||
await user.type(screen.getByPlaceholderText(/smart_router/i), "rotated-token-router");
|
||||
await user.click(screen.getByRole("button", { name: /add auto router/i }));
|
||||
await waitFor(() => expect(mockFetchAvailableModels).toHaveBeenCalledTimes(2));
|
||||
expect(mockFetchAvailableModels.mock.calls[1][0]).toBe("stale-token");
|
||||
|
||||
// The token rotates while the re-check above is still in flight.
|
||||
rerender(<AddAutoRouterTab handleOk={vi.fn()} accessToken="fresh-token" userRole="Admin" />);
|
||||
|
|
@ -318,7 +320,7 @@ describe("AddAutoRouterTab", () => {
|
|||
resolveInitialRecheck(ALL_FAMILY_MODELS);
|
||||
|
||||
await waitFor(() => expect(mockHandleAddAutoRouterSubmit).toHaveBeenCalled());
|
||||
expect(mockHandleAddAutoRouterSubmit.mock.calls.at(-1)?.[1]).toBe("fresh-token");
|
||||
expect(mockHandleAddAutoRouterSubmit.mock.calls.at(-1)?.[1]).toBe("stale-token");
|
||||
});
|
||||
|
||||
// The headline behavior: selecting a preset must pre-fill the tier config so the created
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import React, { useEffect, useRef, useState } from "react";
|
||||
import React, { useEffect, useState } from "react";
|
||||
import { useQuery } from "@tanstack/react-query";
|
||||
import { Card, Form, Button, Tooltip, Typography, Select as AntdSelect, Modal } from "antd";
|
||||
import { TextInput } from "@tremor/react";
|
||||
|
|
@ -74,13 +74,6 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
createScope = "unscoped-ok",
|
||||
}) => {
|
||||
const requiresTeamScope = createScope === "team-required";
|
||||
// submitRecommendedRouter awaits a network round trip before creating the router; reading
|
||||
// accessToken through this ref instead of the closure keeps that call on whatever token is
|
||||
// current when it actually fires, not whichever one was live when submit was clicked.
|
||||
const accessTokenRef = useRef(accessToken);
|
||||
useEffect(() => {
|
||||
accessTokenRef.current = accessToken;
|
||||
}, [accessToken]);
|
||||
const [form] = Form.useForm();
|
||||
const [modelAccessGroups, setModelAccessGroups] = useState<string[]>([]);
|
||||
|
||||
|
|
@ -204,16 +197,16 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
// background refetch failure keeps trusting the stale cache by design - see modelsUnverifiable
|
||||
// above). The backend does not re-check a router's referenced model names against the caller's
|
||||
// access either, so this is the only place that can catch it: force a fresh fetch right before
|
||||
// creating the router, rather than trusting whatever's cached. Fetch directly against
|
||||
// accessTokenRef instead of the query's own refetch, which stays bound to whichever token was
|
||||
// current when this render's useQuery was set up - using it here could verify one caller's
|
||||
// models and create the router under another if the token rotates mid-check.
|
||||
const verifyPresetStillAvailable = async (presetKey: string): Promise<boolean> => {
|
||||
// creating the router, rather than trusting whatever's cached. Takes the token as a parameter,
|
||||
// captured once by the caller, so this check and the create call it gates can never end up
|
||||
// disagreeing about which caller they represent - reading accessToken independently at each
|
||||
// point only chases a moving target and can never fully close that gap.
|
||||
const verifyPresetStillAvailable = async (presetKey: string, token: string): Promise<boolean> => {
|
||||
const preset = getPresetByKey(presetKey);
|
||||
if (!preset) return false;
|
||||
let freshModels: ModelGroup[];
|
||||
try {
|
||||
freshModels = await fetchAvailableModels(accessTokenRef.current);
|
||||
freshModels = await fetchAvailableModels(token);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
|
|
@ -228,7 +221,7 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
return;
|
||||
}
|
||||
|
||||
if (selectedPreset !== "custom" && !(await verifyPresetStillAvailable(selectedPreset))) {
|
||||
if (selectedPreset !== "custom" && !(await verifyPresetStillAvailable(selectedPreset, accessToken))) {
|
||||
setShowValidationErrors(true);
|
||||
NotificationManager.fromBackend(
|
||||
"This template's models are no longer available. Please reselect a template or switch to Custom.",
|
||||
|
|
@ -314,7 +307,7 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
model_access_group: form.getFieldValue("model_access_group"),
|
||||
};
|
||||
|
||||
await handleAddAutoRouterSubmit(submitValues, accessTokenRef.current, form, handleOk);
|
||||
await handleAddAutoRouterSubmit(submitValues, accessToken, form, handleOk);
|
||||
} catch (error) {
|
||||
console.error("Validation failed:", error);
|
||||
NotificationManager.fromBackend("Please fill in all required fields");
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue