feat: add alerting for unreachable errors by simone-stacks · Pull Request #7046 · stacks-network/stacks-core

simone-stacks · 2026-03-26T18:54:47Z

Description

Adds distinct alerting when "unreachable" errors are triggered. These are errors that should never occur in valid execution and indicate potential bugs.

Today these errors flow through the generic "problematic transaction" path and get logged at info! level with no way to distinguish them from normal rejectable errors.

This PR adds:

is_unreachable() methods on RuntimeCheckErrorKind, StaticCheckErrorKind, and ParseError
error! level logs with a UNREACHABLE_ERROR_TRIGGERED marker and structured event_name=unreachable_error field at all consensus rejection sites (block validation + miner)
a stacks_unreachable_errors_total Prometheus counter (behind monitoring_prom feature flag)

There's still an open discussion about whether some StaticCheckErrorKind variants documented as internal errors (TypeAlreadyAnnotatedFailure, CheckerImplementationFailure ecc) should also be alerted on, see stx-labs/core-epics#119 (comment). Keeping those out for now.

Applicable issues

fixes stx-labs/core-epics#160

Checklist

Test coverage for new or modified code paths
For new Clarity features or consensus changes, add property tests (see docs/property-testing.md)
Changelog is updated
Required documentation changes (e.g., rpc/openapi.yaml for RPC endpoints, event-dispatcher.md for new events)
New clarity functions have corresponding PR in clarity-benchmarking repo

…rrors

…eachable

coveralls · 2026-03-27T12:51:04Z

Coverage Report for CI Build 24078323220

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage decreased (-8.3%) to 77.398%

Details

Coverage decreased (-8.3%) from the base build.
Patch coverage: 8 uncovered changes across 1 file (134 of 142 lines covered, 94.37%).
19497 coverage regressions across 183 files.

Uncovered Changes

File	Changed	Covered	%
stackslib/src/chainstate/stacks/db/transactions.rs	122	114	93.44%

Coverage Regressions

19497 previously-covered lines in 183 files lost coverage.

Top 10 Files by Coverage Loss	Lines Losing Coverage	Coverage
stacks-node/src/neon_node.rs	2537	4.48%
stacks-node/src/burnchains/bitcoin_regtest_controller.rs	1946	24.22%
stacks-signer/src/v0/signer.rs	1249	0.0%
stacks-node/src/nakamoto_node/relayer.rs	1037	15.13%
stacks-node/src/nakamoto_node/miner.rs	1012	8.62%
stacks-node/src/run_loop/neon.rs	556	0.94%
stacks-signer/src/v0/signer_state.rs	510	34.32%
stacks-node/src/event_dispatcher.rs	468	33.41%
stackslib/src/net/p2p.rs	389	65.4%
stacks-node/src/run_loop/nakamoto.rs	386	0.0%

Coverage Stats


Relevant Lines:	217854
Covered Lines:	168614
Line Coverage:	77.4%
Coverage Strength:	7582881.09 hits per line

💛 - Coveralls

clarity-types/src/errors/ast.rs

… miner

…ock validation

stackslib/src/chainstate/stacks/db/transactions.rs

federico-stacks · 2026-04-03T10:16:08Z

clarity/src/vm/analysis/errors.rs

+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn runtime_check_unreachable_returns_true() {
+        let err = RuntimeCheckErrorKind::Unreachable("test bug".into());
+        assert!(err.is_unreachable());
+    }
+
+    #[test]
+    fn runtime_check_cost_overflow_not_unreachable() {
+        assert!(!RuntimeCheckErrorKind::CostOverflow.is_unreachable());
+    }
+
+    #[test]
+    fn runtime_check_type_error_not_unreachable() {
+        let err = RuntimeCheckErrorKind::TypeError(
+            Box::new(TypeSignature::IntType),
+            Box::new(TypeSignature::UIntType),
+        );
+        assert!(!err.is_unreachable());
+    }
+
+    #[test]
+    fn runtime_check_value_too_large_not_unreachable() {
+        assert!(!RuntimeCheckErrorKind::ValueTooLarge.is_unreachable());
+    }
+
+    #[test]
+    fn static_check_unreachable_returns_true() {
+        let err = StaticCheckErrorKind::Unreachable("test bug".into());
+        assert!(err.is_unreachable());
+    }
+
+    #[test]
+    fn static_check_supertype_too_large_not_unreachable() {
+        assert!(!StaticCheckErrorKind::SupertypeTooLarge.is_unreachable());
+    }
+
+    #[test]
+    fn static_check_cost_overflow_not_unreachable() {
+        assert!(!StaticCheckErrorKind::CostOverflow.is_unreachable());
+    }
+
+    #[test]
+    fn static_check_type_already_annotated_not_unreachable() {
+        assert!(!StaticCheckErrorKind::TypeAlreadyAnnotatedFailure.is_unreachable());
+    }
+}


I have a few suggestions that might help:

for consistency in test structure (at least it seems for vm/analysis), it may be worth moving these into src/vm/analysis/tests/errors.rs

tests could be collapsed per error type, e.g. static_check_is_unreachable() and runtime_check_is_unreachable()

At the moment, the tests are not exhaustive, as they don’t cover all variants. I see a few possible approaches here:

Make them fully exhaustive by matching all variants per type (though this could become quite heavy and maybe not worthy in this case).

Keep the pattern matching focused on the “unreachable” variants, and use a catch-all _ arm for the rest.

Alternatively, we may consider to drop them.

crc: 4fca9f1

I see that point 1 and point 2 have been addressed, but not point 3. I would like to know your thoughts about it :)

You're right, I missed the exhaustiveness concern, crc is in 68e514f, I dropped them entirely. is_unreachable() is a trivial matches! one-liner, these were just testing Rust pattern matching. removed from both analysis and ast. What do you think?

IMHO, the point here isn’t the triviality of the code, but rather that we’re not using is_unreachable() as part of the business logic (as stated in the docstring).

I’m fine with this. Let’s keep this open for now to gather feedback from other reviewers.

clarity/src/vm/ast/errors.rs

brice-stacks

LGTM!

federico-stacks

Implementation looks good overall.

I’ve left a specific note in this thread: #7046 (comment)

One additional thought: since there are other error variants that might be considered unreachable (as mentioned in the PR description), it could be worth including them in our monitoring as well. More in general we could collect from the consensus tests documentation all the error variants currently treated as unreachable and track them accordingly. (This could also be done in a follow-up PR)

Any thoughts?

simone-stacks · 2026-04-13T13:48:08Z

Yeah i think it's a great follow-up. The strongest candidates are TypeAlreadyAnnotatedFailure and CheckerImplementationFailure: they're documented as "indicating a bug" and their error message says "internal error please file an issue"

There's also VmInternalError::Expect and InvariantViolation in vm/errors.rs which are documented as "believed to be unreachable", but those flow through the Acceptable path in handle_clarity_runtime_error so they silently eat fees without any alerting today

There's also epoch-gated stuff (SupertypeTooLarge, ExpressionStackDepthTooDeep) but I believe the situation is different here since those are expected consensus behavior and not bugs

I'd tackle it in a follow-up PR, focusing on the first two categories. does that make sense? anything else I might be missing?

federico-stacks · 2026-04-13T16:26:18Z

I'd tackle it in a follow-up PR, focusing on the first two categories. does that make sense? anything else I might be missing?

we could consider all the unreachable variants from the consensus tests (the one under stackslib src/chainstate/tests/)

federico-stacks

LGTM!

heads-up: a lot of CI checks are failed. Should not be related, but worth to have a clean run before merge this

simone-stacks added 3 commits March 26, 2026 15:11

feat: add prometheus counter and logging for unreachable Clarity VM e…

7fa3892

…rrors

feat: make ParseErrorKind::UnexpectedParserFailure rejectable and unr…

80155fd

…eachable

chore: add changelog fragments

58e0ea7

federico-stacks reviewed Mar 27, 2026

View reviewed changes

clarity-types/src/errors/ast.rs Outdated Show resolved Hide resolved

simone-stacks added 4 commits March 30, 2026 13:34

fix: move parse/static unreachable alerting to reachable code path in…

8bcf155

… miner

crc: remove UnexpectedParserFailure from rejectable_in_epoch

6a79ef3

fix: move unreachable alerting before rejectable_in_epoch guard in bl…

6ee9e0e

…ock validation

Merge branch 'develop' into feat/unreachable-errors-alert

ae6a02a

simone-stacks marked this pull request as ready for review March 31, 2026 19:58

fix: eliminate double-counting of unreachable error alerts in miner path

11f9e09

simone-stacks force-pushed the feat/unreachable-errors-alert branch from c33a7c7 to 11f9e09 Compare April 1, 2026 18:42

simone-stacks requested review from benjamin-stacks, brice-stacks and federico-stacks April 1, 2026 18:44

federico-stacks reviewed Apr 3, 2026

View reviewed changes

crc: deduplicate unreachable error logging into shared function

4fca9f1

simone-stacks requested a review from federico-stacks April 7, 2026 11:10

brice-stacks previously approved these changes Apr 7, 2026

View reviewed changes

federico-stacks reviewed Apr 13, 2026

View reviewed changes

crc: remove is_unreachable tests from analysis and ast error modules

68e514f

simone-stacks dismissed brice-stacks’s stale review via 68e514f April 13, 2026 13:21

chore: clarify is_unreachable is monitoring-only

c3aa896

federico-stacks reviewed Apr 13, 2026

View reviewed changes

Conversation

simone-stacks commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Applicable issues

Checklist

Uh oh!

coveralls commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 24078323220

Coverage decreased (-8.3%) to 77.398%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

federico-stacks Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simone-stacks Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

federico-stacks Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

simone-stacks Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

federico-stacks Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brice-stacks left a comment

Choose a reason for hiding this comment

Uh oh!

federico-stacks left a comment

Choose a reason for hiding this comment

Uh oh!

simone-stacks commented Apr 13, 2026

Uh oh!

federico-stacks commented Apr 13, 2026

Uh oh!

federico-stacks left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

simone-stacks commented Mar 26, 2026 •

edited

Loading

coveralls commented Mar 27, 2026 •

edited

Loading

federico-stacks Apr 3, 2026 •

edited

Loading