Skip to content

Commit 067f827

Browse files
flowbrixclaude
andcommitted
fix(backend): return 404 instead of 500 for audio of failed generations
A failed generation stores an empty audio_path. resolve_storage_path("") resolved to the data directory itself, which exists, so the route's 404 guard passed and FileResponse raised RuntimeError ("File at path .../data is not a file"), surfacing as a 500. - resolve_storage_path now returns None for empty paths - audio routes check is_file() instead of exists() so directories never reach FileResponse - GET /audio/{generation_id} reports "Generation failed; no audio available" when the generation status is failed Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent f2cf2a7 commit 067f827

3 files changed

Lines changed: 127 additions & 5 deletions

File tree

backend/config.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@ def to_storage_path(path: str | Path) -> str:
7676

7777
def resolve_storage_path(path: str | Path | None) -> Path | None:
7878
"""Resolve a DB-stored path against the configured data dir."""
79-
if path is None:
79+
# Empty paths (e.g. failed generations) must not resolve to the data
80+
# dir itself, which exists and would defeat the callers' 404 guards.
81+
if not path:
8082
return None
8183

8284
stored_path = Path(path)

backend/routes/audio.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ async def get_version_audio(version_id: str, db: Session = Depends(get_db)):
3434
raise HTTPException(status_code=404, detail="Version not found")
3535

3636
audio_path = config.resolve_storage_path(version.audio_path)
37-
if audio_path is None or not audio_path.exists():
37+
if audio_path is None or not audio_path.is_file():
3838
raise HTTPException(status_code=404, detail="Audio file not found")
3939

4040
return FileResponse(
@@ -52,8 +52,13 @@ async def get_audio(generation_id: str, db: Session = Depends(get_db)):
5252
raise HTTPException(status_code=404, detail="Generation not found")
5353

5454
audio_path = config.resolve_storage_path(generation.audio_path)
55-
if audio_path is None or not audio_path.exists():
56-
raise HTTPException(status_code=404, detail="Audio file not found")
55+
if audio_path is None or not audio_path.is_file():
56+
detail = (
57+
"Generation failed; no audio available"
58+
if generation.status == "failed"
59+
else "Audio file not found"
60+
)
61+
raise HTTPException(status_code=404, detail=detail)
5762

5863
return FileResponse(
5964
audio_path,
@@ -72,7 +77,7 @@ async def get_sample_audio(sample_id: str, db: Session = Depends(get_db)):
7277
raise HTTPException(status_code=404, detail="Sample not found")
7378

7479
audio_path = config.resolve_storage_path(sample.audio_path)
75-
if audio_path is None or not audio_path.exists():
80+
if audio_path is None or not audio_path.is_file():
7681
raise HTTPException(status_code=404, detail="Audio file not found")
7782

7883
return FileResponse(
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
"""
2+
Regression tests for GET /audio/{generation_id} on failed generations.
3+
4+
A failed generation stores an empty ``audio_path``. Previously,
5+
``config.resolve_storage_path("")`` resolved to the data directory itself,
6+
which exists, so the route's 404 guard passed and ``FileResponse`` raised
7+
``RuntimeError: File at path .../data is not a file`` — a 500 instead of
8+
a clean 404.
9+
10+
Usage:
11+
python -m pytest backend/tests/test_audio_failed_generation.py -v
12+
"""
13+
14+
import sys
15+
from pathlib import Path
16+
17+
import pytest
18+
from fastapi import FastAPI
19+
from sqlalchemy import create_engine
20+
from sqlalchemy.orm import sessionmaker
21+
from starlette.testclient import TestClient
22+
23+
# Repo root on sys.path so ``backend`` imports as a package (the audio
24+
# routes use package-relative imports).
25+
sys.path.insert(0, str(Path(__file__).parent.parent.parent))
26+
27+
from backend import config
28+
from backend.database import Base, Generation, VoiceProfile, get_db
29+
from backend.routes.audio import router as audio_router
30+
31+
32+
def test_resolve_storage_path_empty_returns_none():
33+
"""An empty stored path must not resolve to the data dir itself."""
34+
assert config.resolve_storage_path("") is None
35+
assert config.resolve_storage_path(None) is None
36+
37+
38+
@pytest.fixture()
39+
def client(tmp_path, monkeypatch):
40+
"""Minimal app with only the audio routes and a temp sqlite DB."""
41+
monkeypatch.setattr(config, "_data_dir", tmp_path)
42+
43+
engine = create_engine(
44+
f"sqlite:///{tmp_path / 'test.db'}",
45+
connect_args={"check_same_thread": False},
46+
)
47+
Base.metadata.create_all(bind=engine)
48+
TestingSessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine)
49+
50+
session = TestingSessionLocal()
51+
profile = VoiceProfile(id="profile-1", name="Test Profile")
52+
session.add(profile)
53+
54+
session.add_all(
55+
[
56+
Generation(
57+
id="gen-failed-empty",
58+
profile_id="profile-1",
59+
text="failed generation",
60+
audio_path="",
61+
status="failed",
62+
error="engine exploded",
63+
),
64+
Generation(
65+
id="gen-failed-null",
66+
profile_id="profile-1",
67+
text="failed generation",
68+
audio_path=None,
69+
status="failed",
70+
),
71+
Generation(
72+
id="gen-missing-file",
73+
profile_id="profile-1",
74+
text="completed but file deleted",
75+
audio_path="generations/does-not-exist.wav",
76+
status="completed",
77+
),
78+
]
79+
)
80+
session.commit()
81+
session.close()
82+
83+
app = FastAPI()
84+
app.include_router(audio_router)
85+
86+
def override_get_db():
87+
db = TestingSessionLocal()
88+
try:
89+
yield db
90+
finally:
91+
db.close()
92+
93+
app.dependency_overrides[get_db] = override_get_db
94+
return TestClient(app)
95+
96+
97+
@pytest.mark.parametrize("generation_id", ["gen-failed-empty", "gen-failed-null"])
98+
def test_failed_generation_returns_404(client, generation_id):
99+
"""Failed generations (empty/null audio_path) get a clean 404, not a 500."""
100+
response = client.get(f"/audio/{generation_id}")
101+
assert response.status_code == 404
102+
assert response.json()["detail"] == "Generation failed; no audio available"
103+
104+
105+
def test_missing_audio_file_returns_404(client):
106+
"""A completed generation whose file vanished still 404s."""
107+
response = client.get("/audio/gen-missing-file")
108+
assert response.status_code == 404
109+
assert response.json()["detail"] == "Audio file not found"
110+
111+
112+
def test_unknown_generation_returns_404(client):
113+
response = client.get("/audio/no-such-generation")
114+
assert response.status_code == 404
115+
assert response.json()["detail"] == "Generation not found"

0 commit comments

Comments
 (0)