#36101 doc: Add an error handling strategy
https://github.com/bitcoin/bitcoin/pull/36101 · · +124/-0 in 1 files, 1 commits · labels: Brainstorming, Docs
Goal
- Establish a project-wide error handling strategy for Bitcoin Core developers
- Resolve recurring design disputes over return values, exceptions, and process crashes
This pull request adds `doc/error-handling.md`, proposing an architectural error-handling strategy for Bitcoin Core. It formalizes a three-tier model: return values represent successful domain outcomes, exceptions represent external runtime errors, and defects (bugs and invariant violations) trigger fail-fast termination. It explicitly forbids new code from introducing `std::expected` or similar return-value mechanisms for propagating errors.
Problem: Bitcoin Core lacks a unified error-handling strategy, leading to repeated ad-hoc debates across validation and subsystem PRs over when to use exceptions, return types like `util::Expected`, or process aborts.
Category: Documentation (#1 of 9)
P2 · decision needed
- P2 because it addresses a recurring architectural dispute affecting multiple active PRs
- An agreed policy unblocks stalled design discussions across validation, I/O, and database code
Establishing an architectural strategy for error handling is strategically important to resolve recurring debates across open validation and P2P refactors. The PR provides significant leverage by attempting to define project-wide standards for distinguishing consensus validation results, operational errors, and software defects.
Membership: Adds a new architectural design document under doc/error-handling.md.
Factors: security/stability 1, bug 0, performance 0, user value 1, leverage 2
Reviewability: Ready
- Ready for review
The proposal is complete as a design document and actively being debated. There are no technical blockers to review.
Author status: Active; engaging in architectural debates and proposing clarifying amendments to address concerns.
Open concerns:
- Mandating C++ exceptions contradicts recent project adoption of explicit return types like `util::Expected` and `util::Result` (ajtowns, willcl-ark).
- Exceptions introduce invisible, non-local control flow and increase code-review cognitive load, particularly around thread safety and shutdown (l0rinc, maflcko).
- Abrupt termination on defects without graceful flushing endangers wallet consistency and funds safety (furszy).
- A uniform termination policy does not fit modules like P2P and RPC, where remotely triggered assert defects should not crash the entire node (mzumsande).
- The document belongs in `doc/developer-notes.md` rather than a standalone top-level doc file (maflcko).
Agreement: Blocked
- Concept approval for establishing clear error handling guidelines (sedited, josibake, willcl-ark)
- Strong objection to preferring exceptions over functional return values (ajtowns, l0rinc)
- Concern that unconditional aborts risk wallet corruption and remote p2p crashes (furszy, mzumsande)
- Concern that exceptions increase review burden and multithread safety risks (maflcko)
Disputed: ajtowns and l0rinc NACK the mandate for exceptions; furszy and mzumsande object to global defect aborts.
There is clear concept support for documenting an error handling strategy, but the specific technical recommendations (banning Expected in favor of exceptions, and global abort on defect) face active NACKs and serious objections from multiple long-time contributors.
- ajtowns: 'NACK, this isn't a problem that needs a 750 word essay to address...'
- l0rinc: 'Approach NACK...'
- furszy: 'abrupt terminations can cause the wallet to enter an inconsistent/unrecoverable state'
- mzumsande: 'I see no reason why we would regress on these pragmatic, domain-specific solutions in favor of a global policy'
- josibake: 'Concept ACK: I strongly agree that an established error handling strategy is essential for a project of this size.'
Objections:
| Reviewer | Kind | Harm | Status | Blocking | Author replied | Quote |
|---|---|---|---|---|---|---|
| ajtowns | approach | Contradicts established codebase patterns like util::expected.h, introducing technical debt and worse error handling semantics. | open | yes | no | 2026-08-28: 'NACK, this isn't a problem that needs a 750 word essay to address... and the essay provided doesn't reflect existing practice (eg util/expected.h or util/result.h)' |
| l0rinc | approach | Exceptions create invisible non-local control flow and unpredictable side effects contrary to recent modern C++ practices. | open | yes | yes | 2026-08-27: 'Approach NACK, instead of presenting your personal opinion as the only way forward, try gathering info first on the project's needs' |
| furszy | safety | Ungraceful process aborts on defects prevent graceful flushes and leave wallet databases in unrecoverable states, endangering funds. | open | yes | yes | 2026-08-27: 'There are multiple examples where abrupt terminations can cause the wallet to enter an inconsistent/unrecoverable state, which puts users' funds at risk.' |
| mzumsande | safety | A global abort-on-defect directive breaks existing resilience patterns where non-consensus modules like P2P or RPC can handle bugs without node crashes. | open | yes | yes | 2026-08-28: 'in p2p code that is triggerable remotely it is often best practice to add a debug-only Assume() and just abort the processing of the message, but not the entire program' |
| maflcko | maintenance | Broad exception propagation forces reviewers to audit all calling threads and destructors for deadlock hazards during shutdown. | open | no | yes | 2026-08-28: 'If exceptions were used more broadly, reviewers must always implicitly consider all calling threads for all functions and waste review cycles on that instead of focussing on the logic itself.' |
| maflcko | approach | Broader exception usage risks multi-threaded shutdown deadlocks or memory corruption, adds review overhead, creates contradictory docs, and is in the wrong location | open | yes | yes | 2026-08-29: 'Again, as mentioned above, I am not against this change, but it can absolutely not be merged in its current form, while ignoring the review feedback above. Merging it would create internally contradictory documentation. Also, doc/error-handling.md is the wrong place to put the docs.' |
Support:
- josibake: Having a central error handling strategy prevents fragmented recurring arguments across different PRs.
- sedited: Agrees with the core distinction between domain results and errors and the use of exceptions.
- willcl-ark: Supports establishing a well-defined approach to error handling across the project.
Participants: sedited (support), l0rinc (objection), furszy (objection), ajtowns (objection), maflcko (objection), josibake (support), willcl-ark (support), mzumsande (objection)
State derived from the lists: blocking objection open with no author reply (ajtowns) (model's own read: Disputed)
Review verdicts (DrahtBot): 0 -2
- Concept ACK: sedited, josibake, willcl-ark
- Approach NACK: l0rinc
- Concept NACK: ajtowns
Dependencies
Files
0 lines under test/bench/ci.
- doc/error-handling.md +124/-0
Card
This pull request introduces doc/error-handling.md, proposing a formal project-wide error handling architecture that categorizes program behavior into results, errors (propagated via exceptions), and defects (handled via fail-fast termination). The problem it addresses is the lack of a consistent error-handling convention, which currently fuels recurring disagreements in validation and I/O pull requests. Multiple reviewers support establishing clear guidelines, but the specific proposal faces strong pushback and NACKs from maintainers and contributors over mandating exceptions over std::expected and requiring process aborts for defects. The PR is Ready for review as a design document but is heavily Disputed on its core approach.