fix(render): recover View panics in initial render and re-render (#48)#50
Merged
Conversation
View renders had no panic recovery, unlike action handlers and the OnInit/OnConnect/OnDispose lifecycle guards. A panic in viewFn escaped to the embedding http.Server: a dropped connection plus a naked stderr stack on the initial GET, and a crashed process on the broadcast SyncNow goroutine — with no via log line either way. Wrap both viewFn callsites: renderView logs and writes a controlled 500 on the GET path; renderFragment logs and drops the fragment on the async re-render path (action autoflush + broadcast), letting the signal flush still proceed. Recovery is unconditional; mw.Recover stays as the backstop for non-via handlers.
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 #48.
Problem
View renders had no panic recovery, unlike action handlers (
WithActionErrorHandler) and theOnInit/OnConnect/OnDisposelifecycle guards. A panic inviewFnescaped the framework:http.Server: naked stderr stack + dropped connection (no controlled 500, no via log line).flushDirty) — escaped via two callers: the action autoflush defer (dropped the action POST connection) and thego c.SyncNow()broadcast goroutine, where an unrecovered panic crashed the whole process.The asymmetry was the surprise: the same programmer mistake got wildly different treatment depending on which callback it sat in — and the precise
on.*panic messages from #36 surface at View time, exactly where the framework didn't structure them.Fix
Two sibling guards wrap every
viewFncallsite, matching the existing recovery patterns:renderView(initial GET) — recovers, logs viaa.logErr("View panicked: %v"), writes a controlled 500, and short-circuits the page document.renderFragment(async re-render, used byflushDirty) — recovers, logs, and returns""(a no-op prepend, dropped by the drain'selems != ""guard). The dirty-signal flush still proceeds. Covers both the action-autoflush and broadcast-goroutine callers centrally.Recovery is unconditional (the issue's open question) —
mw.Recoverstays as the backstop for non-via handlers, plugin endpoints, and custom middleware.Tests
Built test-first (RYGBA TDD); each confirmed failing for the right reason before implementing (the broadcast test crashed the test binary in Red, as designed):
TestView_panicReachesTheConfiguredLoggerNotBareStderrTestView_panicProducesControlled500NotADroppedConnection(also pins: no half-rendered document appended after the 500)TestView_panicDuringReRenderDoesNotEscapeToTheServerTestView_panicDuringReRenderStillFlushesDirtySignals(pins: recover wraps only the render, not the whole flush)TestView_panicInBroadcastReRenderIsRecoveredNotProcessCrashingFull suite green under
go test -race ./...(and-count=20stress on the panic tests).