Reraise unexpected spectra crashes instead of masking them - #43
Merged
Merged
Conversation
encode/4-5, decode/4-5, and schema/3-4 rescue `error in ErlangError`
to translate a few known spectra configuration errors into
ArgumentError. That rescue clause also binds every other exception
Elixir normalizes a raw BEAM error into (e.g. %BadMapError{},
%KeyError{}), not just literal %ErlangError{} structs.
handle_erlang_error/4 only pattern-matched on %ErlangError{}, so any
other crash from spectra failed inside Spectral's own handler with a
misleading FunctionClauseError, discarding the original exception and
stacktrace. It also used `raise` rather than `reraise`, which would
have dropped the original stacktrace even for a recognized
%ErlangError{} it didn't have a specific translation for.
handle_erlang_error/4 now takes the rescue's __STACKTRACE__ and
reraises anything that isn't one of the known configuration errors,
preserving the original exception and where it happened.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WK6VcSwrRa5XLKUrEhEi2T
andreashasse
commented
Sep 14, 2026
Keeps the original %ErlangError{original: original} = error head and
its case original do ... end body untouched (content-wise) rather than
moving the {:module_types_not_found, ...} etc. patterns into a case on
the bare error. A second function clause now catches everything that
isn't %ErlangError{} at all and reraises it, which is the only new
behavior actually needed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WK6VcSwrRa5XLKUrEhEi2T
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Add schema-path coverage and an unknown-ErlangError regression case.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This pull request updates Spectral exception handling so unexpected codec crashes preserve their original exceptions and stacktraces.
Changes:
- Captures rescue stacktraces and uses
reraise. - Adds crashing codec regression tests.
- Documents the fix in the changelog.
File summaries
| File | Summary |
|---|---|
test/support/crashing_codec.ex |
Adds a deliberately crashing codec fixture. |
test/spectral_unexpected_crash_test.exs |
Tests unexpected encode/decode crash propagation. |
lib/spectral.ex |
Preserves and reraises unexpected errors. |
CHANGELOG.md |
Documents the fix. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Addresses Copilot review feedback: the existing regression tests only
covered encode/decode, and only the non-%ErlangError{} fallback clause
(via %KeyError{}). Neither schema/2 nor schema/4 were exercised, and
the other fallback -- an %ErlangError{} whose original doesn't match
any of the four known configuration errors -- was untested, so the
raise-to-reraise fix for that branch had no coverage.
CrashingCodec gains a schema/5 clause for `t` (same %KeyError{} crash
as encode/decode) and a new `t2` type whose encode/decode raise a raw
BEAM error Elixir has no specific exception for, producing a genuine
%ErlangError{} with an unrecognized original.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WK6VcSwrRa5XLKUrEhEi2T
1.20.0-rc.4/28.3 was a pre-release pin from before 1.20 shipped. Elixir 1.20's changelog states it requires OTP 27+ and is compatible with OTP 29, so bump both. Also rewrites CrashingCodec's KeyError crash as an explicit `raise` instead of `Map.fetch!(Map.new(), :missing_key)`: Elixir 1.20's type system now statically proves that call always raises and reports it as a type warning, which fails `MIX_ENV=test mix compile --warnings-as-errors` on the new matrix entry. Verified the full suite, credo, dialyzer, ex_dna, and format-check pass on both Elixir 1.20.4/OTP 29.0.6 and the existing Elixir 1.17/OTP 27 baseline. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WK6VcSwrRa5XLKUrEhEi2T
andreashasse
pushed a commit
that referenced
this pull request
Sep 14, 2026
Merges in the unexpected-crash reraise fix (#43), which landed on main after this branch. Resolves the handle_erlang_error/5 conflict by keeping both the new fallback clause and the existing describe_type helper. Also bumps spectra to 0.14.1 (still satisfied by the existing ~> 0.14.0 constraint) and adds a regression test plus a CHANGELOG entry for its fix: encoding a struct with non-map data crashed with a raw badmap error instead of returning {:error, [...]}. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTMXez4gy2fHp6HgovtfGG
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.
Summary
encode/4-5,decode/4-5, andschema/3-4inlib/spectral.exrescueerror in ErlangErrorto translate a few known spectra configuration errors (module/type not found, unsupported type) intoArgumentError.ErlangErrorspecially: Elixir also binds it to every other exception it normalizes a raw BEAM error into (e.g.%BadMapError{},%KeyError{}), not only literal%ErlangError{}structs — confirmed empirically, not just by reading the docs.handle_erlang_error/4only pattern-matched on%ErlangError{original: original} = error, so any other crash from spectra failed the function clause and surfaced as a misleadingFunctionClauseErrorpointing atSpectral.handle_erlang_error/4, discarding the original exception and stacktrace. Its_other -> raise errorfallback, meant to re-raise unrecognized errors, could therefore never run for these — and usedraiserather thanreraisebesides, which rebuilds the stacktrace at theraisecall site instead of preserving the original.Fix
__STACKTRACE__in eachrescueclause and pass it intohandle_erlang_error/4.handle_erlang_error/4's parameter to accept any exception, matching the four known%ErlangError{original: ...}shapes explicitly, andreraise error, stacktracefor everything else — both unrecognized%ErlangError{}originals and every other normalized exception.Test plan
test/support/crashing_codec.ex, aSpectral.Codecwhoseencode/decodedeliberately raise aKeyError(spectra invokes codecs directly with no try/catch, so this reachesSpectral.encode/decodeunchanged).test/spectral_unexpected_crash_test.exsasserting the original%KeyError{}is reraised unchanged (notFunctionClauseError), with its original stacktrace pointing into the codec.FunctionClauseErrorwhen the fix is reverted, and pass with it restored — both against the realspectradependency (temporarily pointed at a local, separately-fixed checkout for testing) and against the currently publishedspectra ~> 0.13.1on Hex, since this fix doesn't depend on the spectra-side bug at all.mix test(20 doctests, 3 properties, 219 tests, 0 failures),mix credo --strict,mix dialyzer,mix ex_dna(duplication),mix format --check-formatted, andmix compile --warnings-as-errors.🤖 Generated with Claude Code
https://claude.ai/code/session_01WK6VcSwrRa5XLKUrEhEi2T
Generated by Claude Code