fix(products): bomb-safe image processing + per-image error isolation; honest claim/resumability docs (SLICE-7 review)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -89,23 +89,28 @@ def _process_one(pool, store, image: dict, allow_private: bool) -> None:
|
||||
with pool.connection() as conn:
|
||||
repo.mark_image_failed(conn, image_id, str(exc)[:200]); conn.commit()
|
||||
return
|
||||
result = images_mod.process(data)
|
||||
if isinstance(result, images_mod.Rejected):
|
||||
reason = ("below the resolution bar" if result.reason == "rejected_low_res"
|
||||
else "not a supported image")
|
||||
try:
|
||||
result = images_mod.process(data)
|
||||
if isinstance(result, images_mod.Rejected):
|
||||
reason = ("below the resolution bar" if result.reason == "rejected_low_res"
|
||||
else "not a supported image")
|
||||
with pool.connection() as conn:
|
||||
repo.mark_image_rejected(conn, image_id, result.reason, reason); conn.commit()
|
||||
return
|
||||
keys = {"original": _key(sf, image_id, "original"),
|
||||
"thumb": _key(sf, image_id, "thumb.webp"),
|
||||
"card": _key(sf, image_id, "card.webp"),
|
||||
"detail": _key(sf, image_id, "detail.webp")}
|
||||
store.put(keys["original"], data, ctype)
|
||||
store.put(keys["thumb"], result.renditions["thumb"], "image/webp")
|
||||
store.put(keys["card"], result.renditions["card"], "image/webp")
|
||||
store.put(keys["detail"], result.renditions["detail"], "image/webp")
|
||||
with pool.connection() as conn:
|
||||
repo.mark_image_rejected(conn, image_id, result.reason, reason); conn.commit()
|
||||
return
|
||||
keys = {"original": _key(sf, image_id, "original"),
|
||||
"thumb": _key(sf, image_id, "thumb.webp"),
|
||||
"card": _key(sf, image_id, "card.webp"),
|
||||
"detail": _key(sf, image_id, "detail.webp")}
|
||||
store.put(keys["original"], data, ctype)
|
||||
store.put(keys["thumb"], result.renditions["thumb"], "image/webp")
|
||||
store.put(keys["card"], result.renditions["card"], "image/webp")
|
||||
store.put(keys["detail"], result.renditions["detail"], "image/webp")
|
||||
with pool.connection() as conn:
|
||||
repo.mark_image_fetched(conn, image_id, keys); conn.commit()
|
||||
repo.mark_image_fetched(conn, image_id, keys); conn.commit()
|
||||
except Exception as exc: # noqa: BLE001 — one bad image must never wedge the run (review fix)
|
||||
with pool.connection() as conn:
|
||||
repo.mark_image_failed(conn, image_id, f"processing error: {type(exc).__name__}")
|
||||
conn.commit()
|
||||
|
||||
|
||||
def run_image_phase(pool, store, run_id: int, allow_private: bool) -> None:
|
||||
|
||||
@@ -60,7 +60,7 @@ def process(data: bytes) -> Processed | Rejected:
|
||||
height=height,
|
||||
renditions=renditions,
|
||||
)
|
||||
except (UnidentifiedImageError, OSError, ValueError):
|
||||
except (UnidentifiedImageError, OSError, ValueError, Image.DecompressionBombError):
|
||||
return Rejected(reason="rejected_not_image")
|
||||
|
||||
|
||||
|
||||
@@ -53,3 +53,13 @@ def test_not_an_image_rejected_not_image():
|
||||
def test_jpeg_source_format_preserved_in_result():
|
||||
result = images.process(_jpeg(800, 800))
|
||||
assert result.source_format == "JPEG"
|
||||
|
||||
|
||||
def test_decompression_bomb_rejected_not_image(monkeypatch):
|
||||
# A small file whose pixel count exceeds Pillow's bomb threshold must be a
|
||||
# typed rejection, never an escaping exception.
|
||||
from PIL import Image as PILImage
|
||||
monkeypatch.setattr(PILImage, "MAX_IMAGE_PIXELS", 100) # 10x10 exceeds it
|
||||
result = images.process(_png(800, 800))
|
||||
assert isinstance(result, images.Rejected)
|
||||
assert result.reason == "rejected_not_image"
|
||||
|
||||
@@ -121,6 +121,28 @@ def test_run_phase_classifies_each_image(pool, host, tmp_path):
|
||||
assert _run_status(pool, rid) == "complete_with_problems"
|
||||
|
||||
|
||||
def test_unexpected_processing_error_marks_image_failed_not_wedged(pool, host, tmp_path, monkeypatch):
|
||||
# If anything after the fetch raises unexpectedly (e.g. objectstore.put),
|
||||
# the image is marked failed and the run still reaches a terminal status —
|
||||
# never stranded in fetching_images.
|
||||
store = objectstore.LocalObjectStore(str(tmp_path))
|
||||
sf, _acct, rid = _seed(pool)
|
||||
p = _product(pool, sf, "lamp")
|
||||
iid = _image(pool, p, f"{host}/good.png", rid, position=1)
|
||||
|
||||
def _boom(*a, **k):
|
||||
raise RuntimeError("storage down")
|
||||
monkeypatch.setattr(store, "put", _boom)
|
||||
|
||||
imagefetch.run_image_phase(pool, store, rid, allow_private=True)
|
||||
with pool.connection() as conn:
|
||||
counts = repo.run_image_counts(conn, rid)
|
||||
assert counts["failed"] == 1 and counts["pending"] == 0
|
||||
assert _run_status(pool, rid) == "complete_with_problems"
|
||||
row = _img_row(pool, iid)
|
||||
assert row["status"] == "failed"
|
||||
|
||||
|
||||
def test_resume_after_kill_completes_remaining(pool, host, tmp_path):
|
||||
store = objectstore.LocalObjectStore(str(tmp_path))
|
||||
sf, _acct, rid = _seed(pool)
|
||||
|
||||
Reference in New Issue
Block a user