fix: stop discarding errors that had somewhere to go - #140
Merged
Merged
Conversation
meszmate
force-pushed
the
feat/fallible-model-callbacks
branch
from
August 14, 2026 04:37
1fb58a9 to
7d94d85
Compare
An audit of the 194 `catch {}` sites in src. Most are in component
`view` functions, where the signature genuinely cannot report anything
-- those need the fallible-view treatment and are left for a follow-up.
These are the ones where an error was being dropped despite having a
caller that could handle it.
`LayerStack.render` returned `""` when the cell grid could not be
allocated and swallowed every write failure after that, so an
out-of-memory frame reached the screen as a truncated one, looking like
a compositing bug. It is fallible now.
The `.cache_image` and `.delete_image` commands dropped every error,
while `.image_file` right next to them propagated. An unsupported
protocol already returns false rather than erroring, so what was being
discarded was genuine I/O failure -- and a cache_image that quietly
failed left the following place_cached_image drawing nothing, with no
signal anywhere.
`dev_console` swallowed an append that cannot fail: the slice is a
subset of what was just cleared, so the capacity is already there.
`appendSliceAssumeCapacity` says so.
The rest -- terminal cleanup, the suspend path -- are best-effort by
nature: they run while unwinding, with nobody left to report to. Those
now carry a comment saying why, so the next reader does not have to
work it out again.
meszmate
force-pushed
the
fix/stop-swallowing-errors
branch
from
August 14, 2026 04:50
6141588 to
d1a2ca6
Compare
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.
Item 6 from the review: an audit of the 194
catch {}sites insrc.163 of them are in component
viewfunctions, where the signature genuinely cannot report anything. Those need the fallible-view treatment (#132) applied component by component and are left for a follow-up. This PR is the remaining 20 — the ones where an error was being dropped despite having a caller that could handle it.LayerStack.renderreturned a truncated frameAn out-of-memory frame reached the screen as an empty or half-drawn one, indistinguishable from a compositing bug.
renderis fallible now.Image commands dropped real I/O failures
.cache_imageand.delete_imageswallowed everything, while.image_fileimmediately below them propagated. An unsupported protocol already returnsfalserather than erroring, so what was being discarded was genuine I/O failure — and acache_imagethat quietly failed left the followingplace_cached_imagedrawing nothing, with no signal anywhere.An append that cannot fail
dev_consoledidclearRetainingCapacity()thenappendSlice(keep.items) catch {}.keepis a subset of what was just cleared, so the capacity is already there.appendSliceAssumeCapacitystates the invariant instead of hiding behind a catch.The rest are best-effort, and now say so
Terminal cleanup and the suspend path run while already unwinding, with nobody left to report to — a broken pipe must not stop the remaining modes from being reset. Those keep their
catch {}and gain a comment explaining why, so the next reader does not have to work it out again.Tests
A failing-allocator test walks the failure index across
LayerStack.renderand asserts the result is always either a whole frame or an error — never something in between.zig build testandzig buildclean on 0.16.0.