Add structured preview exception hierarchy
PreviewError subclasses carry a concise short label for UI, a full log message, backend name, and metadata fields (OnlyOffice code/status/url, backend stage, timeout seconds, cancel reason). Exceptions travel across the worker pool wire pickled in the binary response payload (ok=False) and are re-raised with their original type on the caller side. Combined pipelines such as pdf+pyvips tag the failing stage.
This commit is contained in:
@@ -17,12 +17,14 @@ from mediapreview.backends import (
|
||||
process_video,
|
||||
)
|
||||
from mediapreview.cache import CachedPreview, PreviewCache
|
||||
from mediapreview.exceptions import PreviewError
|
||||
from mediapreview.formats import is_previewable_path
|
||||
from mediapreview.protocol import PreviewRequest, PreviewResponse
|
||||
|
||||
__all__ = [
|
||||
"CachedPreview",
|
||||
"PreviewCache",
|
||||
"PreviewError",
|
||||
"PreviewRequest",
|
||||
"PreviewResponse",
|
||||
"dispatch",
|
||||
|
||||
@@ -31,6 +31,7 @@ from pathlib import Path
|
||||
from docopt import docopt
|
||||
|
||||
from mediapreview.backends import dispatch
|
||||
from mediapreview.exceptions import PreviewError
|
||||
from mediapreview.util.logformat import EmojiFormatter
|
||||
|
||||
|
||||
@@ -66,12 +67,16 @@ def _preview(args: dict) -> None:
|
||||
sys.stderr.write(f"error: no such file: {path}\n")
|
||||
sys.exit(2)
|
||||
|
||||
try:
|
||||
result, resp = dispatch(
|
||||
path,
|
||||
quality=int(args["-q"]),
|
||||
maxsize=int(args["--maxsize"]),
|
||||
maxzoom=float(args["--maxzoom"]),
|
||||
)
|
||||
except PreviewError as e:
|
||||
sys.stderr.write(f"error: {e}\n")
|
||||
sys.exit(1)
|
||||
if not resp.ok or result is None:
|
||||
sys.stderr.write(f"error: {resp.error or 'preview failed'}\n")
|
||||
if resp.stderr:
|
||||
|
||||
@@ -15,8 +15,8 @@ from mediapreview.backends.image import (
|
||||
)
|
||||
from mediapreview.backends.pdf import process_pdf
|
||||
from mediapreview.backends.video import process_video
|
||||
from mediapreview.exceptions import PreviewError, backend_error
|
||||
from mediapreview.formats import DOC_PREVIEW_SUFFIXES
|
||||
from mediapreview.protocol import PreviewResponse
|
||||
|
||||
__all__ = [
|
||||
"dispatch",
|
||||
@@ -49,13 +49,17 @@ def dispatch(path, quality, maxsize, maxzoom, data=None):
|
||||
if mime_type and mime_type.startswith("image/"):
|
||||
backend = "pyvips"
|
||||
return process_image(path, quality=quality, maxsize=maxsize)
|
||||
except PreviewError:
|
||||
# Already structured (e.g. a stage of a combined pipeline like
|
||||
# pdf+pyvips) — keep the original backend/stage identity.
|
||||
raise
|
||||
except ValueError as e:
|
||||
return None, PreviewResponse(ok=False, backend=backend, error=str(e))
|
||||
raise backend_error(backend, str(e)) from e
|
||||
except ImportError as e:
|
||||
# Missing optional extra — expected, so a plain message, no traceback.
|
||||
logger.error("Preview dispatch failed for %s: %s", path, e) # noqa: TRY400
|
||||
return None, PreviewResponse(ok=False, backend=backend, error=str(e))
|
||||
raise backend_error(backend, str(e)) from e
|
||||
except Exception as e:
|
||||
logger.exception("Preview dispatch failed for %s", path)
|
||||
return None, PreviewResponse(ok=False, backend=backend, error=str(e))
|
||||
return None, PreviewResponse(ok=False, backend=backend, error="preview unsupported")
|
||||
raise backend_error(backend, str(e)) from e
|
||||
raise backend_error(backend, "preview unsupported")
|
||||
|
||||
@@ -5,6 +5,7 @@ from time import perf_counter
|
||||
import pyvips
|
||||
|
||||
from mediapreview.backends.image import AVIF_FAST_EFFORT
|
||||
from mediapreview.exceptions import backend_error
|
||||
from mediapreview.protocol import PreviewResponse
|
||||
|
||||
try:
|
||||
@@ -12,6 +13,8 @@ try:
|
||||
except ImportError: # pragma: no cover - optional pdf extra
|
||||
pymupdf = None
|
||||
|
||||
BACKEND = "pdf+pyvips"
|
||||
|
||||
|
||||
def process_pdf(path, *, maxsize, maxzoom, quality, page_number=0):
|
||||
if pymupdf is None:
|
||||
@@ -19,26 +22,30 @@ def process_pdf(path, *, maxsize, maxzoom, quality, page_number=0):
|
||||
"PDF previews require the 'pdf' extra: pip install mediapreview[pdf]"
|
||||
)
|
||||
t_load_start = perf_counter()
|
||||
try:
|
||||
with pymupdf.open(path) as pdf:
|
||||
page = pdf.load_page(page_number)
|
||||
w, h = page.rect[2:4]
|
||||
zoom = min(maxsize / w, maxsize / h, maxzoom)
|
||||
mat = pymupdf.Matrix(zoom, zoom)
|
||||
pix = page.get_pixmap(matrix=mat)
|
||||
samples, width, height, n = pix.samples_mv, pix.width, pix.height, pix.n
|
||||
except Exception as e:
|
||||
raise backend_error(BACKEND, str(e), stage="pdf") from e
|
||||
t_load_end = perf_counter()
|
||||
|
||||
t_save_start = perf_counter()
|
||||
img = pyvips.Image.new_from_memory(
|
||||
pix.samples_mv, pix.width, pix.height, pix.n, "uchar"
|
||||
)
|
||||
try:
|
||||
img = pyvips.Image.new_from_memory(samples, width, height, n, "uchar")
|
||||
ret = img.write_to_buffer(".avif", Q=quality, effort=AVIF_FAST_EFFORT, keep="none")
|
||||
backend = "pdf+pyvips"
|
||||
except Exception as e:
|
||||
raise backend_error(BACKEND, str(e), stage="pyvips") from e
|
||||
t_save_end = perf_counter()
|
||||
|
||||
return ret, PreviewResponse(
|
||||
ok=True,
|
||||
mime="image/avif",
|
||||
backend=backend,
|
||||
backend=BACKEND,
|
||||
timings=[
|
||||
round((t_load_end - t_load_start) * 1000, 1),
|
||||
round((t_save_end - t_save_start) * 1000, 1),
|
||||
|
||||
@@ -0,0 +1,186 @@
|
||||
"""Structured preview exceptions.
|
||||
|
||||
Preview failures are represented by ``PreviewError`` and a small number of
|
||||
subclasses, carrying their fields directly: ``short`` (concise, for UI with
|
||||
limited space — the backend name is usually printed in front of it, so it is
|
||||
left out), the exception message itself (for logs), ``backend``, and
|
||||
optional subclass-specific metadata such as ``code`` or ``timeout_seconds``
|
||||
for callers that wish to do their own processing.
|
||||
|
||||
The worker pool ships exceptions between processes with pickle (same
|
||||
trust domain — the pool unpickles only data from its own workers), so any
|
||||
exception arrives intact on the caller side, no per-class serialization
|
||||
machinery needed. Because the initializers are keyword-heavy, pickling is
|
||||
routed through ``__dict__`` via ``PreviewError.__reduce__``.
|
||||
|
||||
The hierarchy is intentionally small:
|
||||
|
||||
- ``OnlyOfficeError`` covers all OnlyOffice failures; optional fields
|
||||
(``code``, ``status``, ``url``, ``snippet``) describe the specific failure.
|
||||
- ``PreviewBackendError`` covers backend conversion failures (ffmpeg, pyvips,
|
||||
pdf, etc.); ``stage`` identifies the failing step of a combined pipeline
|
||||
(e.g. "pdf" vs "pyvips" in the "pdf+pyvips" backend).
|
||||
- ``PreviewTimeoutError`` covers timeouts for any backend.
|
||||
- ``PreviewCancelledError`` covers cancellations (e.g. pool shutdown).
|
||||
|
||||
BaseExceptions such as ``KeyboardInterrupt``, ``SystemExit`` and
|
||||
``asyncio.CancelledError`` are never wrapped in these types.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
|
||||
class PreviewError(Exception):
|
||||
"""Base preview exception.
|
||||
|
||||
``short`` is concise text for UIs with limited space (the backend name
|
||||
is usually printed in front of it); ``str(err)`` is the full
|
||||
human-readable message; ``backend`` identifies the backend
|
||||
(e.g. "onlyoffice", "ffmpeg").
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
message: str = "preview failed",
|
||||
short: str = "error",
|
||||
*,
|
||||
backend: str | None = None,
|
||||
):
|
||||
super().__init__(message)
|
||||
self.short = short
|
||||
self.backend = backend
|
||||
|
||||
def __reduce__(self):
|
||||
# Keyword-heavy initializers do not unpickle via args; pass the message
|
||||
# positionally and restore the rest from __dict__.
|
||||
return (type(self), (str(self),), self.__dict__)
|
||||
|
||||
|
||||
class OnlyOfficeError(PreviewError):
|
||||
"""OnlyOffice conversion failed. Specifics are in the extra fields."""
|
||||
|
||||
def __init__( # noqa: PLR0913 - metadata fields are independent
|
||||
self,
|
||||
message: str = "OnlyOffice conversion failed",
|
||||
short: str = "error",
|
||||
*,
|
||||
code: str | None = None,
|
||||
status: int | None = None,
|
||||
url: str | None = None,
|
||||
snippet: str | None = None,
|
||||
backend: str | None = "onlyoffice",
|
||||
):
|
||||
super().__init__(message, short, backend=backend)
|
||||
self.code = code
|
||||
self.status = status
|
||||
self.url = url
|
||||
self.snippet = snippet
|
||||
|
||||
|
||||
class PreviewBackendError(PreviewError):
|
||||
"""Backend conversion failure (image/video/pdf/etc)."""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
message: str = "preview failed",
|
||||
short: str = "error",
|
||||
*,
|
||||
stage: str | None = None,
|
||||
backend: str | None = None,
|
||||
):
|
||||
super().__init__(message, short, backend=backend)
|
||||
self.stage = stage
|
||||
|
||||
|
||||
class PreviewTimeoutError(PreviewError):
|
||||
"""Preview conversion exceeded its timeout for a given backend."""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
message: str = "preview timed out",
|
||||
short: str = "timeout",
|
||||
*,
|
||||
timeout_seconds: float = 0.0,
|
||||
backend: str | None = None,
|
||||
):
|
||||
super().__init__(message, short, backend=backend)
|
||||
self.timeout_seconds = timeout_seconds
|
||||
|
||||
|
||||
class PreviewCancelledError(PreviewError):
|
||||
"""Preview was cancelled (e.g. pool shut down)."""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
message: str = "Preview cancelled (pool closed)",
|
||||
short: str = "cancelled",
|
||||
*,
|
||||
reason: str = "pool closed",
|
||||
backend: str | None = None,
|
||||
):
|
||||
super().__init__(message, short, backend=backend)
|
||||
self.reason = reason
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Factory helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_OO_CODE_ERRORS = {
|
||||
"-8": ("jwt error", "OnlyOffice JWT authentication failed"),
|
||||
"-4": ("input error", "OnlyOffice input error"),
|
||||
"-2": ("timeout error", "OnlyOffice conversion timed out"),
|
||||
"-1": ("unknown error", "OnlyOffice conversion failed with unknown error"),
|
||||
}
|
||||
|
||||
|
||||
def onlyoffice_error_from_code(code: str | None = None) -> OnlyOfficeError:
|
||||
"""Build an OnlyOfficeError from a conversion status code (e.g. "-8")."""
|
||||
if code in _OO_CODE_ERRORS:
|
||||
short, log = _OO_CODE_ERRORS[code]
|
||||
elif code:
|
||||
short, log = f"{code} error", f"OnlyOffice conversion failed: {code}"
|
||||
else:
|
||||
short, log = "unknown error", "OnlyOffice conversion failed with unknown error"
|
||||
return OnlyOfficeError(log, short, code=code)
|
||||
|
||||
|
||||
def onlyoffice_unavailable_error(url: str | None = None) -> OnlyOfficeError:
|
||||
log = "OnlyOffice document server not reachable"
|
||||
if url:
|
||||
log = f"{log} at {url}"
|
||||
return OnlyOfficeError(log, "unavailable", url=url)
|
||||
|
||||
|
||||
def onlyoffice_http_error(status: int) -> OnlyOfficeError:
|
||||
return OnlyOfficeError(f"OnlyOffice HTTP error: {status}", "http error", status=status)
|
||||
|
||||
|
||||
def onlyoffice_no_fileurl_error(snippet: str | None = None) -> OnlyOfficeError:
|
||||
log = "OnlyOffice response did not contain FileUrl"
|
||||
if snippet:
|
||||
log = f"{log}: {snippet}"
|
||||
return OnlyOfficeError(log, "no-fileurl error", snippet=snippet)
|
||||
|
||||
|
||||
def backend_error(backend: str, message: str, *, stage: str | None = None) -> PreviewBackendError:
|
||||
short = message.splitlines()[0][:60]
|
||||
return PreviewBackendError(
|
||||
f"[{backend}] preview failed: {message}",
|
||||
short,
|
||||
backend=backend,
|
||||
stage=stage,
|
||||
)
|
||||
|
||||
|
||||
def preview_timeout_error(backend: str, timeout_seconds: float) -> PreviewTimeoutError:
|
||||
return PreviewTimeoutError(
|
||||
f"{backend.capitalize()} preview timed out after {timeout_seconds}s",
|
||||
"timeout",
|
||||
backend=backend,
|
||||
timeout_seconds=timeout_seconds,
|
||||
)
|
||||
|
||||
|
||||
def preview_cancelled_error(reason: str = "pool closed") -> PreviewCancelledError:
|
||||
return PreviewCancelledError(f"Preview cancelled ({reason})", "cancelled", reason=reason)
|
||||
+41
-23
@@ -30,6 +30,14 @@ from pathlib import Path
|
||||
from time import perf_counter
|
||||
from urllib.parse import quote
|
||||
|
||||
from mediapreview.exceptions import (
|
||||
onlyoffice_error_from_code,
|
||||
onlyoffice_http_error,
|
||||
onlyoffice_no_fileurl_error,
|
||||
onlyoffice_unavailable_error,
|
||||
preview_timeout_error,
|
||||
)
|
||||
|
||||
try:
|
||||
import httpx
|
||||
import jwt
|
||||
@@ -45,6 +53,7 @@ logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
_httpx_client: httpx.AsyncClient | None = None
|
||||
_httpx_client_loop: asyncio.AbstractEventLoop | None = None
|
||||
|
||||
|
||||
def _get_onlyoffice_url() -> str:
|
||||
@@ -82,35 +91,27 @@ def _get_callback_host() -> str:
|
||||
return "127.0.0.1"
|
||||
|
||||
|
||||
def onlyoffice_error_short_text(detail: str) -> str:
|
||||
"""Short human-readable label for an OnlyOffice failure, for log annotation."""
|
||||
if detail.startswith("OnlyOffice conversion error:"):
|
||||
code = detail.rsplit(":", 1)[-1].strip()
|
||||
return {
|
||||
"-8": "onlyoffice jwt error",
|
||||
"-4": "onlyoffice input error",
|
||||
"-2": "onlyoffice timeout error",
|
||||
"-1": "onlyoffice unknown error",
|
||||
}.get(code, f"onlyoffice {code} error")
|
||||
if "OnlyOffice response did not contain FileUrl" in detail:
|
||||
return "onlyoffice no-fileurl error"
|
||||
return "onlyoffice error"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# ---------------------------------------------------------------------------
|
||||
# Async HTTP client
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def get_httpx_client() -> httpx.AsyncClient:
|
||||
"""Return the shared async HTTP client for OnlyOffice requests."""
|
||||
"""Return the shared async HTTP client for OnlyOffice requests.
|
||||
|
||||
The client is recreated if the running event loop changes, because an
|
||||
``httpx.AsyncClient`` is bound to the loop that created it.
|
||||
"""
|
||||
if httpx is None:
|
||||
raise ImportError(
|
||||
"OnlyOffice integration requires the 'office' extra: pip install mediapreview[office]"
|
||||
)
|
||||
global _httpx_client
|
||||
if _httpx_client is None:
|
||||
global _httpx_client, _httpx_client_loop
|
||||
current_loop = asyncio.get_running_loop()
|
||||
if _httpx_client is None or _httpx_client_loop is not current_loop:
|
||||
_httpx_client = httpx.AsyncClient()
|
||||
_httpx_client_loop = current_loop
|
||||
return _httpx_client
|
||||
|
||||
|
||||
@@ -330,6 +331,7 @@ async def convert_to_png_async(file_path: Path, request_timeout: float = 5.0) ->
|
||||
headers["Authorization"] = token
|
||||
|
||||
t_start = perf_counter()
|
||||
try:
|
||||
response = await client.post(
|
||||
convert_url,
|
||||
content=json.dumps(payload).encode(),
|
||||
@@ -337,19 +339,26 @@ async def convert_to_png_async(file_path: Path, request_timeout: float = 5.0) ->
|
||||
timeout=request_timeout,
|
||||
)
|
||||
response.raise_for_status()
|
||||
except httpx.TimeoutException as e:
|
||||
raise preview_timeout_error("onlyoffice", request_timeout) from e
|
||||
except httpx.HTTPStatusError as e:
|
||||
raise onlyoffice_http_error(e.response.status_code) from e
|
||||
except httpx.RequestError as e:
|
||||
raise onlyoffice_unavailable_error(_get_onlyoffice_url()) from e
|
||||
body = response.content
|
||||
t_end = perf_counter()
|
||||
|
||||
# Parse XML response
|
||||
text = body.decode("utf-8", errors="replace")
|
||||
if "<Error>" in text:
|
||||
code = "unknown"
|
||||
if "<Error>" in text and "</Error>" in text:
|
||||
code = None
|
||||
if "</Error>" in text:
|
||||
code = text.split("<Error>")[1].split("</Error>")[0]
|
||||
raise RuntimeError(f"OnlyOffice conversion error: {code}")
|
||||
raise onlyoffice_error_from_code(code)
|
||||
|
||||
if "<FileUrl>" not in text:
|
||||
raise RuntimeError("OnlyOffice response did not contain FileUrl")
|
||||
snippet = text if len(text) <= 200 else text[:200] + "..."
|
||||
raise onlyoffice_no_fileurl_error(snippet)
|
||||
|
||||
file_url = text.split("<FileUrl>")[1].split("</FileUrl>")[0]
|
||||
file_url = file_url.replace("&", "&")
|
||||
@@ -357,8 +366,17 @@ async def convert_to_png_async(file_path: Path, request_timeout: float = 5.0) ->
|
||||
logger.debug("OnlyOffice converted in %.2fs: %s", t_end - t_start, file_url)
|
||||
|
||||
# Download converted PNG
|
||||
try:
|
||||
png_response = await client.get(file_url, timeout=request_timeout)
|
||||
png_response.raise_for_status()
|
||||
except httpx.TimeoutException as e:
|
||||
raise preview_timeout_error("onlyoffice", request_timeout) from e
|
||||
except httpx.HTTPStatusError as e:
|
||||
raise onlyoffice_http_error(e.response.status_code) from e
|
||||
except httpx.RequestError as e:
|
||||
# The converted file lives on the OO server, so a request failure here
|
||||
# usually means OO itself could not be reached after conversion.
|
||||
raise onlyoffice_unavailable_error(_get_onlyoffice_url()) from e
|
||||
return png_response.content
|
||||
finally:
|
||||
await asyncio.to_thread(httpd.shutdown)
|
||||
@@ -385,7 +403,7 @@ class OOConversionManager:
|
||||
async def convert(self, filepath: Path) -> bytes:
|
||||
"""Return PNG bytes for *filepath*, deduplicating concurrent requests."""
|
||||
if not await is_available_cached():
|
||||
raise RuntimeError("OnlyOffice server not reachable")
|
||||
raise onlyoffice_unavailable_error(_get_onlyoffice_url())
|
||||
stat = await asyncio.to_thread(filepath.stat)
|
||||
key = f"{filepath}:{stat.st_mtime_ns}"
|
||||
|
||||
|
||||
+50
-51
@@ -4,6 +4,7 @@ import asyncio
|
||||
import contextlib
|
||||
import logging
|
||||
import os
|
||||
import pickle
|
||||
import signal
|
||||
import struct
|
||||
import sys
|
||||
@@ -20,6 +21,13 @@ except ImportError as e: # pragma: no cover - optional worker extra
|
||||
"The worker pool requires the 'worker' extra: pip install mediapreview[worker]"
|
||||
) from e
|
||||
|
||||
from mediapreview.exceptions import (
|
||||
PreviewError,
|
||||
PreviewTimeoutError,
|
||||
backend_error,
|
||||
preview_cancelled_error,
|
||||
preview_timeout_error,
|
||||
)
|
||||
from mediapreview.formats import (
|
||||
expected_backend as _expected_preview_backend,
|
||||
)
|
||||
@@ -35,7 +43,6 @@ from mediapreview.protocol import PreviewRequest, PreviewResponse
|
||||
__all__ = [
|
||||
"PREVIEW_TIMEOUT",
|
||||
"PreviewError",
|
||||
"PreviewPoolClosedError",
|
||||
"PreviewTimeoutError",
|
||||
"generate_office_preview",
|
||||
"is_previewable_path",
|
||||
@@ -47,33 +54,6 @@ __all__ = [
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class PreviewTimeoutError(Exception):
|
||||
"""Raised when the preview subprocess exceeds PREVIEW_TIMEOUT."""
|
||||
|
||||
def __init__(self, message: str, *, backend: str | None = None):
|
||||
super().__init__(message)
|
||||
self.backend = backend
|
||||
|
||||
|
||||
class PreviewError(Exception):
|
||||
"""Raised when the preview subprocess exits with a non-zero status."""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
message: str,
|
||||
*,
|
||||
stderr: str | None = None,
|
||||
backend: str | None = None,
|
||||
):
|
||||
super().__init__(message)
|
||||
self.stderr = stderr
|
||||
self.backend = backend
|
||||
|
||||
|
||||
class PreviewPoolClosedError(PreviewError):
|
||||
"""The preview worker pool has been shut down."""
|
||||
|
||||
|
||||
class WorkerChecksumError(Exception):
|
||||
"""Raised when worker response checksum does not match the packet."""
|
||||
|
||||
@@ -82,6 +62,20 @@ class WorkerProtocolError(Exception):
|
||||
"""Raised when worker response packet is malformed."""
|
||||
|
||||
|
||||
def _reraise_worker_error(resp: PreviewResponse, payload: bytes) -> None:
|
||||
"""Re-raise the exception the worker sent back in the payload, if any.
|
||||
|
||||
The payload is pickled by our own worker processes (same trust domain), so
|
||||
the original exception type arrives intact on the caller side. Falls back
|
||||
to a plain PreviewBackendError built from the error message.
|
||||
"""
|
||||
if payload:
|
||||
exc = pickle.loads(payload) # noqa: S301 - trusted: our own workers
|
||||
if isinstance(exc, Exception):
|
||||
raise exc
|
||||
raise backend_error(resp.backend or "unknown", resp.error or "preview worker error")
|
||||
|
||||
|
||||
PREVIEW_TIMEOUT = 10.0 # seconds until preview subprocess is killed
|
||||
PREVIEW_WORKERS = max(2, min(8, cpu_count()))
|
||||
WORKER_KILL_GRACE = 5.0 # max seconds to wait for a killed worker to be reaped
|
||||
@@ -139,11 +133,7 @@ class _PreviewWorker:
|
||||
|
||||
resp = msgspec.json.decode(meta_raw, type=PreviewResponse)
|
||||
if not resp.ok:
|
||||
raise PreviewError(
|
||||
resp.error or "preview worker error",
|
||||
stderr=resp.stderr,
|
||||
backend=resp.backend,
|
||||
)
|
||||
_reraise_worker_error(resp, payload)
|
||||
return payload or None, resp
|
||||
|
||||
async def kill(self) -> None:
|
||||
@@ -279,9 +269,9 @@ class _PreviewWorkerPool:
|
||||
)
|
||||
if not future.done():
|
||||
future.set_exception(
|
||||
PreviewTimeoutError(
|
||||
args[0].name,
|
||||
backend=_expected_preview_backend(args[0]),
|
||||
preview_timeout_error(
|
||||
_expected_preview_backend(args[0]),
|
||||
PREVIEW_TIMEOUT,
|
||||
)
|
||||
)
|
||||
return
|
||||
@@ -305,9 +295,9 @@ class _PreviewWorkerPool:
|
||||
)
|
||||
if not future.done():
|
||||
future.set_exception(
|
||||
PreviewTimeoutError(
|
||||
filepath.name,
|
||||
backend=_expected_preview_backend(filepath),
|
||||
preview_timeout_error(
|
||||
_expected_preview_backend(filepath),
|
||||
PREVIEW_TIMEOUT,
|
||||
)
|
||||
)
|
||||
except WorkerChecksumError:
|
||||
@@ -319,7 +309,10 @@ class _PreviewWorkerPool:
|
||||
)
|
||||
if not future.done():
|
||||
future.set_exception(
|
||||
PreviewError(f"worker checksum mismatch for {filepath.name}")
|
||||
backend_error(
|
||||
_expected_preview_backend(filepath),
|
||||
f"worker checksum mismatch for {filepath.name}",
|
||||
)
|
||||
)
|
||||
except PreviewError as e:
|
||||
if not future.done():
|
||||
@@ -342,14 +335,20 @@ class _PreviewWorkerPool:
|
||||
)
|
||||
if not future.done():
|
||||
future.set_exception(
|
||||
PreviewError(f"worker protocol failure for {filepath.name}: {e}")
|
||||
backend_error(
|
||||
_expected_preview_backend(filepath),
|
||||
f"worker protocol failure for {filepath.name}: {e}",
|
||||
)
|
||||
)
|
||||
except Exception:
|
||||
replace = True
|
||||
logger.exception("Unexpected preview worker error for %s", filepath.name)
|
||||
if not future.done():
|
||||
future.set_exception(
|
||||
PreviewError(f"unexpected worker error for {filepath.name}")
|
||||
backend_error(
|
||||
_expected_preview_backend(filepath),
|
||||
f"unexpected worker error for {filepath.name}",
|
||||
)
|
||||
)
|
||||
finally:
|
||||
if replace:
|
||||
@@ -378,7 +377,7 @@ class _PreviewWorkerPool:
|
||||
data: bytes | None = None,
|
||||
):
|
||||
if self._closed:
|
||||
raise PreviewPoolClosedError("preview worker pool closed")
|
||||
raise preview_cancelled_error("preview worker pool closed")
|
||||
loop = asyncio.get_running_loop()
|
||||
future = loop.create_future()
|
||||
self._in_flight.add(future)
|
||||
@@ -410,18 +409,14 @@ class _PreviewWorkerPool:
|
||||
# out their timeouts during server shutdown.
|
||||
for future in list(self._in_flight):
|
||||
if not future.done():
|
||||
future.set_exception(
|
||||
PreviewPoolClosedError("preview worker pool closed")
|
||||
)
|
||||
future.set_exception(preview_cancelled_error("pool closed"))
|
||||
while not self._pending.empty():
|
||||
try:
|
||||
_priority, _seq, future, _args = self._pending.get_nowait()
|
||||
except asyncio.QueueEmpty:
|
||||
break
|
||||
if not future.done():
|
||||
future.set_exception(
|
||||
PreviewPoolClosedError("preview worker pool closed")
|
||||
)
|
||||
future.set_exception(preview_cancelled_error("pool closed"))
|
||||
while not self._idle.empty():
|
||||
try:
|
||||
self._idle.get_nowait()
|
||||
@@ -469,7 +464,11 @@ async def shutdown_preview_workers() -> None:
|
||||
async def generate_office_preview(
|
||||
filepath: Path, quality: int, maxsize: int, maxzoom: float
|
||||
) -> tuple[bytes | None, PreviewResponse | None]:
|
||||
"""Generate a preview for an office file using OnlyOffice + worker AVIF conversion."""
|
||||
"""Generate a preview for an office file using OnlyOffice + worker AVIF conversion.
|
||||
|
||||
Raises:
|
||||
OnlyOfficeError: If the OnlyOffice Document Server cannot convert the file.
|
||||
"""
|
||||
manager = get_oo_manager()
|
||||
t_oo_start = perf_counter()
|
||||
png_bytes = await manager.convert(filepath)
|
||||
@@ -490,5 +489,5 @@ async def run_preview(
|
||||
"""Run preview request in a persistent worker process."""
|
||||
await start_preview_workers()
|
||||
if _preview_pool is None:
|
||||
raise PreviewPoolClosedError("preview worker pool closed")
|
||||
raise preview_cancelled_error("preview worker pool closed")
|
||||
return await _preview_pool.run(filepath, quality, maxsize, maxzoom, data)
|
||||
|
||||
@@ -11,7 +11,7 @@ class PreviewRequest(msgspec.Struct, omit_defaults=True):
|
||||
|
||||
|
||||
class PreviewResponse(msgspec.Struct, omit_defaults=True):
|
||||
ok: bool
|
||||
ok: bool # Indicates whether binary payload is the file or a pickled exception
|
||||
mime: str | None = None
|
||||
backend: str | None = None
|
||||
timings: list[float] | None = None
|
||||
|
||||
+19
-10
@@ -19,6 +19,7 @@ import contextlib
|
||||
import io
|
||||
import logging
|
||||
import os
|
||||
import pickle
|
||||
import signal
|
||||
import struct
|
||||
import sys
|
||||
@@ -43,6 +44,18 @@ from mediapreview.util.logformat import format_level_prefix
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _serialize_exception(e: BaseException) -> bytes:
|
||||
"""Pickle an exception for re-raising across the pool wire.
|
||||
|
||||
Falls back to an empty payload (caller uses the plain error message) in
|
||||
the unlikely case the exception cannot be pickled.
|
||||
"""
|
||||
try:
|
||||
return pickle.dumps(e)
|
||||
except Exception:
|
||||
return b""
|
||||
|
||||
|
||||
class _WorkerLogFormatter(logging.Formatter):
|
||||
"""Emoji level prefix like the main process, tagged with the worker pid."""
|
||||
|
||||
@@ -125,21 +138,17 @@ def _run_loop() -> None:
|
||||
result, resp = dispatch(
|
||||
Path(req.path), req.quality, req.maxsize, req.maxzoom, data
|
||||
)
|
||||
if not resp.ok:
|
||||
captured = stderr_capture.getvalue().strip()
|
||||
if captured:
|
||||
resp = PreviewResponse(
|
||||
ok=False,
|
||||
backend=resp.backend,
|
||||
error=resp.error,
|
||||
stderr=captured,
|
||||
)
|
||||
_write_response(resp, result or b"")
|
||||
except Exception as e:
|
||||
logger.exception("Preview worker error for %s", req.path)
|
||||
captured = stderr_capture.getvalue().strip()
|
||||
_write_response(
|
||||
PreviewResponse(ok=False, error=str(e), stderr=captured or None), b""
|
||||
PreviewResponse(
|
||||
ok=False,
|
||||
error=str(e),
|
||||
stderr=captured or None,
|
||||
),
|
||||
_serialize_exception(e),
|
||||
)
|
||||
finally:
|
||||
root_logger.removeHandler(handler)
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
"""Tests for OnlyOffice error handling and structured responses."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pickle
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from mediapreview import office
|
||||
from mediapreview.exceptions import (
|
||||
OnlyOfficeError,
|
||||
PreviewBackendError,
|
||||
backend_error,
|
||||
onlyoffice_error_from_code,
|
||||
onlyoffice_http_error,
|
||||
onlyoffice_no_fileurl_error,
|
||||
onlyoffice_unavailable_error,
|
||||
)
|
||||
from mediapreview.pool import generate_office_preview
|
||||
|
||||
FILES = Path(__file__).resolve().parent / "files"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("code", "short", "message"),
|
||||
[
|
||||
("-8", "jwt error", "OnlyOffice JWT authentication failed"),
|
||||
("-4", "input error", "OnlyOffice input error"),
|
||||
("-2", "timeout error", "OnlyOffice conversion timed out"),
|
||||
("-1", "unknown error", "OnlyOffice conversion failed with unknown error"),
|
||||
("-99", "-99 error", "OnlyOffice conversion failed: -99"),
|
||||
(None, "unknown error", "OnlyOffice conversion failed with unknown error"),
|
||||
],
|
||||
)
|
||||
def test_onlyoffice_error_from_code(code, short, message):
|
||||
"""Conversion status codes map to clean labels and messages."""
|
||||
err = onlyoffice_error_from_code(code)
|
||||
assert str(err) == message
|
||||
assert err.short == short
|
||||
assert err.code == code
|
||||
assert err.backend == "onlyoffice"
|
||||
assert isinstance(err, OnlyOfficeError)
|
||||
|
||||
|
||||
def test_onlyoffice_http_error():
|
||||
err = onlyoffice_http_error(502)
|
||||
assert str(err) == "OnlyOffice HTTP error: 502"
|
||||
assert err.status == 502
|
||||
assert err.short == "http error"
|
||||
|
||||
|
||||
def test_onlyoffice_unavailable_error():
|
||||
err = onlyoffice_unavailable_error("http://localhost:8988")
|
||||
assert "http://localhost:8988" in str(err)
|
||||
assert err.url == "http://localhost:8988"
|
||||
assert err.short == "unavailable"
|
||||
|
||||
|
||||
def test_onlyoffice_no_fileurl_error():
|
||||
err = onlyoffice_no_fileurl_error("<empty />")
|
||||
assert "<empty />" in str(err)
|
||||
assert err.snippet == "<empty />"
|
||||
assert err.short == "no-fileurl error"
|
||||
|
||||
|
||||
def test_backend_error_stage():
|
||||
"""Combined pipelines tag the failing stage."""
|
||||
err = backend_error("pdf+pyvips", "cannot read document", stage="pdf")
|
||||
assert err.backend == "pdf+pyvips"
|
||||
assert err.stage == "pdf"
|
||||
assert err.short == "cannot read document"
|
||||
assert isinstance(err, PreviewBackendError)
|
||||
|
||||
|
||||
def test_error_pickle_round_trip():
|
||||
"""Exceptions survive pickling (the worker pool wire) intact."""
|
||||
err = onlyoffice_error_from_code("-8")
|
||||
restored = pickle.loads(pickle.dumps(err))
|
||||
assert type(restored) is type(err)
|
||||
assert str(restored) == str(err)
|
||||
assert restored.code == err.code
|
||||
assert restored.short == err.short
|
||||
assert restored.backend == err.backend
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_generate_office_preview_raises_structured_error(monkeypatch):
|
||||
"""On OnlyOffice failure, generate_office_preview raises OnlyOfficeError."""
|
||||
async def fake_convert(_filepath: Path, request_timeout: float = 5.0) -> bytes:
|
||||
raise onlyoffice_error_from_code("-8")
|
||||
|
||||
monkeypatch.setattr(office, "convert_to_png_async", fake_convert)
|
||||
|
||||
with pytest.raises(OnlyOfficeError) as exc_info:
|
||||
await generate_office_preview(
|
||||
FILES / "file-sample_100kB.docx",
|
||||
quality=60,
|
||||
maxsize=512,
|
||||
maxzoom=2.0,
|
||||
)
|
||||
|
||||
err = exc_info.value
|
||||
assert err.code == "-8"
|
||||
assert err.short == "jwt error"
|
||||
assert str(err) == "OnlyOffice JWT authentication failed"
|
||||
Reference in New Issue
Block a user