diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index f837e75039e..fe4f9285751 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -21687,6 +21687,7 @@ async def get_config( # Check if slack alerting is on _alerting: Final = _general_settings.get("alerting", []) + _alerting_types: Final = proxy_logging_obj.slack_alerting_instance.alert_types alerting_data: Final = [] if "slack" in _alerting: _slack_values, _ = resolve_fields( @@ -21694,7 +21695,6 @@ async def get_config( ) _slack_env_vars: Final = _apply_alerting_env_role_gate(_slack_values, is_full_admin) - _alerting_types: Final = proxy_logging_obj.slack_alerting_instance.alert_types _all_alert_types: Final = proxy_logging_obj.slack_alerting_instance._all_possible_alert_types() _alerts_to_webhook: Final = _apply_webhook_role_gate( proxy_logging_obj.slack_alerting_instance.alert_to_webhook_url, is_full_admin @@ -21739,6 +21739,7 @@ async def get_config( "callbacks": _data_to_return, "alerts": alerting_data, "active_alerting_destinations": tuple(_alerting), + "alert_types": _alerting_types, "router_settings": _router_settings, "available_callbacks": all_available_callbacks, } diff --git a/tests/unit/proxy/proxy_server/test_routes_config.py b/tests/unit/proxy/proxy_server/test_routes_config.py index e3b8aff8fcb..8c0adab3ee8 100644 --- a/tests/unit/proxy/proxy_server/test_routes_config.py +++ b/tests/unit/proxy/proxy_server/test_routes_config.py @@ -1413,6 +1413,34 @@ def test_get_config_callbacks_redacts_slack_webhook_urls_for_view_only_admin(cli assert admin_slack["variables"]["SLACK_WEBHOOK_URL"] != "REDACTED" +def test_get_config_callbacks_returns_alert_types_when_slack_alerting_is_off(client, auth_as, mock_prisma, monkeypatch): + from litellm.proxy import proxy_server as ps + from litellm.proxy._types import AlertType, LitellmUserRoles + + _install_litellm_config(mock_prisma) + monkeypatch.setattr(ps, "prisma_client", mock_prisma) + monkeypatch.setattr(ps, "llm_router", None) + fake_proxy_config = MagicMock() + fake_proxy_config.get_config = AsyncMock( + return_value={ + "litellm_settings": {"success_callback": []}, + "general_settings": {"alerting": ["ms_teams"]}, + "environment_variables": {}, + } + ) + monkeypatch.setattr(ps, "proxy_config", fake_proxy_config) + teams_alert_types: Final = [AlertType.llm_exceptions, AlertType.spend_reports] + monkeypatch.setattr(ps.proxy_logging_obj.slack_alerting_instance, "alert_types", teams_alert_types) + + with auth_as(LitellmUserRoles.PROXY_ADMIN): + response = client.get("/get/config/callbacks") + assert response.status_code == 200 + body = response.json() + + assert body["alert_types"] == ["llm_exceptions", "spend_reports"] + assert all(alert["name"] != "slack" for alert in body["alerts"]) + + def test_redact_callback_env_vars_helper_handles_none_and_non_secret_keys(): from litellm.proxy import proxy_server as ps diff --git a/ui/litellm-dashboard/src/components/settings.test.tsx b/ui/litellm-dashboard/src/components/settings.test.tsx index 9b4145c47ad..833f7143570 100644 --- a/ui/litellm-dashboard/src/components/settings.test.tsx +++ b/ui/litellm-dashboard/src/components/settings.test.tsx @@ -2,6 +2,7 @@ import { act, fireEvent, render, screen, waitFor, within } from "@testing-librar import userEvent from "@testing-library/user-event"; import { FormProvider, useForm } from "react-hook-form"; import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { toast } from "@/lib/toast"; import { renderWithProviders } from "../../tests/test-utils"; import { alertingSettingsCall, getCallbackConfigsCall, getCallbacksCall, setCallbacksCall } from "./networking"; import Settings, { backendCallbackLogoSrc, CallbackSelector } from "./settings"; @@ -15,6 +16,10 @@ vi.mock("./networking", () => ({ alertingSettingsCall: vi.fn().mockResolvedValue([]), })); +vi.mock("@/lib/toast", () => ({ + toast: { success: vi.fn(), fromError: vi.fn() }, +})); + vi.mock("./alerting/alerting_settings", () => ({ __esModule: true, default: () =>
Mock Alerting Settings
, @@ -55,6 +60,33 @@ beforeAll(() => { }); }); +const teamsOnlyCallbacksResponse = { + callbacks: [], + available_callbacks: [], + alerts: [ + { name: "email", variables: { EMAIL_ADDRESS: null } }, + { name: "ms_teams", variables: { MS_TEAMS_WEBHOOK_URL: null } }, + ], + active_alerting_destinations: ["ms_teams"], + alert_types: ["llm_exceptions", "spend_reports"], +}; + +const slackCallbacksResponseWithoutTopLevelAlertTypes = { + callbacks: [], + available_callbacks: [], + alerts: [ + { + name: "slack", + active_alerts: ["llm_exceptions"], + variables: { SLACK_WEBHOOK_URL: null }, + alerts_to_webhook: { model_deprecation_warnings: "https://hooks.example.com/dep" }, + }, + { name: "email", variables: { EMAIL_ADDRESS: null } }, + { name: "ms_teams", variables: { MS_TEAMS_WEBHOOK_URL: null } }, + ], + active_alerting_destinations: ["slack"], +}; + describe("Settings", () => { const defaultProps = { accessToken: "token", @@ -520,6 +552,139 @@ describe("Settings", () => { }); }); + it("should start alert type switches from the proxy's alert types and keep them on save when Slack is not configured", async () => { + mockGetCallbacksCall.mockResolvedValue(teamsOnlyCallbacksResponse); + + const user = userEvent.setup(); + renderWithProviders(); + await user.click(await screen.findByRole("tab", { name: "Alerting Types" })); + + const llmExceptionsRow = screen.getByRole("row", { name: "LLM Exceptions" }); + const llmExceptionsSwitch = within(llmExceptionsRow).getByRole("switch"); + expect(llmExceptionsSwitch).toBeChecked(); + + const deprecationWarningsRow = screen.getByRole("row", { name: "Model Deprecation Warnings" }); + const deprecationWarningsSwitch = within(deprecationWarningsRow).getByRole("switch"); + expect(deprecationWarningsSwitch).not.toBeChecked(); + await user.click(deprecationWarningsSwitch); + await user.click(llmExceptionsSwitch); + + expect(deprecationWarningsSwitch).toBeChecked(); + expect(llmExceptionsSwitch).not.toBeChecked(); + await user.click(screen.getByRole("button", { name: "Save Changes" })); + + await waitFor(() => { + expect(vi.mocked(setCallbacksCall)).toHaveBeenCalledWith( + "token", + expect.objectContaining({ + general_settings: expect.objectContaining({ + alert_types: ["spend_reports", "model_deprecation_warnings"], + }), + }), + ); + }); + + await user.click(deprecationWarningsSwitch); + await user.click(screen.getByRole("button", { name: "Save Changes" })); + + await waitFor(() => { + expect(vi.mocked(setCallbacksCall)).toHaveBeenLastCalledWith( + "token", + expect.objectContaining({ + general_settings: expect.objectContaining({ + alert_types: ["spend_reports"], + }), + }), + ); + }); + }); + + it("should leave the proxy's alert types unset when only webhooks change on save", async () => { + mockGetCallbacksCall.mockResolvedValue(teamsOnlyCallbacksResponse); + + const user = userEvent.setup(); + renderWithProviders(); + await user.click(await screen.findByRole("tab", { name: "Alerting Types" })); + + const deprecationWarningsRow = screen.getByRole("row", { name: "Model Deprecation Warnings" }); + const deprecationWarningsSwitch = within(deprecationWarningsRow).getByRole("switch"); + await user.click(deprecationWarningsSwitch); + await user.click(deprecationWarningsSwitch); + await user.click(screen.getByRole("button", { name: "Save Changes" })); + + await waitFor(() => { + expect(vi.mocked(setCallbacksCall)).toHaveBeenCalledTimes(1); + }); + const savedSettings = vi.mocked(setCallbacksCall).mock.calls[0][1].general_settings; + expect(savedSettings).toHaveProperty("alert_to_webhook_url"); + expect(savedSettings).not.toHaveProperty("alert_types"); + }); + + it("should save the reverted alert types when a switch is reverted while a save is in flight", async () => { + mockGetCallbacksCall.mockResolvedValue(teamsOnlyCallbacksResponse); + const finishSaves: Array<() => void> = []; + const pendingSave = () => + new Promise((resolve) => { + finishSaves.push(resolve); + }); + vi.mocked(setCallbacksCall).mockImplementationOnce(pendingSave).mockImplementationOnce(pendingSave); + + const user = userEvent.setup(); + renderWithProviders(); + await user.click(await screen.findByRole("tab", { name: "Alerting Types" })); + + const deprecationWarningsRow = screen.getByRole("row", { name: "Model Deprecation Warnings" }); + const deprecationWarningsSwitch = within(deprecationWarningsRow).getByRole("switch"); + const saveButton = screen.getByRole("button", { name: "Save Changes" }); + await user.click(deprecationWarningsSwitch); + await user.click(saveButton); + await user.click(deprecationWarningsSwitch); + await user.click(saveButton); + await act(async () => { + finishSaves.forEach((finish) => finish()); + }); + await user.click(saveButton); + + await waitFor(() => { + expect(vi.mocked(setCallbacksCall)).toHaveBeenLastCalledWith( + "token", + expect.objectContaining({ + general_settings: expect.objectContaining({ + alert_types: ["llm_exceptions", "spend_reports"], + }), + }), + ); + }); + }); + + it("should load alert types and webhook values from the Slack callback row", async () => { + mockGetCallbacksCall.mockResolvedValue(slackCallbacksResponseWithoutTopLevelAlertTypes); + + const user = userEvent.setup(); + renderWithProviders(); + await user.click(await screen.findByRole("tab", { name: "Alerting Types" })); + + const llmExceptionsRow = screen.getByRole("row", { name: "LLM Exceptions" }); + const deprecationWarningsRow = screen.getByRole("row", { name: "Model Deprecation Warnings" }); + expect(within(llmExceptionsRow).getByRole("switch")).toBeChecked(); + expect(within(deprecationWarningsRow).getByRole("switch")).not.toBeChecked(); + expect(screen.getByDisplayValue("https://hooks.example.com/dep")).toBeInTheDocument(); + }); + + it("should show an error toast and no success toast when saving alert types fails", async () => { + vi.mocked(setCallbacksCall).mockRejectedValueOnce(new Error("save failed")); + + const user = userEvent.setup(); + renderWithProviders(); + await user.click(await screen.findByRole("tab", { name: "Alerting Types" })); + await user.click(screen.getByRole("button", { name: "Save Changes" })); + + await waitFor(() => { + expect(vi.mocked(toast.fromError)).toHaveBeenCalledWith(expect.any(Error)); + }); + expect(vi.mocked(toast.success)).not.toHaveBeenCalled(); + }); + it("should hold the callbacks table in loading state until the fetch settles", async () => { let resolveCallbacks: (value: { callbacks: never[]; diff --git a/ui/litellm-dashboard/src/components/settings.tsx b/ui/litellm-dashboard/src/components/settings.tsx index d2d65b9f526..10531313496 100644 --- a/ui/litellm-dashboard/src/components/settings.tsx +++ b/ui/litellm-dashboard/src/components/settings.tsx @@ -334,6 +334,8 @@ const Settings: React.FC = ({ accessToken, userRole, userID, const [catchAllWebhookURL, setCatchAllWebhookURL] = useState(""); const [alertToWebhooks, setAlertToWebhooks] = useState>({}); const [activeAlerts, setActiveAlerts] = useState([]); + const [savedAlerts, setSavedAlerts] = useState([]); + const [isSavingAlerts, setIsSavingAlerts] = useState(false); const [showAddCallbacksModal, setShowAddCallbacksModal] = useState(false); const [callbackConfigs, setCallbackConfigs] = useState([]); @@ -432,16 +434,20 @@ const Settings: React.FC = ({ accessToken, userRole, userID, setAllCallbacks(data.available_callbacks); let alerts_data = data.alerts; - if (alerts_data) { - if (alerts_data.length > 0) { - let _alert_info = alerts_data[0]; - let catch_all_webhook = _alert_info.variables.SLACK_WEBHOOK_URL; - - let active_alerts = _alert_info.active_alerts; - setActiveAlerts(active_alerts); - setCatchAllWebhookURL(catch_all_webhook); - setAlertToWebhooks(_alert_info.alerts_to_webhook); - } + const slack_alert_info = alerts_data?.find( + (alert: { + name: string; + active_alerts?: string[] | null; + variables?: { SLACK_WEBHOOK_URL?: string | null } | null; + alerts_to_webhook?: Record | null; + }) => alert.name === "slack", + ); + const loadedAlerts: string[] = data.alert_types ?? slack_alert_info?.active_alerts ?? []; + setActiveAlerts(loadedAlerts); + setSavedAlerts(loadedAlerts); + if (slack_alert_info) { + setCatchAllWebhookURL(slack_alert_info.variables?.SLACK_WEBHOOK_URL ?? ""); + setAlertToWebhooks(slack_alert_info.alerts_to_webhook ?? {}); } setAlerts(alerts_data); @@ -555,18 +561,25 @@ const Settings: React.FC = ({ accessToken, userRole, userID, updatedAlertToWebhooks[key] = newWebhookValue; }); + const alertTypesChanged = + activeAlerts.length !== savedAlerts.length || activeAlerts.some((alert) => !savedAlerts.includes(alert)); const payload = { general_settings: { alert_to_webhook_url: updatedAlertToWebhooks, - alert_types: activeAlerts, + ...(alertTypesChanged ? { alert_types: activeAlerts } : {}), }, }; + setIsSavingAlerts(true); try { await setCallbacksCall(accessToken, payload); } catch (error) { toast.fromError(error); + return; + } finally { + setIsSavingAlerts(false); } + setSavedAlerts(activeAlerts); toast.success(t("loggingAndAlerts.alertTypes.toasts.saved")); }; @@ -710,7 +723,7 @@ const Settings: React.FC = ({ accessToken, userRole, userID, ))} -