combinators: catch the error TYPE, not a list of its variants; gate CI on nightly - #25
Open
bobzhang wants to merge 2 commits into
Open
combinators: catch the error TYPE, not a list of its variants; gate CI on nightly#25bobzhang wants to merge 2 commits into
bobzhang wants to merge 2 commits into
Conversation
`attempt` and `retry` each spelled out all three `WorkflowError` variants
to say one thing: "a typed workflow failure". Three arms with identical
bodies, and a fourth variant would have needed a fourth arm in two files
— an enum that cannot grow without editing its own consumers. The
wildcard constructor pattern says it directly:
WorkflowError::_ as error => Err(error) // attempt
WorkflowError::_ as failure => !retriable(failure) // retry's fatal_error
The remaining variant matches stay as they are, because they DISCRIMINATE
rather than collapse: `worth_retrying` treats `Skipped` unlike the other
failures, and the `Show` impl renders each one differently.
The same pattern fixes a real hole in `hosted`'s sidecar write, which
caught `_` — every error, cancellation included. A run that was cancelled
mid-write went on walking, since the catch that exists to tolerate a full
disk also ate the unwind. `@os_error.OSError::_` names what "could not be
written" actually means and re-raises everything else, which is what
`spawn/contract.mbt` already does the long way round
(`error if @async.is_being_cancelled() => raise error`).
Where the compiler proves the type, no change was needed: `@json.parse`
and `@json.from_json` declare `raise ParseError` / `raise
JsonDecodeError`, so those `catch { _ => … }` arms are already as narrow
as the pattern would make them — the compiler rejects `@json.ParseError::_`
there, because the caught value is not an open `Error` to begin with. The
journal's line parser and viz's file read keep their wildcards on purpose:
both mean "any damage to this line/file is skipped", which spans utf8,
parse and decode errors, and narrowing would turn a torn tail into a crash.
Gates: `moon check --deny-warn` and `moon test` on native and wasm,
`moon fmt --check`, `moon info` (no interface change — `note` is private).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SRugKEyspcxgc7dFMqM7V2
The wildcard constructor pattern this module now catches with —
`WorkflowError::_`, `@os_error.OSError::_` — is nightly-only. Stable does
not merely warn about it; it cannot parse the binding at all:
combinators.mbt:94 WorkflowError::_ as failure => !retriable(failure)
The value identifier failure is unbound.
So the matrix flips. Nightly, which was advance warning of breakage, is
now the channel that must be green, and stable becomes the non-blocking
signal in the other direction: the day the stable jobs pass, the pattern
has landed there and the flag flips back. Both channels keep running on
both operating systems — dropping stable would throw away the only
notice of when this constraint lifts.
`publish.yml` installed stable and ran `moon check --deny-warn` as its
prepublish gate, which would now fail every release before reaching
`moon publish`; it installs nightly for the same reason CI does.
The requirement is written where someone hits it: the README's opening,
so a reader who clones and builds knows before the error, and AGENTS.md
next to the gates it changes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SRugKEyspcxgc7dFMqM7V2
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.
attemptandretryeach spelled out all threeWorkflowErrorvariants to say one thing: "a typed workflow failure". Three arms with identical bodies, and a fourth variant would have needed a fourth arm in two files — an enum that cannot grow without editing its own consumers. The wildcard constructor pattern says it directly:The remaining variant matches stay as they are, because they DISCRIMINATE rather than collapse:
worth_retryingtreatsSkippedunlike the other failures, and theShowimpl renders each one differently.The same pattern fixes a real hole in
hosted's sidecar write, which caught_— every error, cancellation included. A run cancelled mid-write went on walking, since the catch that exists to tolerate a full disk also ate the unwind.@os_error.OSError::_names what "could not be written" actually means and re-raises everything else, which is whatspawn/contract.mbtalready does the long way round (error if @async.is_being_cancelled() => raise error).Where no change was needed:
@json.parseand@json.from_jsondeclareraise ParseError/raise JsonDecodeError, so thosecatch { _ => … }arms are already as narrow as the pattern would make them — the compiler rejects@json.ParseError::_there, because the caught value is not an openErrorto begin with. The journal's line parser and viz's file read keep their wildcards on purpose: both mean "any damage to this line/file is skipped", spanning utf8, parse and decode errors, and narrowing would turn a torn tail into a crash.The toolchain consequence
XXError::_is nightly-only, and stable does not merely warn — it cannot parse the binding:So the CI matrix flips. Nightly, which was advance warning of breakage, is now the channel that must be green; stable becomes the non-blocking signal in the other direction — the day the stable jobs pass, the pattern has landed there and the flag flips back. Both channels keep running on both operating systems, because dropping stable would throw away the only notice of when this constraint lifts.
Observed on this PR, exactly as intended:
publish.ymlinstalled stable and ranmoon check --deny-warnas its prepublish gate, which would have failed every release before reachingmoon publish; it installs nightly for the same reason CI does. The requirement is stated in the README's opening and in AGENTS.md next to the gates it changes.🤖 Generated with Claude Code
https://claude.ai/code/session_01SRugKEyspcxgc7dFMqM7V2