Skip to content

Commit

Permalink
Backend error handling + polish
Browse files Browse the repository at this point in the history
Closes #113

In preparation for releasing 0.4.0, I was updating the minority-game
project to the new Api handling. It was during this process I identified
several usability issues.

First, passing any context information into an ApiCallback is a verbose
mess due to needing `move |..| async move {}`. A helper
new_with_context internalizes this ugliness, allowing for simpler
callbacks when some context information needs to be passed into the
callback.

Second, I realized by removing the associated Api type from Backend,
there now was no way for a Backend to automatically configure itself
with its Apis. Thus, Backend::configure was added.

Finally, I realized I was needing to do a lot of unwrapping, and there
was a now-unused type BackendError. Rather than removing the dead type,
I went ahead and took the time to implement basic backend error handling.
  • Loading branch information
ecton committed Mar 29, 2022
1 parent 14136ee commit fbbe2d0
Show file tree
Hide file tree
Showing 16 changed files with 139 additions and 71 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
API. New types, `AsyncDatabase` and `AsyncStorage` have been added that
provide the previous behavior. The types can be converted between each other
using helpers as/into/to_blocking/async available on each type.

These changes allow `bonsaidb-local` to be compiled without Tokio. To enable
tokio, enable feature `async` if using `bonsaidb-local` directly, or enable
feature `local-async` when using the `bonsaidb` crate.
- For `bonsaidb-server`, it still uses networking driven by Tokio.
`Server`/`CustomServer` implement `AsyncStorageConnection`, and `Server` can
convert to `Storage` via the `From` trait for synchronous database access.
Expand Down Expand Up @@ -105,6 +109,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
match the `CustomServer` being created. In general the Rust compiler should be
able to infer this type based on usage, and therefore shouldn't be a breaking
change to many people.
- The `Backend` trait now has an associated `Error` type, which allows for
custom error types to be used. When an error occurs during initialization, it
is returned. Currently, errors that are returned during client connection
handling are printed using `log::error` and ignored.
- `Key` has had its encoding functionality moved to a new trait, `KeyEncoding`.
`KeyEncoding` has been implemented for borrowed representations of `Key`
types.
Expand Down Expand Up @@ -158,6 +166,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
`WeeksSinceUnixEpoch`, `DaysSinceUnixEpoch`, ...
- Timestamps relative to the Bonsai Epoch (Mar 20, 2031 04:31:47 UTC):
`TimestampAsWeeks`, `TimestampAsDays`, ...
- `Backend::configure()` is a new function that allows a `Backend` to set
configuration options on `ServerConfiguration` before the server is
initialized. This is a good location for `Backend`s to define their `Api`s and
`Schema`s.

[221]: https://github.com/khonsulabs/bonsaidb/pull/221

Expand Down
6 changes: 1 addition & 5 deletions crates/bonsaidb-client/src/builder.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
use std::{collections::HashMap, sync::Arc};

use bonsaidb_core::{
api::{self},
networking::CURRENT_PROTOCOL_VERSION,
schema::ApiName,
};
use bonsaidb_core::{api, networking::CURRENT_PROTOCOL_VERSION, schema::ApiName};
#[cfg(not(target_arch = "wasm32"))]
use fabruic::Certificate;
#[cfg(not(target_arch = "wasm32"))]
Expand Down
20 changes: 18 additions & 2 deletions crates/bonsaidb-client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -848,9 +848,7 @@ async fn process_response_payload(
custom_apis.get(&payload.name).and_then(Option::as_ref),
payload.value,
) {
// if let Some(custom_api_callback) = custom_api_callback {
custom_api_callback.response_received(value).await;
// }
} else {
log::warn!("unexpected api response received ({})", payload.name);
}
Expand Down Expand Up @@ -896,6 +894,24 @@ impl<Api: api::Api> ApiCallback<Api> {
generator: Box::new(ApiFutureBoxer::<Api::Response, Fut>(Box::new(callback))),
}
}
/// Returns a new instance wrapping the provided function.
pub fn new_with_context<
Context: Send + Sync + Clone + 'static,
F: Fn(Api::Response, Context) -> Fut + Send + Sync + 'static,
Fut: Future<Output = ()> + Send + Sync + 'static,
>(
context: Context,
callback: F,
) -> Self {
Self {
generator: Box::new(ApiFutureBoxer::<Api::Response, Fut>(Box::new(
move |request| {
let context = context.clone();
callback(request, context)
},
))),
}
}
}

#[async_trait]
Expand Down
3 changes: 1 addition & 2 deletions crates/bonsaidb-core/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ use crate::{
view::{self, map::MappedDocuments},
Map, MappedValue, Nameable, NamedReference, Schema, SchemaName, SerializedCollection,
},
transaction::{self},
Error,
transaction, Error,
};

mod has_session;
Expand Down
5 changes: 2 additions & 3 deletions crates/bonsaidb-local/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,8 @@ use crate::{
error::Error,
open_trees::OpenTrees,
views::{
mapper::{self},
view_document_map_tree_name, view_entries_tree_name, view_invalidated_docs_tree_name,
ViewEntry,
mapper, view_document_map_tree_name, view_entries_tree_name,
view_invalidated_docs_tree_name, ViewEntry,
},
Storage,
};
Expand Down
66 changes: 47 additions & 19 deletions crates/bonsaidb-server/src/backend.rs
Original file line number Diff line number Diff line change
@@ -1,51 +1,68 @@
use std::fmt::Debug;
use std::{convert::Infallible, fmt::Debug};

use async_trait::async_trait;
use bonsaidb_core::{
api::{ApiError, Infallible},
connection::Session,
permissions::PermissionDenied,
schema::{InsertError, InvalidNameError},
};

use crate::{server::ConnectedClient, CustomServer, Error};
use crate::{server::ConnectedClient, CustomServer, Error, ServerConfiguration};

/// Tailors the behavior of a server to your needs.
#[async_trait]
pub trait Backend: Debug + Send + Sync + Sized + 'static {
/// The error type that can be returned from the backend functions. If a
/// backend doesn't need an error type, [`Infallible`] can be used.
type Error: std::error::Error + Send + Sync;
/// The type of data that can be stored in
/// [`ConnectedClient::set_client_data`]. This allows state to be stored
/// associated with each connected client.
type ClientData: Send + Sync + Debug;

/// Invoked once upon the server starting up.
/// Invoked once before the server is initialized.
#[allow(unused_variables)]
async fn initialize(server: &CustomServer<Self>) {}
fn configure(
config: ServerConfiguration<Self>,
) -> Result<ServerConfiguration<Self>, BackendError<Self::Error>> {
Ok(config)
}

/// Invoked once after initialization during
/// [`Server::open`/`CustomServer::open`](CustomServer::open).
#[allow(unused_variables)]
async fn initialize(server: &CustomServer<Self>) -> Result<(), BackendError<Self::Error>> {
Ok(())
}

/// A client disconnected from the server. This is invoked before authentication has been performed.
#[allow(unused_variables)]
#[must_use]
async fn client_connected(
client: &ConnectedClient<Self>,
server: &CustomServer<Self>,
) -> ConnectionHandling {
) -> Result<ConnectionHandling, BackendError<Self::Error>> {
log::info!(
"{:?} client connected from {:?}",
client.transport(),
client.address()
);

ConnectionHandling::Accept
Ok(ConnectionHandling::Accept)
}

/// A client disconnected from the server.
#[allow(unused_variables)]
async fn client_disconnected(client: ConnectedClient<Self>, server: &CustomServer<Self>) {
async fn client_disconnected(
client: ConnectedClient<Self>,
server: &CustomServer<Self>,
) -> Result<(), BackendError<Self::Error>> {
log::info!(
"{:?} client disconnected ({:?})",
client.transport(),
client.address()
);
Ok(())
}

/// A client successfully authenticated.
Expand All @@ -54,12 +71,13 @@ pub trait Backend: Debug + Send + Sync + Sized + 'static {
client: ConnectedClient<Self>,
session: &Session,
server: &CustomServer<Self>,
) {
) -> Result<(), BackendError<Self::Error>> {
log::info!(
"{:?} client authenticated as user: {:?}",
client.transport(),
session.identity
);
Ok(())
}
}

Expand All @@ -69,6 +87,7 @@ pub trait Backend: Debug + Send + Sync + Sized + 'static {
pub enum NoBackend {}

impl Backend for NoBackend {
type Error = Infallible;
type ClientData = ();
}

Expand All @@ -82,7 +101,7 @@ pub enum ConnectionHandling {

/// An error that can occur inside of a [`Backend`] function.
#[derive(thiserror::Error, Debug)]
pub enum BackendError<E: ApiError = Infallible> {
pub enum BackendError<E = Infallible> {
/// A backend-related error.
#[error("backend error: {0}")]
Backend(E),
Expand All @@ -91,41 +110,50 @@ pub enum BackendError<E: ApiError = Infallible> {
Server(#[from] Error),
}

impl<E: ApiError> From<PermissionDenied> for BackendError<E> {
impl<E> From<PermissionDenied> for BackendError<E> {
fn from(permission_denied: PermissionDenied) -> Self {
Self::Server(Error::from(permission_denied))
}
}

impl<E: ApiError> From<bonsaidb_core::Error> for BackendError<E> {
impl<E> From<bonsaidb_core::Error> for BackendError<E> {
fn from(err: bonsaidb_core::Error) -> Self {
Self::Server(Error::from(err))
}
}

impl<E: ApiError> From<InvalidNameError> for BackendError<E> {
impl<E> From<bonsaidb_local::Error> for BackendError<E> {
fn from(err: bonsaidb_local::Error) -> Self {
Self::Server(Error::from(err))
}
}

impl<E> From<std::io::Error> for BackendError<E> {
fn from(err: std::io::Error) -> Self {
Self::Server(Error::from(err))
}
}

impl<E> From<InvalidNameError> for BackendError<E> {
fn from(err: InvalidNameError) -> Self {
Self::Server(Error::from(err))
}
}

#[cfg(feature = "websockets")]
impl<E: ApiError> From<bincode::Error> for BackendError<E> {
impl<E> From<bincode::Error> for BackendError<E> {
fn from(other: bincode::Error) -> Self {
Self::Server(Error::from(bonsaidb_local::Error::from(other)))
}
}

impl<E: ApiError> From<pot::Error> for BackendError<E> {
impl<E> From<pot::Error> for BackendError<E> {
fn from(other: pot::Error) -> Self {
Self::Server(Error::from(bonsaidb_local::Error::from(other)))
}
}

impl<T, E> From<InsertError<T>> for BackendError<E>
where
E: ApiError,
{
impl<T, E> From<InsertError<T>> for BackendError<E> {
fn from(error: InsertError<T>) -> Self {
Self::Server(Error::from(error.error))
}
Expand Down
7 changes: 5 additions & 2 deletions crates/bonsaidb-server/src/cli/certificate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::path::PathBuf;
use clap::Subcommand;
use tokio::io::AsyncReadExt;

use crate::{Backend, CustomServer, Error};
use crate::{Backend, BackendError, CustomServer};

/// Command to manage the server's certificates.
#[derive(Subcommand, Debug)]
Expand Down Expand Up @@ -35,7 +35,10 @@ pub enum Command {

impl Command {
/// Executes the command.
pub async fn execute<B: Backend>(&self, server: &CustomServer<B>) -> Result<(), Error> {
pub async fn execute<B: Backend>(
&self,
server: &CustomServer<B>,
) -> Result<(), BackendError<B::Error>> {
match self {
Self::InstallSelfSigned { overwrite } => {
server.install_self_signed_certificate(*overwrite).await?;
Expand Down
12 changes: 9 additions & 3 deletions crates/bonsaidb-server/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub mod serve;
use bonsaidb_local::cli::StorageCommand;
use clap::Parser;

use crate::{Backend, CustomServer, Error, NoBackend, ServerConfiguration};
use crate::{Backend, BackendError, CustomServer, NoBackend, ServerConfiguration};

/// Available commands for `bonsaidb server`.
#[derive(Parser, Debug)]
Expand All @@ -25,12 +25,18 @@ pub enum Command<B: Backend = NoBackend> {

impl<B: Backend> Command<B> {
/// Executes the command.
pub async fn execute(&self, configuration: ServerConfiguration<B>) -> Result<(), Error> {
pub async fn execute(
&self,
configuration: ServerConfiguration<B>,
) -> Result<(), BackendError<B::Error>> {
let server = CustomServer::<B>::open(configuration).await?;
match self {
Self::Certificate(command) => command.execute(&server).await,
Self::Serve(command) => command.execute(&server).await,
Self::Storage(command) => command.execute_on_async(&server).await.map_err(Error::from),
Self::Storage(command) => command
.execute_on_async(&server)
.await
.map_err(BackendError::from),
}
}
}
6 changes: 3 additions & 3 deletions crates/bonsaidb-server/src/cli/serve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::time::Duration;

use clap::Args;

use crate::{Backend, CustomServer, Error, TcpService};
use crate::{Backend, BackendError, CustomServer, TcpService};

/// Execute the server
#[derive(Args, Debug)]
Expand All @@ -33,7 +33,7 @@ pub struct Serve<B: Backend> {

impl<B: Backend> Serve<B> {
/// Starts the server.
pub async fn execute(&self, server: &CustomServer<B>) -> Result<(), Error> {
pub async fn execute(&self, server: &CustomServer<B>) -> Result<(), BackendError<B::Error>> {
self.execute_with(server, ()).await
}

Expand All @@ -46,7 +46,7 @@ impl<B: Backend> Serve<B> {
&self,
server: &CustomServer<B>,
service: S,
) -> Result<(), Error> {
) -> Result<(), BackendError<B::Error>> {
// Try to initialize a logger, but ignore it if it fails. This API is
// public and another logger may already be installed.
drop(env_logger::try_init());
Expand Down
2 changes: 1 addition & 1 deletion crates/bonsaidb-server/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{collections::HashMap, marker::PhantomData, path::Path, sync::Arc};
#[cfg(feature = "encryption")]
use bonsaidb_core::document::KeyId;
use bonsaidb_core::{
api::{self},
api,
permissions::Permissions,
schema::{ApiName, Schema},
};
Expand Down
Loading

0 comments on commit fbbe2d0

Please sign in to comment.