Log effect exceptions and clear the render lock owner - #56
Merged
Conversation
An exception in an effect (or an effect cleanup) was only collected into exceptions_self and then shown as an error page. The render loop logged it at INFO, so a production incident left nothing at WARNING or above: an effect that broke the widget tree was invisible in server logs and in Sentry. Component-body exceptions were already logged with logger.exception, so effects now do the same at every catch site, in both renderers. Control flow is unchanged. render() set _lock_thread when it took the lock but never cleared it. The recursion guard therefore fired on a stale owner: thread A rendered last, thread B holds the lock (close() takes it without setting _lock_thread), and A calling render() got "Recursive render detected" instead of waiting. Clearing _lock_thread before the lock is released makes the guard describe the current owner only.
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.
Why
Two things found while chasing a production crash (reacton PR #54).
logger.exceptionin_render. In a server deployment with thereactonlogger at WARNING, an effect crash therefore never reaches the logs or Sentry: the crash behind PR Survive a widget/component slot flip inside one render call #54 was invisible for months, users saw the traceback page and nothing else recorded it.render()sets_lock_threadafter acquiringthread_lockand never clears it.close()takes the lock without setting it. The recursion guard at the top ofrender()compares_lock_threadto the current thread while the lock is held, so a thread that merely rendered last, whileclose()runs on another thread, gets a falseRuntimeError("Recursive render detected")instead of waiting.What
logger.exception("Effect %r raised exception %r", ...)and the cleanup variant at every catch site in both renderers, no control-flow change._lock_thread = Nonein thefinallyofrender(), before the lock is released.exc_info; aclose()on another thread makesrender()wait instead of raising.185 passed on both renderers.
🤖 Generated with Claude Code