refactor: Use error types from rpc-spec by godexsoft · Pull Request #3173 · XRPLF/clio

godexsoft · 2026-08-12T12:59:23Z

This is PR 3 out of many. It depends on merging #3171 first.

Moves the RPC error model into the shared spec library. src/rpc/Errors.hpp becomes a shim over <rpcspec/Errors.hpp>, keeping only the Clio-side rendering: makeError, getErrorInfo, getEtlErrorInfo.

ClioError::Etl* is carved out into a Clio-side etl::EtlError (new src/etl/Errors.hpp), since ETL codes aren't an RPC-spec concern. Source::forwardToRippled now returns EtlError instead of ClioError, which is what most of the diff is.

Wire-visible change: the "clio only serves validated data" warning now says "if you want to talk to xrpld" instead of "rippled" — the string moved into rpcspec, which renamed it. 10 expectations updated in RPCServerHandlerTests.cpp.

Drive-by cleanups:

Deleted InvalidParamsError and AccountNotFoundError - unreferenced anywhere in src/ or tests/.
Both error-info tables become std::to_array + ranges::find with a projection.

codecov · 2026-08-12T13:17:00Z

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/etl/impl/ForwardingSource.cpp	80.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

…al-specs-move-errors

mathbunnyru · 2026-08-26T17:06:11Z

+namespace etl {
+
+/** @brief Error codes produced by ETL source operations. Aliased from rpc::EtlError. */
+using EtlError = rpc::EtlError;


This makes the whole etl depend on rpcspec library, which doesn't make much sense.
I think you should make 2 cmake components in your rpc-spec, if you want to have EtlError as part of rpc-spec.
Clio's rpc layer will depend on rpc component, etl one will depend on etl

The whole idea here was to factor ETL errors out of the RPC layer.. but turns out they are not actual ETL errors, they are RPC errors from ETL source/forwarding code - customer facing. So the cleanest fix is to actually bring them back as they were and keep them in rpc-spec without trying to call them etl::EtlError.

Ok so after more research it looks like we can't really do this easily. ETL today is not just the part writing data but also the forwarding component that uses the "etl" error codes. I suggest we move them back into rpc-spec but call them "RpcForwarding[...]" instead. Then at a later stage we factor all forwarding code out of ETL module entirely. This will allow us to cleanly and honestly remove the dependency. Today though ETL will have to depend on rpc-spec simply because ETL is not just the writer part :/

mathbunnyru · 2026-08-26T17:08:35Z

 )

-target_link_libraries(clio_etl PUBLIC clio_data clio_util)
+target_link_libraries(clio_etl PUBLIC clio_data clio_util rpcspec::rpcspec)


This shouldn't be like so

Yeah i agree, ETL should not depend on RPC at all

Having that said, for now we need to keep this because ETL is not only the writer but also the forwarding code, which uses the rpc-spec errors for forwarding failures.

Copilot

Pull request overview

Refactors Clio’s RPC error handling to rely on the shared xrpl-rpc-spec error model, keeping Clio-specific rendering helpers and updating call sites/tests to use the spec types and renamed forwarding error codes.

Changes:

Replaced most direct usage of Clio-defined RPC error types with <rpcspec/Errors.hpp> and updated forwarding-related errors from Etl* to RpcForwarding*.
Updated the wire-visible “validated data” warning text to reference xrpld instead of rippled, and adjusted unit tests accordingly.
Bumped xrpl-rpc-spec dependency to 0.1.7 and linked rpcspec::rpcspec where needed.

Reviewed changes

Copilot reviewed 84 out of 85 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/unit/web/RPCServerHandlerTests.cpp	Update warning message expectation; switch to rpcspec header include.
tests/unit/web/ng/RPCServerHandlerTests.cpp	Replace Clio Errors include with rpcspec include.
tests/unit/web/ng/impl/ErrorHandlingTests.cpp	Replace Clio Errors include with rpcspec include.
tests/unit/web/LoadWarningTests.cpp	Replace Clio Errors include with rpcspec include.
tests/unit/web/impl/ErrorHandlingTests.cpp	Replace Clio Errors include with rpcspec include.
tests/unit/rpc/RPCHelpersTests.cpp	Add rpcspec errors include for updated error model.
tests/unit/rpc/RPCEngineTests.cpp	Switch to rpcspec errors include.
tests/unit/rpc/handlers/VaultInfoTests.cpp	Add rpcspec errors include.
tests/unit/rpc/handlers/UnsubscribeTests.cpp	Add rpcspec errors include.
tests/unit/rpc/handlers/TxTests.cpp	Add rpcspec errors include.
tests/unit/rpc/handlers/TestHandlerTests.cpp	Add rpcspec errors include.
tests/unit/rpc/handlers/SubscribeTests.cpp	Add rpcspec errors include.
tests/unit/rpc/handlers/ServerInfoTests.cpp	Add rpcspec errors include; update forwarding error expectations.
tests/unit/rpc/handlers/LedgerTests.cpp	Add rpcspec errors include.
tests/unit/rpc/handlers/LedgerEntryTests.cpp	Add rpcspec errors include.
tests/unit/rpc/handlers/LedgerDataTests.cpp	Add rpcspec errors include.
tests/unit/rpc/handlers/CredentialHelpersTests.cpp	Switch to rpcspec errors include.
tests/unit/rpc/handlers/AccountOffersTests.cpp	Add rpcspec errors include.
tests/unit/rpc/handlers/AccountLinesTests.cpp	Add rpcspec errors include.
tests/unit/rpc/handlers/AccountInfoTests.cpp	Add rpcspec errors include.
tests/unit/rpc/handlers/AccountCurrenciesTests.cpp	Add rpcspec errors include.
tests/unit/rpc/ForwardingProxyTests.cpp	Switch to rpcspec errors include; update forwarding error expectations.
tests/unit/rpc/ErrorTests.cpp	Add rpcspec errors include; update warning text and forwarding error codes.
tests/unit/rpc/common/TypesTests.cpp	Drop direct Clio Errors include (Types already pulls it in).
tests/unit/rpc/common/SpecsTests.cpp	Switch to rpcspec errors include.
tests/unit/rpc/common/CheckersTests.cpp	Switch to rpcspec errors include.
tests/unit/rpc/BaseTests.cpp	Switch to rpcspec errors include.
tests/unit/etl/SourceImplTests.cpp	Switch to rpcspec errors include.
tests/unit/etl/LoadBalancerTests.cpp	Switch to rpcspec errors include; update forwarding error expectations.
tests/unit/etl/ForwardingSourceTests.cpp	Switch to rpcspec errors include; update forwarding error expectations.
tests/unit/etl/ETLStateTests.cpp	Switch to rpcspec errors include; update forwarding error expectations.
tests/unit/app/WebHandlersTests.cpp	Switch to rpcspec errors include.
tests/common/util/MockSource.hpp	Switch to rpcspec errors include.
src/web/ng/impl/ErrorHandling.cpp	Update forwarding error enum names; include rpcspec errors.
src/web/impl/ErrorHandling.hpp	Update forwarding error enum names.
src/web/CMakeLists.txt	Link rpcspec library into web target.
src/rpc/RPCHelpers.cpp	Switch to rpcspec errors include.
src/rpc/handlers/VaultInfo.cpp	Switch to rpcspec errors include.
src/rpc/handlers/Unsubscribe.cpp	Switch to rpcspec errors include.
src/rpc/handlers/TransactionEntry.cpp	Switch to rpcspec errors include.
src/rpc/handlers/Subscribe.cpp	Switch to rpcspec errors include.
src/rpc/handlers/NoRippleCheck.cpp	Switch to rpcspec errors include.
src/rpc/handlers/NFTsByIssuer.cpp	Switch to rpcspec errors include.
src/rpc/handlers/NFTOffersCommon.cpp	Switch to rpcspec errors include.
src/rpc/handlers/NFTInfo.cpp	Switch to rpcspec errors include.
src/rpc/handlers/NFTHistory.cpp	Switch to rpcspec errors include.
src/rpc/handlers/MPTokenIssuanceHistory.cpp	Switch to rpcspec errors include.
src/rpc/handlers/MPTHolders.cpp	Switch to rpcspec errors include.
src/rpc/handlers/LedgerIndex.cpp	Switch to rpcspec errors include.
src/rpc/handlers/LedgerEntry.cpp	Switch to rpcspec errors include.
src/rpc/handlers/LedgerData.cpp	Switch to rpcspec errors include.
src/rpc/handlers/GetAggregatePrice.cpp	Remove Clio Errors include (no longer needed).
src/rpc/handlers/GatewayBalances.cpp	Switch to rpcspec errors include.
src/rpc/handlers/Feature.cpp	Switch to rpcspec errors include.
src/rpc/handlers/DepositAuthorized.cpp	Switch to rpcspec errors include.
src/rpc/handlers/AMMInfo.cpp	Switch to rpcspec errors include.
src/rpc/handlers/AccountTx.cpp	Switch to rpcspec errors include.
src/rpc/handlers/AccountOffers.cpp	Switch to rpcspec errors include.
src/rpc/handlers/AccountObjects.cpp	Switch to rpcspec errors include.
src/rpc/handlers/AccountNFTs.cpp	Switch to rpcspec errors include.
src/rpc/handlers/AccountMPTokens.cpp	Switch to rpcspec errors include.
src/rpc/handlers/AccountMPTokenIssuances.cpp	Switch to rpcspec errors include.
src/rpc/handlers/AccountLines.cpp	Switch to rpcspec errors include.
src/rpc/handlers/AccountInfo.cpp	Switch to rpcspec errors include.
src/rpc/handlers/AccountCurrencies.cpp	Switch to rpcspec errors include.
src/rpc/handlers/AccountChannels.cpp	Switch to rpcspec errors include.
src/rpc/Factories.cpp	Switch to rpcspec errors include.
src/rpc/Errors.hpp	Convert to shim over rpcspec errors; keep Clio-side rendering surface.
src/rpc/Errors.cpp	Move remaining Clio-only error-info table; use `std::to_array`/`ranges::find`; update forwarding error names.
src/rpc/CredentialHelpers.cpp	Switch to rpcspec errors include.
src/rpc/common/Validators.cpp	Switch to rpcspec errors include.
src/rpc/common/Specs.hpp	Docstring tweaks referencing `Status`.
src/rpc/common/Specs.cpp	Switch to rpcspec errors include.
src/rpc/common/MetaProcessors.cpp	Switch to rpcspec errors include.
src/rpc/common/AnyHandler.hpp	Docstring tweak referencing `Status`.
src/feed/CMakeLists.txt	Link rpcspec library into feed target.
src/etl/Source.hpp	Switch to rpcspec errors include for forwarding error type usage.
src/etl/LoadBalancer.cpp	Update default forwarding error code to renamed `RpcForwarding*`.
src/etl/impl/SourceImpl.hpp	Switch to rpcspec errors include.
src/etl/impl/ForwardingSource.hpp	Switch to rpcspec errors include.
src/etl/impl/ForwardingSource.cpp	Rename forwarding error returns to `RpcForwarding*`.
src/etl/CMakeLists.txt	Link rpcspec library into etl target.
src/app/WebHandlers.cpp	Add rpcspec errors include.
conanfile.py	Bump `xrpl-rpc-spec` dependency to `0.1.7`.
conan.lock	Update lockfile entry for `xrpl-rpc-spec/0.1.7`.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

 #include <boost/uuid/uuid.hpp>
 #include <grpcpp/support/status.h>
 #include <org/xrpl/rpc/v1/get_ledger.pb.h>
+#include <rpcspec/Errors.hpp>


mathbunnyru · 2026-08-27T14:26:48Z

    if (not expectedConnection) {
        LOG(log_.debug()) << "Couldn't connect to rippled to forward request.";
-        return std::unexpected{rpc::ClioError::EtlConnectionError};
+        return std::unexpected{rpc::ClioError::RpcForwardingConnectionError};


Since you have ClioError, I think it would be great to make rpc-spec not have access to it, when server is rippled.

Maybe as a final cleanup step when everything is working everywhere :)

That's easier & better to do now, before someone (probably you 😆) accidentally start using clio codes in rpc-spec for rippled or rippled itself

I'll make a pr for the rpc-spec repo, let's not enforce it in this PR yet to avoid having to make a new version and publish through conan-center

godexsoft added 6 commits August 11, 2026 18:13

Introduce rpcspec 0.1.3 dependency

2601f8d

Add the missing cmake file back

8ce6b04

Fix precommit

df88182

Resolve conflicts

0cf14ab

Use errors from rpcspec

3196ef5

Remove dead code

95b28f7

godexsoft added 6 commits August 12, 2026 14:22

Fix clang-tidy

e31d7b5

Fix precommit

aef5b35

Fix doxy

9d4ee75

Fix more clang-tidy

48faa0c

Merge branch 'develop' into refactor/consteval-specs-move-errors

866deea

Merge branch 'develop' of github.com:XRPLF/clio into refactor/constev…

5ab47a9

…al-specs-move-errors

godexsoft requested a review from mathbunnyru August 26, 2026 16:07

Fix clang-tidy

577bc40

mathbunnyru requested changes Aug 26, 2026

View reviewed changes

mathbunnyru reviewed Aug 26, 2026

View reviewed changes

Rename to RpcForwarding errors and move back to rpc-spec

30621e6

godexsoft requested a review from mathbunnyru August 27, 2026 12:18

mathbunnyru requested a lite review from Copilot August 27, 2026 14:21

Copilot started reviewing on behalf of mathbunnyru August 27, 2026 14:22 View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

Comment thread src/etl/Source.hpp

#include <boost/uuid/uuid.hpp>

#include <grpcpp/support/status.h>

#include <org/xrpl/rpc/v1/get_ledger.pb.h>

#include <rpcspec/Errors.hpp>

mathbunnyru reviewed Aug 27, 2026

View reviewed changes

mathbunnyru approved these changes Aug 27, 2026

View reviewed changes

Uh oh!

Conversation

godexsoft commented Aug 12, 2026

Uh oh!

codecov Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mathbunnyru Aug 26, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mathbunnyru Aug 27, 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Aug 12, 2026 •

edited

Loading

mathbunnyru Aug 26, 2026 •

edited

Loading

mathbunnyru Aug 27, 2026 •

edited

Loading