Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cuprated: initial RPC module skeleton #262

Merged
merged 23 commits into from
Sep 8, 2024
Merged

Conversation

hinto-janai
Copy link
Contributor

@hinto-janai hinto-janai commented Sep 1, 2024

What

Adds the initial skeleton for the RPC module in cuprated and many function signatures filled with todo!().

@github-actions github-actions bot added A-docs Related to documentation. A-binaries Related to binaries. labels Sep 1, 2024
@github-actions github-actions bot added A-dependency Related to dependencies, or changes to a Cargo.{toml,lock} file. A-workspace Changes to a root workspace file or general repo file. labels Sep 2, 2024
@hinto-janai hinto-janai mentioned this pull request Sep 2, 2024
24 tasks
@github-actions github-actions bot added the A-rpc Related to RPC. label Sep 2, 2024
binaries/cuprated/Cargo.toml Show resolved Hide resolved
binaries/cuprated/src/main.rs Show resolved Hide resolved
binaries/cuprated/src/rpc/handler.rs Show resolved Hide resolved
binaries/cuprated/src/rpc/json.rs Outdated Show resolved Hide resolved
Comment on lines 19 to 28
#[derive(Debug, thiserror::Error)]
pub enum RpcError {
/// A [`std::io::Error`] from the database.
#[error("database I/O error: {0}")]
DatabaseIo(#[from] std::io::Error),

/// A (non-I/O related) database error.
#[error("database error: {0}")]
DatabaseError(RuntimeError),
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to cuprate_rpc_interface::RpcError. The database(s) return RuntimeError in their Service signatures so we forward that.

This also means that cuprate_rpc_interface depends on cuprate_database, it doesn't seem ideal to pull all of it in for just the error type though, especially since none of the errors other than IO can happen at this level.

cuprated already pulls everything in anyway, but this means other RpcHandler impls will have to pull in cuprate-database too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it worth just making this a BoxError? that's what I have done elsewhere, like in the consensus crate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to be (ab)using opaque errors internally, what about using anyhow::Error?

It's basically a better Box<dyn Error + Send + Sync + 'static>.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sure that's probably a good idea.

rpc/interface/src/rpc_service.rs Outdated Show resolved Hide resolved
@hinto-janai hinto-janai marked this pull request as ready for review September 6, 2024 00:38
binaries/cuprated/src/rpc/handler.rs Outdated Show resolved Hide resolved
binaries/cuprated/src/rpc/json.rs Outdated Show resolved Hide resolved
Comment on lines 19 to 28
#[derive(Debug, thiserror::Error)]
pub enum RpcError {
/// A [`std::io::Error`] from the database.
#[error("database I/O error: {0}")]
DatabaseIo(#[from] std::io::Error),

/// A (non-I/O related) database error.
#[error("database error: {0}")]
DatabaseError(RuntimeError),
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it worth just making this a BoxError? that's what I have done elsewhere, like in the consensus crate.

@Boog900 Boog900 merged commit 9280081 into Cuprate:main Sep 8, 2024
6 checks passed
@hinto-janai hinto-janai deleted the rpc-handler branch September 8, 2024 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-binaries Related to binaries. A-dependency Related to dependencies, or changes to a Cargo.{toml,lock} file. A-docs Related to documentation. A-rpc Related to RPC. A-workspace Changes to a root workspace file or general repo file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants