scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-15 23:31:29 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 213

fix(cli): clean up leaked proxy and surface clean errors in autoroute

Browse source 
Three related gaps, all following an UpError getting raised somewhere
that wasn't catching it yet:

- up() left the just-launched ephemeral proxy running with no pid
  record if load_json_or_empty/write_backup/secure_create raised after
  the health check passed, mirroring the existing ProcessLaunchError
  cleanup for the health-check-failure branch.
- _teardown() didn't catch restore_claude_settings raising UpError
  (e.g. a corrupt backup at stop time), which would otherwise escape
  to Click as an unhandled error in the normal-exit path, or print
  "Error in atexit" in the atexit path. up.py's own _restore_once
  handles the identical case the same way.
- read_pid_record let a corrupt PID file surface a raw
  pydantic.ValidationError instead of a clean message, and did so in
  down(), the command specifically meant for crash recovery. down()
  now clears an unreadable pid record and continues cleanup instead of
  aborting, since a corrupt pid file must never block the one command
  meant to recover from exactly this kind of crash.
This commit is contained in:
Krrish Dholakia 2026-07-15 19:05:18 -07:00
parent 1c66608a7a
commit 9b0e625b29
4 changed files with 139 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

54
litellm/proxy/client/cli/commands/autoroute/commands.py
View file

@ -9,7 +9,7 @@ import click
import yaml
from pydantic import JsonValue, TypeAdapter, ValidationError
from ..up import CLAUDE_SETTINGS_PATH, load_json_or_empty, restore_claude_settings, write_backup
from ..up import CLAUDE_SETTINGS_PATH, UpError, load_json_or_empty, restore_claude_settings, write_backup
from ..up import BackupRecord as ClaudeBackupRecord
from .process import (
AUTOROUTE_DIR,
@ -81,7 +81,10 @@ def up() -> None:
if not CONFIG_PATH.exists():
raise click.ClickException("No config found. Run `lite autoroute configure` first.")
existing_pid = read_pid_record()
try:
existing_pid = read_pid_record()
except UpError as e:
raise click.ClickException(str(e))
if existing_pid is not None and is_running(existing_pid.pid):
raise click.ClickException(
"An ephemeral proxy is already running (lite autoroute up looks already active). "
@ -107,16 +110,21 @@ def up() -> None:
clear_pid_record()
raise click.ClickException(str(e))
original_existed = CLAUDE_SETTINGS_PATH.exists()
original_settings = load_json_or_empty(CLAUDE_SETTINGS_PATH)
write_backup(
ClaudeBackupRecord(existed=original_existed, content=original_settings if original_existed else None),
AUTOROUTE_BACKUP_PATH,
)
merged = merge_claude_settings_static_token(original_settings, base_url, master_key)
CLAUDE_SETTINGS_PATH.parent.mkdir(parents=True, exist_ok=True)
with secure_create(CLAUDE_SETTINGS_PATH) as f:
json.dump(merged, f, indent=2)
try:
original_existed = CLAUDE_SETTINGS_PATH.exists()
original_settings = load_json_or_empty(CLAUDE_SETTINGS_PATH)
write_backup(
ClaudeBackupRecord(existed=original_existed, content=original_settings if original_existed else None),
AUTOROUTE_BACKUP_PATH,
)
merged = merge_claude_settings_static_token(original_settings, base_url, master_key)
CLAUDE_SETTINGS_PATH.parent.mkdir(parents=True, exist_ok=True)
with secure_create(CLAUDE_SETTINGS_PATH) as f:
json.dump(merged, f, indent=2)
except UpError as e:
terminate(process.pid)
clear_pid_record()
raise click.ClickException(str(e))
click.echo(f"litellm: ephemeral auto-router proxy up at {base_url} (pid {process.pid})")
click.echo("Claude Code sessions started now will route through it. Press Ctrl-C to stop and restore.")
@ -129,7 +137,14 @@ def up() -> None:
return
terminate(process.pid)
clear_pid_record()
restore_claude_settings(CLAUDE_SETTINGS_PATH, AUTOROUTE_BACKUP_PATH)
try:
restore_claude_settings(CLAUDE_SETTINGS_PATH, AUTOROUTE_BACKUP_PATH)
except UpError as e:
# Runs from atexit/a signal handler too, outside Click's own exception
# handling -- raising here would only produce an unhandled-exception
# warning on stderr, not a clean message.
click.echo(str(e), err=True)
return
click.echo("\nStopped ephemeral proxy and restored Claude Code settings.")
click.echo(
f"Restart any Claude Code session still open from this session, or another local account could "
@ -154,13 +169,22 @@ def up() -> None:
@autoroute_group.command("down")
def down() -> None:
"""Restore Claude Code settings and stop a leftover ephemeral proxy, if any"""
record: PidRecord | None = read_pid_record()
try:
record: PidRecord | None = read_pid_record()
except UpError as e:
# down is the crash-recovery path -- a corrupt pid record must not block it; clear the
# unusable record and keep going rather than leaving the user with no way to clean up.
click.echo(f"{e} Clearing it and continuing cleanup.", err=True)
record = None
if record is not None and is_running(record.pid):
terminate(record.pid)
click.echo(f"Stopped leftover ephemeral proxy (pid {record.pid}).")
clear_pid_record()
restored = restore_claude_settings(CLAUDE_SETTINGS_PATH, AUTOROUTE_BACKUP_PATH)
try:
restored = restore_claude_settings(CLAUDE_SETTINGS_PATH, AUTOROUTE_BACKUP_PATH)
except UpError as e:
raise click.ClickException(str(e))
if restored is None:
click.echo("Nothing to restore.")
elif restored.existed:

10
litellm/proxy/client/cli/commands/autoroute/process.py
View file

@ -12,9 +12,9 @@ from pathlib import Path
import click
import requests
from pydantic import TypeAdapter
from pydantic import TypeAdapter, ValidationError
from ..up import secure_create
from ..up import UpError, secure_create
AUTOROUTE_DIR = Path.home() / ".litellm" / "autorouter"
CONFIG_PATH = AUTOROUTE_DIR / "config.yaml"
@ -103,7 +103,11 @@ def read_pid_record(path: Path | None = None) -> PidRecord | None:
if not resolved_path.exists():
return None
with open(resolved_path, "r") as f:
return _PID_RECORD_ADAPTER.validate_json(f.read())
content = f.read()
try:
return _PID_RECORD_ADAPTER.validate_json(content)
except ValidationError:
raise UpError(f"{resolved_path} contains invalid or unexpected JSON; cannot proceed safely.")
def clear_pid_record(path: Path | None = None) -> None:

85
tests/test_litellm/proxy/client/cli/autoroute/test_commands.py
View file

@ -148,6 +148,34 @@ class TestUpCommand:
assert written_config["general_settings"]["master_key"] == "fixed-master-key"
assert stat.S_IMODE(config_path.stat().st_mode) == 0o600
def test_teardown_reports_clean_error_when_backup_is_corrupt(self, monkeypatch, tmp_path):
"""A corrupt backup at teardown time (e.g. a concurrent process wrote garbage to it) must
not crash the whole command -- _restore_once in up.py handles the identical case in
lite up the same way, echoing the error instead of propagating it."""
config_path, _log_path, claude_settings_path, backup_path, pid_record_path = _patch_paths(monkeypatch, tmp_path)
config_path.write_text(yaml.safe_dump({"model_list": []}))
claude_settings_path.write_text(json.dumps({"theme": "dark"}))
_silence_signal_handling(monkeypatch)
fake_process = FakeProcess(pid=11111)
monkeypatch.setattr(commands_module, "launch_proxy", lambda *a, **k: fake_process)
monkeypatch.setattr(commands_module, "poll_liveliness", lambda *a, **k: None)
monkeypatch.setattr(commands_module, "allocate_free_port", lambda: 65432)
monkeypatch.setattr(commands_module, "terminate", lambda pid, **k: None)
monkeypatch.setattr(commands_module.secrets, "token_urlsafe", lambda n: "fixed-master-key")
def fake_wait(self, timeout=None):
backup_path.write_text("not json at all {{{")
return True
monkeypatch.setattr("threading.Event.wait", fake_wait)
result = self.runner.invoke(up)
assert result.exit_code == 0, result.output
assert "invalid or unexpected JSON" in result.output
assert not pid_record_path.exists()
def test_surfaces_clean_error_and_cleans_up_when_health_check_fails(self, monkeypatch, tmp_path):
config_path, _log_path, claude_settings_path, backup_path, pid_record_path = _patch_paths(monkeypatch, tmp_path)
config_path.write_text(yaml.safe_dump({"model_list": []}))
@ -175,6 +203,30 @@ class TestUpCommand:
assert not backup_path.exists()
assert json.loads(claude_settings_path.read_text()) == original_settings
def test_terminates_ephemeral_proxy_when_claude_settings_is_corrupt(self, monkeypatch, tmp_path):
"""The health check can pass and the proxy can come up fine, but if
~/.claude/settings.json turns out to be corrupt, the just-started proxy must not be left
running with no pid record -- exactly the leak `lite autoroute down` exists to clean up."""
config_path, _log_path, claude_settings_path, backup_path, pid_record_path = _patch_paths(monkeypatch, tmp_path)
config_path.write_text(yaml.safe_dump({"model_list": []}))
claude_settings_path.write_text("not json at all {{{")
fake_process = FakeProcess(pid=777)
terminate_calls = []
monkeypatch.setattr(commands_module, "launch_proxy", lambda *a, **k: fake_process)
monkeypatch.setattr(commands_module, "poll_liveliness", lambda *a, **k: None)
monkeypatch.setattr(commands_module, "allocate_free_port", lambda: 23456)
monkeypatch.setattr(commands_module, "terminate", lambda pid, **k: terminate_calls.append(pid))
monkeypatch.setattr(commands_module.secrets, "token_urlsafe", lambda n: "fixed-master-key")
result = self.runner.invoke(up)
assert result.exit_code != 0
assert "invalid JSON" in result.output
assert terminate_calls == [777]
assert not pid_record_path.exists()
assert not backup_path.exists()
class TestDownCommand:
def setup_method(self):
@ -213,3 +265,36 @@ class TestDownCommand:
assert result.exit_code == 0, result.output
assert "Nothing to restore." in result.output
assert not claude_settings_path.exists()
def test_clears_a_corrupt_pid_record_and_still_restores_settings(self, monkeypatch, tmp_path):
"""down is specifically the crash-recovery path -- a pid file truncated by a mid-write
crash must not block it from clearing the record and restoring Claude settings anyway."""
_config_path, _log_path, claude_settings_path, backup_path, pid_record_path = _patch_paths(
monkeypatch, tmp_path
)
pid_record_path.parent.mkdir(parents=True, exist_ok=True)
pid_record_path.write_text("not json at all {{{")
original_settings = {"theme": "dark"}
write_backup(ClaudeBackupRecord(existed=True, content=original_settings), backup_path)
claude_settings_path.write_text(json.dumps({"env": {"ANTHROPIC_AUTH_TOKEN": "fixed-master-key"}}))
result = self.runner.invoke(down)
assert result.exit_code == 0, result.output
assert "invalid or unexpected JSON" in result.output
assert "Restored" in result.output
assert not pid_record_path.exists()
assert not backup_path.exists()
assert json.loads(claude_settings_path.read_text()) == original_settings
def test_surfaces_clean_error_when_backup_is_corrupt(self, monkeypatch, tmp_path):
_config_path, _log_path, _claude_settings_path, backup_path, _pid_record_path = _patch_paths(
monkeypatch, tmp_path
)
backup_path.parent.mkdir(parents=True, exist_ok=True)
backup_path.write_text("not json at all {{{")
result = self.runner.invoke(down)
assert result.exit_code != 0
assert "invalid or unexpected JSON" in result.output

8
tests/test_litellm/proxy/client/cli/autoroute/test_process.py
View file

@ -9,6 +9,7 @@ from litellm.proxy.client.cli.commands.autoroute import process as process_modul
from litellm.proxy.client.cli.commands.autoroute.process import (
PidRecord,
ProcessLaunchError,
UpError,
allocate_free_port,
clear_pid_record,
is_running,
@ -66,6 +67,13 @@ class TestPidRecordRoundTrip:
def test_read_missing_file_returns_none(self, tmp_path):
assert read_pid_record(tmp_path / "missing.json") is None
def test_read_raises_clean_error_on_corrupt_content(self, tmp_path):
path = tmp_path / "pid.json"
path.write_text("not json at all {{{")
with pytest.raises(UpError, match="invalid or unexpected JSON"):
read_pid_record(path)
def test_clear_removes_an_existing_record(self, tmp_path):
path = tmp_path / "pid.json"
write_pid_record(PidRecord(pid=1, port=1, config_path="a", log_path="b"), path)