Defer error serialization #5869
Merged
Conversation
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
In unisonweb#5661, exceptions were made much prettier, but also accidentally double-prettified in cases where they were written to then read back off the stack. This reverts that double-prettification. Fixes unisonweb#5864.
This preserves structured errors (`CompileExn`, `DecompError`, `RuntimeExn`, and `RuntimePanic`) until much closer to `main`, allowing rendering decisions to be made with more information. For example, pulling issue titles from GitHub is no longer done when running transcripts (to avoid network-induced nondeterminism, reflected in fix-4463.md). Some of the specific changes that were made: A new type, `Unison.Runtime.Error`, wraps `CompileExn`, `RuntimeExn`, and `RuntimePanic`, because they travel together. `Unison.Codebase.Runtime.Error` is gone. It was the result of serializing various error types, and is now represented by either `Unison.Runtime.Error` or `DecompError`, depending on the site. Error serialization has been moved to `Unison.Runtime.Interface`. Ideally, it would be moved out of unison-runtime completely and into unison-cli, but it is still used in tests, debugging, etc. `Unison.Codebase.Editor.Output.EvaluationFailure` now takes a `Unison.Runtime.Error` and a separate function to apply to the prettified result, rather than expecting the failure to be prettified already. This also indents the output for `test` failures (as seen in name-test-sandbox-violators.md).
aryairani
approved these changes
Sep 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Overview
This preserves structured errors (
CompileExn,DecompError,RuntimeExn, andRuntimePanic) until much closer tomain, allowing rendering decisions to be made with more information. For example, pulling issue titles from GitHub is no longer done when running transcripts (to avoid network-induced nondeterminism, reflected in fix-4463.md).Implementation notes
A new type,
Unison.Runtime.Error, wrapsCompileExn,RuntimeExn, andRuntimePanic, because they travel together.Unison.Codebase.Runtime.Erroris gone. It was the result of serializing various error types, and is now represented by eitherUnison.Runtime.ErrororDecompError, depending on the site.Error serialization has been moved to
Unison.Runtime.Interface. Ideally, it would be moved out of unison-runtime completely and into unison-cli, but it is still used in tests, debugging, etc.Unison.Codebase.Editor.Output.EvaluationFailurenow takes aUnison.Runtime.Errorand a separate function to apply to the prettified result, rather than expecting the failure to be prettified already.This also indents the output for
testfailures (as seen in name-test-sandbox-violators.md).Test coverage
This is covered by existing transcripts. Two of the test outputs change (as called out above), and the rest not changing shows that this is mostly a refactor.
Loose ends
This is based on #5868, so that should be merged first. Until it’s merged, this PR’s diff includes the changes from that PR, but that one is pretty small, so it shouldn’t block review.