feat(workflows): degrade gracefully when a status body fails to render#1306
Merged
Conversation
A template render error in one status surface previously propagated out of the lifecycle handler dispatch and aborted the whole `aspect` invocation — that's how the missing `failure_snippets` macro (fixed in #1305) crashed the process instead of just skipping the surface. Add a non-throwing `ctx.template.try_jinja2()` runtime builtin that returns `(ok, text_or_error)` instead of raising, then route every status renderer (bazel/lint/format/gazelle/delivery/warming, summary and details bodies) through a shared `render_{summary,details}_or_fallback` helper. On a render error it logs a `WARNING:` line to CLI output with the message and emits a minimal, macro-free fallback body embedding the same error. The status, title, and conclusion are computed in AXL outside the template, so the check still posts with the correct verdict — only the body degrades. Summary templates carry no macros today and can't hit that failure class; guarding them too is defense-in-depth so the renderer is categorically unable to crash the invocation on a future template edit. Tests: a Rust unit test pins `jinja2_render`'s error surfacing (the contract try_jinja2 relies on), and an AXL test renders a deliberately-broken details template and asserts the fallback body appears, the WARNING is emitted, and the status/title survive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a8dc43563
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
✨ Aspect Workflows Tasks📅 Wed Jul 1 20:31:03 UTC 2026
|
- Extract template_data_to_json in the runtime, collapsing the dict→JSON
loop duplicated across handlebars/jinja2/try_jinja2/liquid.
- Unit-test the shared render_{details,summary}_or_fallback helpers directly
in bazel_results_test (passthrough + fallback for both), covering the
summary-fallback path that only had transitive coverage.
- Correct the helper docstrings: the size guard makes the trim cascade a
no-op on a fallback body; callers don't branch on the returned `ok`.
- Trim stale/duplicated inline comments now covered by the helper docstrings.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The details override in `templates` can be a user template, so a render failure isn't necessarily an aspect-cli bug. Drop the "This is a bug in aspect-cli" line; keep the neutral "status above is still accurate." Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
All three template engines now have a non-throwing try_* variant returning (ok, text). Factor the shared render-Result → tuple mapping into render_result_to_tuple so each try_* is a one-liner. Tests: per-engine ok/error Rust unit tests pin the *_render contract, and an AXL test drives all three try_* methods (success + malformed) through the real evaluator. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses Codex review: the details trim cascade re-rendered with the throwing ctx.template.jinja2 after the first render. A template that only errors on a trimmed shape (e.g. after a section is emptied) would still abort the invocation. Each surface now re-renders via a local _render_details closure over render_details_or_fallback, so a mid-cascade render error also degrades to the fallback body. Also fixes a pre-existing crash the new regression test surfaced: the lint cascade computed `g["count"] - len(kept)`, but count is stringified for the template — `string - int` panics once row-halving runs. Parse it back and re-stringify overflow to match the rendered field shape. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes a template render error in any CI status surface degrade gracefully instead of aborting the whole
aspectprocess.Lifecycle handler dispatch calls each status surface's renderer in a bare loop, so an error raised while rendering one surface's body unwinds the entire invocation. AXL has no
try/except, so the fix starts at the template-expansion layer.Changes:
ctx.template.try_jinja2/try_handlebars/try_liquid— like their raising counterparts but return a(ok, text_or_error)tuple, so a render failure becomes a value the caller can branch on.render_summary_or_fallback/render_details_or_fallback(inbazel_results.axl): on render failure they log aWARNING:to CLI output with the error and return a minimal, macro-free fallback body embedding the same message.The status, title, and conclusion are computed in AXL outside the template, so the check still posts with the correct verdict — only the body text degrades. The fallback body is far under the size limit, so each surface's existing trim cascade is a no-op on it. Templates can be user-supplied overrides, so the fallback attributes nothing to aspect-cli.
Example CLI output when a template fails to render:
Changes are visible to end-users: yes
Suggested release notes
aspectrun — the affected check posts a fallback body with the correct status and aWARNING:is logged.Test plan
jinja2/handlebars/liquidrender, pinning the contract thetry_*variants rely on.bazel_results_test: drive all threetry_*engines (success + malformed), and exerciserender_details_or_fallback/render_summary_or_fallbackdirectly (passthrough + fallback for both bodies).render_check_outputand asserts the fallback body appears and the status/title survive.