refactor: Fix clang-tidy `bugprone-empty-catch` check by godexsoft · Pull Request #6419 · XRPLF/rippled

godexsoft · 2026-02-25T14:29:20Z

High Level Overview of Change

Fixes instances of bugprone-empty-catch of clang-tidy.

Context of Change

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactor (non-breaking change that only restructures code)
Performance (increase or change in throughput and/or latency)
Tests (you added tests for code that already exists, or your new feature included in this PR)
Documentation update
Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
Release

API Impact

No impact.

…enerated files

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

codecov · 2026-02-25T14:54:44Z

Codecov Report

❌ Patch coverage is 20.00000% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.8%. Comparing base (1a7f824) to head (e2b26bc).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/libxrpl/protocol/STTx.cpp	0.0%	2 Missing ⚠️
src/xrpld/app/misc/detail/ValidatorSite.cpp	0.0%	2 Missing ⚠️
src/libxrpl/beast/insight/StatsDCollector.cpp	0.0%	1 Missing ⚠️
src/libxrpl/nodestore/backend/NuDBFactory.cpp	0.0%	1 Missing ⚠️
src/libxrpl/tx/transactors/XChainBridge.cpp	0.0%	1 Missing ⚠️
src/xrpld/app/ledger/detail/InboundLedgers.cpp	0.0%	1 Missing ⚠️
src/xrpld/app/ledger/detail/LedgerMaster.cpp	0.0%	1 Missing ⚠️
src/xrpld/app/main/GRPCServer.cpp	0.0%	1 Missing ⚠️
src/xrpld/overlay/detail/ConnectAttempt.cpp	0.0%	1 Missing ⚠️
src/xrpld/rpc/detail/RPCCall.cpp	0.0%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #6419     +/-   ##
=========================================
- Coverage     79.8%   79.8%   -0.0%     
=========================================
  Files          858     858             
  Lines        67757   67757             
  Branches      7562    7554      -8     
=========================================
- Hits         54081   54073      -8     
- Misses       13676   13684      +8

Files with missing lines	Coverage Δ
src/libxrpl/protocol/STAmount.cpp	`88.3% <100.0%> (ø)`
src/xrpld/app/ledger/detail/SkipListAcquire.cpp	`95.9% <100.0%> (ø)`
src/libxrpl/beast/insight/StatsDCollector.cpp	`0.0% <0.0%> (ø)`
src/libxrpl/nodestore/backend/NuDBFactory.cpp	`64.3% <0.0%> (ø)`
src/libxrpl/tx/transactors/XChainBridge.cpp	`91.2% <0.0%> (ø)`
src/xrpld/app/ledger/detail/InboundLedgers.cpp	`36.4% <0.0%> (ø)`
src/xrpld/app/ledger/detail/LedgerMaster.cpp	`42.7% <0.0%> (ø)`
src/xrpld/app/main/GRPCServer.cpp	`42.2% <0.0%> (ø)`
src/xrpld/overlay/detail/ConnectAttempt.cpp	`0.0% <0.0%> (ø)`
src/xrpld/rpc/detail/RPCCall.cpp	`93.6% <0.0%> (ø)`
... and 2 more

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This PR addresses clang-tidy's bugprone-empty-catch check by adding suppressions for intentional empty catch blocks and refactoring code to move return statements into catch blocks. The PR enables the check in .clang-tidy configuration and adds the related suppressions (NOLINT and NOLINTNEXTLINE) to the spell-checker dictionary.

Changes:

Enabled bugprone-empty-catch check in .clang-tidy configuration
Added NOLINT suppressions to 16 intentional empty catch blocks across production and test code
Refactored 5 functions to move return statements from after catch blocks into the catch blocks themselves
Added NOLINT and NOLINTNEXTLINE to cspell.config.yaml

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`.clang-tidy`	Enabled `bugprone-empty-catch` check and moved it from disabled list to enabled list
`cspell.config.yaml`	Added NOLINT and NOLINTNEXTLINE to dictionary for spell-checking
`src/xrpld/rpc/detail/RPCCall.cpp`	Added NOLINT suppression for intentional empty catch in config file handling
`src/xrpld/overlay/detail/ConnectAttempt.cpp`	Added NOLINT suppression for timer cancel error handling
`src/xrpld/app/misc/detail/ValidatorSite.cpp`	Added NOLINT suppressions in 2 places for timer cancel error handling
`src/xrpld/app/main/GRPCServer.cpp`	Added NOLINT suppression for endpoint parsing error handling
`src/xrpld/app/ledger/detail/SkipListAcquire.cpp`	Added NOLINT suppression for data processing error handling
`src/xrpld/app/ledger/detail/LedgerMaster.cpp`	Added NOLINT suppression for missing node error handling
`src/xrpld/app/ledger/detail/InboundLedgers.cpp`	Added NOLINT suppression for node data error handling
`src/libxrpl/nodestore/backend/NuDBFactory.cpp`	Added NOLINT suppression in destructor for close error handling
`src/libxrpl/beast/insight/StatsDCollector.cpp`	Added NOLINT suppression for timer cancel error handling
`src/tests/libxrpl/basics/scope.cpp`	Added NOLINT suppressions in 6 test cases for scope guard testing
`src/test/jtx/impl/WSClient.cpp`	Added NOLINTNEXTLINE suppression for stream cancel error handling
`src/test/jtx/impl/Oracle.cpp`	Moved return statement into catch block for validation function
`src/test/jtx/impl/Env.cpp`	Moved return statements into catch blocks in 2 functions and changed exception type from `std::exception const&` to `...`
`src/test/core/SociDB_test.cpp`	Added NOLINT suppressions in setup and cleanup methods
`src/test/app/Manifest_test.cpp`	Added NOLINT suppressions in setup and cleanup methods
`src/libxrpl/tx/transactors/XChainBridge.cpp`	Moved return statement into existing catch(...) block
`src/libxrpl/protocol/STTx.cpp`	Moved return statement into catch block and changed exception type from `std::exception const&` to `...`
`src/libxrpl/protocol/STAmount.cpp`	Moved return statement into catch block, changed exception type from `std::exception const&` to `...`, and added blank line for readability

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/test/jtx/impl/Env.cpp

src/libxrpl/protocol/STTx.cpp

src/libxrpl/protocol/STAmount.cpp

kuznetsss · 2026-02-27T16:42:21Z

Probably, it is better to add a NOLINT comment instead of changing catch(some_exception) to catch(...) to not change the code behaviour. Other than that looks good.

godexsoft · 2026-02-27T16:54:09Z

Probably, it is better to add a NOLINT comment instead of changing catch(some_exception) to catch(...) to not change the code behaviour. Other than that looks good.

Copilot makes it sound very scary but in reality i think it's the same thing (yes technically it could change behaviour but i doubt the code throws anything that isn't derived from std::exception.
There are only a handful of places so perhaps we can ask @ximinez to take a look and decide. It's easy to add NOLINT if that's the conclusion.

ximinez

All the places that you converted to catch (...) certainly look to me like they were intended as a "catch everything", and only limited it to std::exception because that's what's supposed to be thrown.

ximinez · 2026-02-27T19:40:02Z

src/test/app/Manifest_test.cpp

    Manifest_test()
    {
        try
        {
            setupDatabaseDir(getDatabasePath());
        }
-        catch (std::exception const&)
+        catch (std::exception const&)  // NOLINT(bugprone-empty-catch)
        {
        }
    }
    ~Manifest_test()
    {
        try
        {
            cleanupDatabaseDir(getDatabasePath());
        }
-        catch (std::exception const&)
+        catch (std::exception const&)  // NOLINT(bugprone-empty-catch)
        {
        }
    }


Probably beyond the scope of this PR, but this is an usual (though not unique) pattern. Usually everything meaningful is done in run() (or in the one(?) test that actually puts anything in that folder).

We should review all our exception use in the codebase some day.

This reverts commit 58e429f.

Copilot

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 10 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/libxrpl/protocol/STAmount.cpp

src/test/jtx/impl/Env.cpp

src/xrpld/app/misc/detail/ValidatorSite.cpp

src/xrpld/app/main/GRPCServer.cpp

src/xrpld/app/ledger/detail/InboundLedgers.cpp

src/xrpld/app/ledger/detail/SkipListAcquire.cpp

src/test/jtx/impl/Env.cpp

src/test/core/SociDB_test.cpp

src/test/app/Manifest_test.cpp

kuznetsss

👍

godexsoft and others added 7 commits February 24, 2026 14:53

Enable bugprone checks that have no issues atm

a92331d

Enable all checks that work already

f9b392e

Enable clang-tidy in CI if config changed

b5c8747

Add documentation about running clang-tidy and don't check protobuf/g…

ce5cea9

…enerated files

Update .github/workflows/reusable-clang-tidy.yml

cdb8add

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Fix CI logic

17f87a6

Fix instances of bugprone-empty-catch

93ef6ae

godexsoft requested review from Copilot, kuznetsss and mathbunnyru and removed request for Copilot February 25, 2026 14:29

Copilot started reviewing on behalf of godexsoft February 25, 2026 14:30 View session

Merge develop and fix issues

05155dc

Copilot AI review requested due to automatic review settings February 27, 2026 16:10

Copilot started reviewing on behalf of godexsoft February 27, 2026 16:11 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

src/test/jtx/impl/Env.cpp Show resolved Hide resolved

src/test/jtx/impl/Env.cpp Show resolved Hide resolved

src/libxrpl/protocol/STTx.cpp Show resolved Hide resolved

src/libxrpl/protocol/STAmount.cpp Show resolved Hide resolved

godexsoft requested a review from ximinez February 27, 2026 16:54

ximinez approved these changes Feb 27, 2026

View reviewed changes

godexsoft added 2 commits March 2, 2026 15:30

Fix some review comments

58e429f

Revert "Fix some review comments"

bbc7952

This reverts commit 58e429f.

Copilot AI review requested due to automatic review settings March 2, 2026 15:46

Copilot started reviewing on behalf of godexsoft March 2, 2026 15:46 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

godexsoft added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Mar 2, 2026

kuznetsss approved these changes Mar 2, 2026

View reviewed changes

Merge branch 'develop' into chore/clang-tidy-checks1

e2b26bc

bthomee changed the title ~~chore: Fix clang-tidy bugprone-empty-catch check~~ refactor: Fix clang-tidy bugprone-empty-catch check Mar 2, 2026

bthomee enabled auto-merge (squash) March 2, 2026 16:48

bthomee merged commit afc660a into XRPLF:develop Mar 2, 2026
27 checks passed

Conversation

godexsoft commented Feb 25, 2026 • edited by bthomee Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High Level Overview of Change

Context of Change

Type of Change

API Impact

Uh oh!

codecov bot commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kuznetsss commented Feb 27, 2026

Uh oh!

godexsoft commented Feb 27, 2026

Uh oh!

ximinez left a comment

Choose a reason for hiding this comment

Uh oh!

ximinez Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

godexsoft Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kuznetsss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

godexsoft commented Feb 25, 2026 •

edited by bthomee

Loading

codecov bot commented Feb 25, 2026 •

edited

Loading