Skip to content

Commit 9fbbbc7

Browse files
kodjima33claude
andauthored
fix(backend): return 400 for chat attachments the file pipeline cannot process (#11853)
POST /v1|/v2/files answered HTTP 500 for two kinds of ordinary user input, observed live on prod image c5b0b5d (api.omi.me): PIL.UnidentifiedImageError: cannot identify image file '/tmp/<uuid>_<name>.heic' routers/chat.py -> chat_file.FileChatTool.upload -> File.generate_thumbnail openai.BadRequestError: Error code: 400 - Invalid extension ogg routers/chat.py -> chat_file.FileChatTool.upload -> openai.files.create mimetypes maps .heic to image/heic, so File.is_image() is true and the thumbnail step runs even though Pillow ships no HEIF decoder -- an iPhone camera-roll photo crashed the route. An .ogg voice note reached OpenAI Files, whose accepted-extension list has no audio formats, and the provider's 400 propagated unhandled. Both are request input arriving over a public route, so both belong on the 4xx side of the request boundary. chat_file now raises a typed UnsupportedChatFileError at the two points where an unprocessable file is identified, and both upload endpoints map it to HTTPException(400) with the extension in the detail. Nothing else changes: the provider is not called for an undecodable image, and supported files upload as before. Verification - New tests/unit/test_chat_file_upload_unsupported.py drives POST /v2/files and POST /v1/files over HTTP against the real routers.chat and the real utils.other.chat_file (only the provider client, auth and Firestore stubbed): heic -> 400, provider-rejected extension -> 400, text/plain -> 200 and persisted. - Control against pre-fix source: 4 of the 5 fail with the verbatim prod exceptions (UnidentifiedImageError, openai.BadRequestError); the happy-path test passes both before and after, confirming no behavior change there. - Real file, not a synthetic blob: a HEIC produced by macOS `sips -s format heic` raises the same UnidentifiedImageError in Pillow 12.2, and driving that file through POST /v2/files returns 500 on pre-fix source and 400 {"detail":"Unsupported attachment: 'heic' files are not supported in chat."} with the fix. - _chat_router_test_harness stubs utils.other.chat_file with the exact leaves routers.chat imports, so the new name was added to that stub inventory; without it test_chat_stream_error_fallback (11) and test_chat_quota_counting_router (9) fail to import. Both green again. - backend/test.sh over the 17 chat/file/upload suites: 64 passed, 5 deselected. Failure-Class: new Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 2ab4332 commit 9fbbbc7

5 files changed

Lines changed: 204 additions & 6 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{
2+
"schema_version": 1,
3+
"id": "FC-request-input-rejection-escapes-as-server-fault",
4+
"violated_contract": "A route owns the classification of its own request input. When a library or an upstream provider rejects something the caller supplied -- a media type with no decoder, an extension outside a vendor's accepted set -- that rejection is a bad request, and the route must answer 4xx. Letting the library's or provider's exception propagate turns ordinary user input into a 5xx: the client can say nothing useful to the user, the failure is indistinguishable from a real outage in serving metrics, and every retry re-pages.",
5+
"canonical_prevention": "Catch the narrow, input-caused exception types at the point where the unprocessable input is identified, convert them to one typed domain error, and map that error to an explicit 4xx at the route boundary with a detail the client can show. Catch only the input-caused types -- transport, disk and quota failures must still surface as 5xx, because those are server faults. A regression test drives the real route over HTTP with the offending input and asserts the status code the client receives.",
6+
"canonical_prevention_artifact": [
7+
"backend/utils/other/chat_file.py",
8+
"backend/routers/chat.py",
9+
"backend/tests/unit/test_chat_file_upload_unsupported.py"
10+
],
11+
"evidence_prs": [
12+
11853
13+
],
14+
"scope_hints": [
15+
"backend/routers/**"
16+
],
17+
"status": "open"
18+
}

backend/routers/chat.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@
6767
from utils.subscription import enforce_chat_quota, is_trial_paywalled
6868
from utils import share_links
6969
from utils.other import endpoints as auth, storage
70-
from utils.other.chat_file import FileChatTool
70+
from utils.other.chat_file import FileChatTool, UnsupportedChatFileError
7171
from utils.multipart import (
7272
CHAT_FILE_MAX_PART_SIZE,
7373
MultipartMaxPartSizeRoute,
@@ -1515,7 +1515,10 @@ def upload_file_chat(
15151515
with temp_file.open("wb") as buffer:
15161516
shutil.copyfileobj(file.file, buffer)
15171517

1518-
result = FileChatTool.upload(temp_file)
1518+
try:
1519+
result = FileChatTool.upload(temp_file)
1520+
except UnsupportedChatFileError as error:
1521+
raise HTTPException(status_code=400, detail=str(error))
15191522

15201523
thumb_name = result.get("thumbnail_name", "")
15211524
if thumb_name != "":
@@ -1580,7 +1583,10 @@ def upload_file_chat_v1(
15801583
with temp_file.open("wb") as buffer:
15811584
shutil.copyfileobj(file.file, buffer)
15821585

1583-
result = FileChatTool.upload(temp_file)
1586+
try:
1587+
result = FileChatTool.upload(temp_file)
1588+
except UnsupportedChatFileError as error:
1589+
raise HTTPException(status_code=400, detail=str(error))
15841590

15851591
thumb_name = result.get("thumbnail_name", "")
15861592
if thumb_name != "":

backend/tests/unit/_chat_router_test_harness.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,9 @@ def with_rate_limit(func, _policy):
177177
storage.schedule_syncing_temporal_file_deletion = MagicMock()
178178
chat_file = install('utils.other.chat_file', ModuleType('utils.other.chat_file'))
179179
chat_file.FileChatTool = MagicMock()
180+
# routers.chat imports this name; the stub must carry it or the module fails to load. A local
181+
# subclass keeps the real module (PIL, openai, database) out of these suites' import graph.
182+
chat_file.UnsupportedChatFileError = type('UnsupportedChatFileError', (Exception,), {})
180183

181184
sync_files = install('utils.sync.files', ModuleType('utils.sync.files'))
182185
sync_files.retrieve_file_paths = MagicMock(return_value=[])
Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
"""POST /v1|/v2/files answers 400 for a file type the pipeline cannot process, never 500.
2+
3+
Live prod signature (backend image c5b0b5d, api.omi.me):
4+
* ``PIL.UnidentifiedImageError: cannot identify image file '/tmp/<uuid>_<name>.heic'``
5+
- mimetypes maps .heic to image/heic, so File.is_image() is true and generate_thumbnail()
6+
runs, but Pillow ships no HEIF decoder. An iPhone camera-roll photo 500'd.
7+
* ``openai.BadRequestError: Invalid extension ogg`` from openai.files.create - the provider's
8+
accepted-extension list has no audio formats. An .ogg voice note 500'd.
9+
10+
Both are ordinary user input arriving over a public route, so both belong on the 4xx side of the
11+
request boundary. The route is driven over HTTP here (real routers.chat, real utils.other.chat_file)
12+
so the assertion is the status code the client actually receives.
13+
"""
14+
15+
import sys
16+
from types import ModuleType, SimpleNamespace
17+
from unittest.mock import MagicMock
18+
19+
import openai
20+
import pytest
21+
from fastapi import FastAPI
22+
from fastapi.testclient import TestClient
23+
24+
from tests.unit import _chat_router_test_harness as harness
25+
from tests.unit._chat_router_test_harness import BACKEND_DIR
26+
27+
# A one-byte payload with a .heic name: mimetypes keys off the extension, and Pillow fails to
28+
# identify the bytes exactly as it does for real HEIC input.
29+
HEIC_BYTES = b'\x00\x00\x00\x18ftypheic\x00\x00\x00\x00heicmif1'
30+
OGG_BYTES = b'OggS\x00\x02' + b'\x00' * 20
31+
32+
33+
def _make_chat_client():
34+
saved = {k: v for k, v in sys.modules.items()}
35+
36+
harness.install_package('models', BACKEND_DIR / 'models')
37+
harness.install_package('database', BACKEND_DIR / 'database')
38+
harness.install_package('utils', BACKEND_DIR / 'utils')
39+
harness.install_package('utils.other', BACKEND_DIR / 'utils' / 'other')
40+
harness.install_package('utils.sync', BACKEND_DIR / 'utils' / 'sync')
41+
harness.install_package('utils.stt', BACKEND_DIR / 'utils' / 'stt')
42+
harness.install_package('utils.llm', BACKEND_DIR / 'utils' / 'llm')
43+
harness.install_package('utils.retrieval', BACKEND_DIR / 'utils' / 'retrieval')
44+
45+
harness.wire_common_stubs(harness.install_module)
46+
harness.install_module('models.app')
47+
48+
# The gateway telemetry chat_file imports is out of scope here (and pulls the real gateway
49+
# client stack in); keep it inert so the upload path itself is what runs.
50+
gateway_client = harness.install_module('utils.llm.gateway_client', ModuleType('utils.llm.gateway_client'))
51+
gateway_client.should_route_features_through_gateway = MagicMock(return_value=False)
52+
gateway_obs = harness.install_module(
53+
'utils.llm.gateway_observability', ModuleType('utils.llm.gateway_observability')
54+
)
55+
gateway_obs.record_direct_exception_surface = MagicMock()
56+
57+
# wire_common_stubs replaces chat_file with a MagicMock; this suite needs the real module,
58+
# because the defect lives in its PIL and provider error handling.
59+
harness.load_real_module('utils.other.chat_file', BACKEND_DIR / 'utils' / 'other' / 'chat_file.py')
60+
61+
chat_utils = harness.install_module('utils.chat', ModuleType('utils.chat'))
62+
for name in (
63+
'acquire_chat_session',
64+
'emit_stream_error_fallback',
65+
'initial_message_util',
66+
'process_voice_message_segment_stream',
67+
'resolve_voice_message_language',
68+
'transcribe_voice_message_segment',
69+
'transcribe_pcm_bytes',
70+
):
71+
setattr(chat_utils, name, MagicMock())
72+
73+
graph = harness.install_module('utils.retrieval.graph', ModuleType('utils.retrieval.graph'))
74+
graph.execute_chat_stream = MagicMock()
75+
graph.execute_graph_chat = MagicMock()
76+
graph.execute_persona_chat_stream = MagicMock()
77+
78+
sys.modules.pop('routers.chat', None)
79+
module = harness.load_real_module('routers.chat', BACKEND_DIR / 'routers' / 'chat.py')
80+
81+
app = FastAPI()
82+
app.include_router(module.router)
83+
return TestClient(app), module, saved
84+
85+
86+
@pytest.fixture
87+
def chat_client():
88+
client, module, saved = _make_chat_client()
89+
try:
90+
yield client, module
91+
finally:
92+
harness.cleanup(saved)
93+
94+
95+
def _bad_request(**_kwargs):
96+
raise openai.BadRequestError(
97+
message="Error code: 400 - Invalid extension ogg.",
98+
response=SimpleNamespace(request=None, status_code=400, headers={}),
99+
body=None,
100+
)
101+
102+
103+
@pytest.mark.parametrize('route', ['/v2/files', '/v1/files'])
104+
def test_heic_photo_is_rejected_as_bad_request(chat_client, route, monkeypatch):
105+
client, module = chat_client
106+
chat_file = sys.modules['utils.other.chat_file']
107+
# Nothing should reach the provider: the failure is local, in thumbnail generation.
108+
monkeypatch.setattr(chat_file.openai, 'files', SimpleNamespace(create=_unreachable))
109+
110+
response = client.post(route, files={'files': ('photo.heic', HEIC_BYTES, 'image/heic')})
111+
112+
assert response.status_code == 400
113+
assert 'heic' in response.json()['detail']
114+
module.chat_db.add_multi_files.assert_not_called()
115+
116+
117+
@pytest.mark.parametrize('route', ['/v2/files', '/v1/files'])
118+
def test_provider_rejected_extension_is_bad_request(chat_client, route, monkeypatch):
119+
client, module = chat_client
120+
chat_file = sys.modules['utils.other.chat_file']
121+
monkeypatch.setattr(chat_file.openai, 'files', SimpleNamespace(create=_bad_request))
122+
123+
response = client.post(route, files={'files': ('note.ogg', OGG_BYTES, 'audio/ogg')})
124+
125+
assert response.status_code == 400
126+
assert 'ogg' in response.json()['detail']
127+
module.chat_db.add_multi_files.assert_not_called()
128+
129+
130+
def test_supported_file_still_uploads(chat_client, monkeypatch):
131+
"""The guard must not swallow the happy path."""
132+
client, module = chat_client
133+
chat_file = sys.modules['utils.other.chat_file']
134+
monkeypatch.setattr(
135+
chat_file.openai,
136+
'files',
137+
SimpleNamespace(create=lambda **_kwargs: SimpleNamespace(id='file-1', filename='note.txt')),
138+
)
139+
140+
response = client.post('/v2/files', files={'files': ('note.txt', b'hello', 'text/plain')})
141+
142+
assert response.status_code == 200
143+
assert response.json()[0]['openai_file_id'] == 'file-1'
144+
module.chat_db.add_multi_files.assert_called_once()
145+
146+
147+
def _unreachable(**_kwargs):
148+
raise AssertionError('provider upload must not be attempted for an undecodable image')

backend/utils/other/chat_file.py

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
ChatCompletionContentPartParam,
1212
ChatCompletionMessageParam,
1313
)
14-
from PIL import Image
14+
from PIL import Image, UnidentifiedImageError
1515
from pydantic import ValidationError
1616

1717
import database.chat as chat_db
@@ -26,6 +26,21 @@
2626
_FILE_SEARCH_ASSISTANT_MODEL = "gpt-4.1"
2727

2828

29+
class UnsupportedChatFileError(Exception):
30+
"""A chat attachment this pipeline cannot process.
31+
32+
The upload routes own the client contract: a file type we cannot handle is bad request
33+
input, not a server fault. Without this, PIL (an iPhone .heic photo has no decoder) and
34+
OpenAI Files (an .ogg voice note is not an accepted extension) escape as 500s.
35+
"""
36+
37+
38+
def _unsupported_chat_file_error(file_path: Union[str, Path]) -> UnsupportedChatFileError:
39+
suffix = Path(file_path).suffix.lstrip('.').lower()
40+
label = f"'{suffix}' files are" if suffix else "this file type is"
41+
return UnsupportedChatFileError(f"Unsupported attachment: {label} not supported in chat.")
42+
43+
2944
def _safe_file_chats(files_data: List[Dict[str, Any]]) -> List[FileChat]:
3045
"""Build FileChat objects from raw file docs, skipping (not raising on) a malformed one.
3146
@@ -158,12 +173,20 @@ def upload(file_path: Union[str, Path]) -> Dict[str, Any]:
158173
file.get_mime_type()
159174

160175
if file.is_image():
161-
file.generate_thumbnail()
176+
try:
177+
file.generate_thumbnail()
178+
except UnidentifiedImageError as error:
179+
# An image mime type Pillow has no decoder for (.heic from an iPhone camera roll).
180+
raise _unsupported_chat_file_error(file_path) from error
162181
file.purpose = "vision"
163182

164183
with open(file_path, 'rb') as f:
165184
# upload file to OpenAI
166-
response = openai.files.create(file=f, purpose=cast(Any, file.purpose))
185+
try:
186+
response = openai.files.create(file=f, purpose=cast(Any, file.purpose))
187+
except openai.BadRequestError as error:
188+
# The provider rejects the extension (audio/video, archives it does not index).
189+
raise _unsupported_chat_file_error(file_path) from error
167190
if response:
168191
file.file_id = response.id
169192
file.file_name = response.filename

0 commit comments

Comments
 (0)