Keep a page rendering when a token's closure throws - #9
Merged
Merged
Conversation
Two registered closures throwing for a reader with no tenant selected — a value calling a tenant-scoped accessor, a link calling a tenant-scoped route helper — returned a 500 for the whole page, on every page those tokens appeared on. The other 95% of each page had nothing to do with them, and a closure that throws only for some reader contexts is invisible to docent:check by construction, so runtime is the only line of defense. Value and link resolution now runs under a failure policy: substitute nothing, report() the throwable, carry on. The exception still reaches exception tracking, so this defers the fix rather than swallowing it. route: tokens missing a bound parameter get the same treatment. The policy lives on the site registry, so IntegrationRegistry itself stays free of Laravel helpers and keeps propagating by default. render.strict_tokens restores the exception.
Independent review found a blocker and a scope problem. The blocker: a degraded render was written to the agent-Markdown and llms-full caches like any other. The viewer fingerprint cannot see the session state that made a resolver throw — every guest shares one, and a single user moving between tenants keeps theirs — so one transient failure served the missing value to every later reader until the cache was cleared. Before this branch the exception prevented the write entirely. remember() now takes a predicate, and both agent caches skip the write when the render lost a token. The scope problem: the guard wrapped container resolution and string conversion as well as the host closure, so a class-string resolver that does not exist and a resolver returning an array were both swallowed and rendered as a quietly missing value. Those are deterministic defects every reader hits, not reader-specific session state, and the project's own rule says they must fail loudly. The guard now covers only invocation of the application's callable. route() tokens come out from under the policy for the same reason, which also lets attempt() go private again. Narrowing exposed a second bug: delegating an unregistered name to the parent registry ran the resolver under the parent's policy, and the global registry deliberately has none, so a globally registered throwing closure escaped. Resolution now looks the registration up through the chain and invokes it under this registry's policy. Also drops a vacuous test — search never resolves values, so it passed on main — and adds coverage for the cache regression, exact-once reporting of a global registration, and each newly loud failure mode.
…osure agentMarkdown() had a nine-line renderer construction inside a closure inside a method call, so the method's actual shape — build a key, snapshot failures, render and cache — was buried. Pulling the construction into its own method leaves the caller reading as those three steps.
…rendering # Conflicts: # CHANGELOG.md
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
1 participant
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.
Closes #4.
The blast radius
A registered value or link closure that throws took down the entire page — every paragraph around it, on every page that token appeared on. The reported cases were a value calling a tenant-scoped accessor and a link calling a tenant-scoped route helper, both for a reader with no tenant selected: a freshly invited user, or someone mid-account-switch.
That's a badly proportioned failure, and a help center is specifically where someone goes when something is already wrong for them. "Your session is in an odd state, so the documentation is also down" is a bad place to land that reader. There's also no way to catch it ahead of time — a closure that throws only for some reader contexts is invisible to
docent:checkby construction.The policy
Value and link resolution now runs under a failure policy: substitute nothing, hand the throwable to
report(), carry on rendering. The reader gets the content; the error still reaches exception tracking, so this defers the fix rather than swallowing it.render.strict_tokensrestores the exception.Resilient is the default, unconditionally, rather than keyed to
app.debug. Tying it to debug would mean the production behavior is the one never exercised by the test suite, andreport()already surfaces the failure locally.Where it lives
IntegrationRegistrygains an injectable failure handler and keeps propagating when none is set, so it stays usable without a Laravel container as its docblock claims.SiteRegistryinstalls the handler on each site registry and readsrender.strict_tokenslazily at failure time.Only the site registry carries the policy — the global registry it falls back to stays pure, so a failure inside a global registration surfaces at the site registry's handler and gets reported exactly once.
{{ route:… }}resolves through Laravel'sroute()rather than the registry, soIntegrationRegistry::attempt()is public and the renderers put route tokens under the identical policy. A route token missing a bound parameter now costs the reader that link, not the page.Scope
Deliberately limited to values and links. Conditions and audiences have the same unguarded path, but swallowing those means failing closed, which makes a block or a whole page silently disappear — a quiet failure with a different risk calculus than a missing inline value. Worth deciding separately.
Verification
vendor/bin/pest— 636 passed (9 new, covering HTML rendering, the agent markdown feed, search indexing, and strict mode).pint --testandphpstanclean.