Display typst compile errors to the user in a friendly way#74
Conversation
It's been deprecated in Sphinx 9, and will be removed in Sphinx 11.
Typst can throw an compile error if the input was malformed. Currently, `typst.compile()` raises a TypstError exception that bubbles up to Sphinx, which then displays a stacktrace. Unfortunately, this stacktrace does not contain information about the typst code that actually failed to compile. Capture the exception, and rethrow it with the right information included. For example, the unit test gives the following output: ``` E sphinx.errors.SphinxError: Typst compilation failed for '/tmp/pytest-of-philipp/pytest-19/typst-compile-error/_build/typstpdf/index.typ': error: unclosed delimiter E ┌─ ../../../../tmp/pytest-of-philipp/pytest-19/typst-compile-error/_build/typstpdf/index.typ:169:16 E │ E 169 │ #let unclosed = [ E │ ^ ```
📝 WalkthroughWalkthroughThe PR removes the Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_builders.py (1)
90-91: ⚡ Quick winAssert the wrapped error includes file-context prefix.
The test currently validates Typst’s diagnostic text but not your wrapper contract (
Typst compilation failed for ...). Add one assertion so regressions in the rethrow message are caught.Proposed test tightening
- with pytest.raises(SphinxError, match="unclosed delimiter"): - app.build() + with pytest.raises(SphinxError, match="unclosed delimiter") as exc_info: + app.build() + + msg = str(exc_info.value) + assert "Typst compilation failed for" in msg + assert "index.typ" in msg🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_builders.py` around lines 90 - 91, The test with pytest.raises currently only validates the Typst diagnostic message "unclosed delimiter" but does not verify that the wrapper error message includes the file-context prefix "Typst compilation failed for". After the app.build() call that raises the SphinxError, add an additional assertion to capture the raised exception and verify that its full message string contains the expected wrapper prefix, ensuring the error rethrow logic that adds the file-context wrapper is functioning correctly and preventing regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_builders.py`:
- Around line 90-91: The test with pytest.raises currently only validates the
Typst diagnostic message "unclosed delimiter" but does not verify that the
wrapper error message includes the file-context prefix "Typst compilation failed
for". After the app.build() call that raises the SphinxError, add an additional
assertion to capture the raised exception and verify that its full message
string contains the expected wrapper prefix, ensuring the error rethrow logic
that adds the file-context wrapper is functioning correctly and preventing
regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01d02bbe-4a2d-45c1-b59f-27ab937adc7c
📒 Files selected for processing (5)
src/atsphinx/typst/builders.pysrc/atsphinx/typst/writer.pytests/test_builders.pytests/testdocs/test-typst-compile-error/conf.pytests/testdocs/test-typst-compile-error/index.rst
1 participant
Typst can throw an compile error if the input was malformed. Currently,
typst.compile()raises a TypstError exception that bubbles up toSphinx, which then displays a stacktrace. Unfortunately, this stacktrace
does not contain information about the typst code that actually failed
to compile.
Capture the exception, and rethrow it with the right information
included. For example, the unit test gives the following output:
Additionally, this PR resolves deprecation warnings that were introduced in Sphinx 9. I included these changes as a separate commit in this PR because the changes conflict otherwise. If you prefer to have two PRs let me know and I'll resolve the merge conflicts after the first PR goes in.
Summary by CodeRabbit
Bug Fixes
Improvements
Tests