Better handling of errors in typst process

Author: RundownRhino

bot/exts/fun/typst.py

@@ -4,6 +4,7 @@
 import sys
 from io import BytesIO
 from pathlib import Path
+from subprocess import CalledProcessError
 from tempfile import TemporaryDirectory
 
 import discord
@@ -112,7 +113,9 @@ async def _ensure_typst_executable(self) -> None:
             stderr=asyncio.subprocess.PIPE,
         )
         stdout, _ = await proc.communicate()
-        log.info(f"Typst executable reports itself as {stdout.decode('utf-8').strip()!r}.")
+        log.info(
+            f"Typst executable reports itself as {stdout.decode('utf-8').strip()!r}."
+        )
 
     async def _download_typst_executable(self) -> None:
         if not Config.typst_archive_url:
@@ -200,7 +203,6 @@ async def typst(self, ctx: commands.Context, *, query: str) -> None:
                     )
                     return
                 except TypstWorkerCrashedError:
-                    log.exception("typst worker crashed")
                     await ctx.send(
                         "Worker process crashed. "
                         f"Perhaps the memory limit of {TYPST_MEMORY_LIMIT/1024**2:.1f}MB was exceeded?"
@@ -228,12 +230,19 @@ async def render_typst(
                         mem_rlimit=TYPST_MEMORY_LIMIT,
                         jobs=WORKER_JOBS,
                     )
-            except RuntimeError as e:
-                raise InvalidTypstError(
-                    e.args[0] if e.args else "<no error message emitted>"
-                )
             except TimeoutError:
                 raise TypstTimeoutError
+            except CalledProcessError as e:
+                err = e.stderr.decode("utf-8")
+                # when the memory limit is reached this usually shows up as signal 6 (SIGABRT), but it can vary
+                if e.returncode < 0:
+                    log.debug(
+                        "Typst subprocess died due to a signal %s",
+                        str(e).split("died with")[-1].strip(),
+                    )
+                    raise TypstWorkerCrashedError
+                # if in doubt we assume it's a normal error and return the logs
+                raise InvalidTypstError(err)
 
         raw_img = res.output
         if len(raw_img) > MAX_RAW_SIZE:

bot/utils/typst.py

@@ -4,6 +4,7 @@
 from dataclasses import dataclass
 from functools import partial
 from pathlib import Path
+from subprocess import CalledProcessError
 from typing import Literal
 
 from bot.constants import Typst as Config
@@ -41,7 +42,7 @@ async def compile_typst(
     """
     typst_path = Path(Config.typst_path).resolve()
     if not typst_path.exists():
-        raise FileNotFoundError("Typst executable was not found at path", typst_path)
+        raise ValueError("Typst executable was not found at path", typst_path)
     if not root_path.is_dir():
         raise ValueError("Root directory was not a directory", root_path)
 
@@ -70,9 +71,16 @@ async def compile_typst(
         )
 
         stdout, stderr = await proc.communicate(input=source.encode("utf-8"))
-    # if the task is cancelled or any other problem happens, make sure to kill the worker
+        if proc.returncode is None:
+            # shouldn't be possible
+            raise RuntimeError("Process didn't terminate after communicate")
+        if proc.returncode != 0:
+            raise CalledProcessError(
+                proc.returncode, [typst_path, *args], stdout, stderr
+            )
+    # if the task is cancelled or any other problem happens, make sure to kill the worker if it still exists
     except BaseException:
-        with contextlib.suppress(UnboundLocalError):
+        with contextlib.suppress(UnboundLocalError, ProcessLookupError):
             proc.kill()
         raise
     return TypstCompileResult(output=stdout, stderr=stderr)

pyproject.toml

@@ -74,6 +74,7 @@ ignore = [
     "RUF029",
     "S311",
     "SIM102", "SIM108",
+    "S404", # S404 is bugged and doesn't respect noqa
 ]
 
 [tool.ruff.lint.isort]