mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
fix(cli): surface clean errors instead of raw tracebacks in lite up/down
load_json_or_empty and read_backup both delegate to pydantic's validate_json, which raises ValidationError on invalid JSON or a non-object root -- neither up() nor down() caught it, so a corrupt settings or backup file surfaced an unformatted Python traceback instead of a clean CLI error. Both now convert to UpError, and down() (previously uncaught entirely) and up()'s teardown path now handle it. restore_claude_settings also gained a parent.mkdir guard before rewriting CLAUDE_SETTINGS_PATH: if ~/.claude/ was removed while `lite up` was running, the restore would crash before deleting the backup file, permanently stranding it and breaking every future `lite down`.
This commit is contained in:
parent
0b930ea919
commit
f7fc41c94c
2 changed files with 74 additions and 5 deletions
|
|
@ -13,7 +13,7 @@ from types import FrameType
|
|||
from typing import IO, Iterator, Mapping
|
||||
|
||||
import click
|
||||
from pydantic import JsonValue, TypeAdapter
|
||||
from pydantic import JsonValue, TypeAdapter, ValidationError
|
||||
|
||||
from litellm.litellm_core_utils.cli_token_utils import is_cli_token_fresh
|
||||
|
||||
|
|
@ -52,7 +52,10 @@ def load_json_or_empty(path: Path) -> dict[str, JsonValue]:
|
|||
content = f.read()
|
||||
if not content.strip():
|
||||
return {}
|
||||
return _SETTINGS_ADAPTER.validate_json(content)
|
||||
try:
|
||||
return _SETTINGS_ADAPTER.validate_json(content)
|
||||
except ValidationError:
|
||||
raise UpError(f"{path} contains invalid JSON (or its root is not an object); cannot proceed safely.")
|
||||
|
||||
|
||||
def merge_claude_settings(
|
||||
|
|
@ -105,7 +108,11 @@ def read_backup(backup_path: Path | None = None) -> BackupRecord | None:
|
|||
if not path.exists():
|
||||
return None
|
||||
with open(path, "r") as f:
|
||||
return _BACKUP_RECORD_ADAPTER.validate_json(f.read())
|
||||
content = f.read()
|
||||
try:
|
||||
return _BACKUP_RECORD_ADAPTER.validate_json(content)
|
||||
except ValidationError:
|
||||
raise UpError(f"{path} contains invalid or unexpected JSON; cannot restore from it safely.")
|
||||
|
||||
|
||||
def restore_claude_settings(settings_path: Path | None = None, backup_path: Path | None = None) -> BackupRecord | None:
|
||||
|
|
@ -119,6 +126,7 @@ def restore_claude_settings(settings_path: Path | None = None, backup_path: Path
|
|||
if record is None:
|
||||
return None
|
||||
if record.existed and record.content is not None:
|
||||
resolved_settings_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
with open(resolved_settings_path, "w") as f:
|
||||
json.dump(record.content, f, indent=2)
|
||||
elif resolved_settings_path.exists():
|
||||
|
|
@ -228,8 +236,15 @@ def up(ctx: click.Context) -> None:
|
|||
stop_event.set()
|
||||
|
||||
def _restore_once() -> None:
|
||||
if restored.acquire(blocking=False):
|
||||
if not restored.acquire(blocking=False):
|
||||
return
|
||||
try:
|
||||
_restore_and_report()
|
||||
except UpError as e:
|
||||
# Runs from atexit/a signal handler, 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)
|
||||
|
||||
signal.signal(signal.SIGINT, _handle_signal)
|
||||
signal.signal(signal.SIGTERM, _handle_signal)
|
||||
|
|
@ -246,7 +261,10 @@ def down() -> None:
|
|||
Use this after a `lite up` process was killed uncleanly (e.g. `kill -9`)
|
||||
instead of stopped with Ctrl-C.
|
||||
"""
|
||||
_restore_and_report()
|
||||
try:
|
||||
_restore_and_report()
|
||||
except UpError as e:
|
||||
raise click.ClickException(str(e))
|
||||
|
||||
|
||||
__all__ = [
|
||||
|
|
|
|||
|
|
@ -89,6 +89,18 @@ class TestLoadJsonOrEmpty:
|
|||
path.write_text(json.dumps({"theme": "dark"}))
|
||||
assert load_json_or_empty(path) == {"theme": "dark"}
|
||||
|
||||
def test_raises_clean_error_on_invalid_json(self, tmp_path):
|
||||
path = tmp_path / "settings.json"
|
||||
path.write_text("not json at all {{{")
|
||||
with pytest.raises(UpError, match="invalid JSON"):
|
||||
load_json_or_empty(path)
|
||||
|
||||
def test_raises_clean_error_on_non_object_root(self, tmp_path):
|
||||
path = tmp_path / "settings.json"
|
||||
path.write_text(json.dumps([1, 2, 3]))
|
||||
with pytest.raises(UpError, match="invalid JSON"):
|
||||
load_json_or_empty(path)
|
||||
|
||||
|
||||
class TestBackupRoundTrip:
|
||||
def test_restores_original_content_when_file_existed(self, monkeypatch, tmp_path):
|
||||
|
|
@ -123,6 +135,26 @@ class TestBackupRoundTrip:
|
|||
assert restore_claude_settings() is None
|
||||
assert not settings_path.exists()
|
||||
|
||||
def test_recreates_claude_dir_if_it_was_deleted_while_up_was_running(self, monkeypatch, tmp_path):
|
||||
"""If ~/.claude/ is removed while `lite up` holds it open, restoring must recreate the
|
||||
directory rather than crash with FileNotFoundError and strand the backup file, which
|
||||
would otherwise permanently break every future `lite down`."""
|
||||
claude_dir = tmp_path / "claude_dir"
|
||||
settings_path = claude_dir / "settings.json"
|
||||
backup_path = tmp_path / "backup.json"
|
||||
monkeypatch.setattr(up_module, "CLAUDE_SETTINGS_PATH", settings_path)
|
||||
monkeypatch.setattr(up_module, "BACKUP_PATH", backup_path)
|
||||
original = {"theme": "dark"}
|
||||
claude_dir.mkdir(parents=True)
|
||||
write_backup(BackupRecord(existed=True, content=original))
|
||||
shutil.rmtree(claude_dir)
|
||||
|
||||
restored = restore_claude_settings()
|
||||
|
||||
assert restored is not None
|
||||
assert json.loads(settings_path.read_text()) == original
|
||||
assert not backup_path.exists()
|
||||
|
||||
def test_read_backup_round_trips_write_backup(self, monkeypatch, tmp_path):
|
||||
_patch_paths(monkeypatch, tmp_path)
|
||||
write_backup(BackupRecord(existed=True, content={"a": 1}))
|
||||
|
|
@ -132,6 +164,14 @@ class TestBackupRoundTrip:
|
|||
_patch_paths(monkeypatch, tmp_path)
|
||||
assert read_backup() is None
|
||||
|
||||
def test_read_backup_raises_clean_error_on_corrupt_content(self, monkeypatch, tmp_path):
|
||||
_settings_path, backup_path = _patch_paths(monkeypatch, tmp_path)
|
||||
backup_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
backup_path.write_text("not json at all {{{")
|
||||
|
||||
with pytest.raises(UpError, match="invalid or unexpected JSON"):
|
||||
read_backup()
|
||||
|
||||
def test_write_backup_restricts_permissions_for_a_new_file(self, monkeypatch, tmp_path):
|
||||
_settings_path, backup_path = _patch_paths(monkeypatch, tmp_path)
|
||||
write_backup(BackupRecord(existed=True, content={"a": 1}))
|
||||
|
|
@ -345,3 +385,14 @@ class TestDownCommand:
|
|||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert "Nothing to restore." in result.output
|
||||
|
||||
def test_surfaces_clean_error_on_a_corrupt_backup_file(self, monkeypatch, tmp_path):
|
||||
_settings_path, backup_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 result.exception is None or isinstance(result.exception, SystemExit)
|
||||
assert "invalid or unexpected JSON" in result.output
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue