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

Make the error returned by the BytesDecode/BytesEncode trait customizable #166

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

irevoire
Copy link
Member

This PR is an attempt at getting rid of the BoxedError type we currently use everywhere.
That allows us to use a « real » type when an error arises in the BytesEncode or BytesDecode traits.

The new traits

/// A trait that represents an encoding structure.
pub trait BytesEncode<'a> {
    type EItem: ?Sized + 'a;
    type Err;

    fn bytes_encode(item: &'a Self::EItem) -> Result<Cow<'a, [u8]>, Self::Err>;
}

/// A trait that represents a decoding structure.
pub trait BytesDecode<'a> {
    type DItem: 'a;
    type Err;

    fn bytes_decode(bytes: &'a [u8]) -> Result<Self::DItem, Self::Err>;
}

I called the type Err to do something similar to the FromStr trait from the stdlib.

The new Error enum

/// An error that encapsulates all possible errors in this crate.
#[derive(Debug)]
pub enum Error<E, D> {
    Io(io::Error),
    Mdb(MdbError),
    Encoding(E),
    Decoding(D),
    InvalidDatabaseTyping,
    DatabaseClosing,
    BadOpenOptions {
        /// The options that were used to originaly open this env.
        options: EnvOpenOptions,
        /// The env opened with the original options.
        env: Env,
    },
}

I had to make the Error enum generic over the decoding and encoding error.

For most functions, that do not add any complexity because we use our Result type;

/// Either a success or an [`Error`].
pub type Result<T, E = Infallible, D = Infallible> = result::Result<T, Error<E, D>>;

That set E and D to Infallible by default.

My issue

For some methods like the following one, we can return an error while encoding the key OR the value + while decoding the key OR the value:

    pub fn get_lower_than<'a, 'txn, KC, DC>(
        &self,
        txn: &'txn RoTxn,
        key: &'a KC::EItem,
    ) -> Result<Option<(KC::DItem, DC::DItem)>, KC::Err, DC::Err>
    where
        KC: BytesEncode<'a> + BytesDecode<'txn>,
        DC: BytesDecode<'txn>,
    {
        assert_eq_env_db_txn!(self, txn);

        let mut cursor = RoCursor::new(txn, self.dbi)?;
        let key_bytes: Cow<[u8]> = KC::bytes_encode(&key).map_err(Error::Encoding)?;
        cursor.move_on_key_greater_than_or_equal_to(&key_bytes)?;

        match cursor.move_on_prev() {
            Ok(Some((key, data))) => match (KC::bytes_decode(key), DC::bytes_decode(data)) {
                (Ok(key), Ok(data)) => Ok(Some((key, data))),
                (Err(e), _) | (_, Err(e)) => Err(Error::Decoding(e)),
            },
            Ok(None) => Ok(None),
            Err(e) => Err(e),
        }
    }

One solution could be adding two other variants to the Error enum.

@Kerollmops Kerollmops added this to the v0.20.0 milestone Mar 5, 2023
@Kerollmops Kerollmops linked an issue Mar 5, 2023 that may be closed by this pull request
@Kerollmops
Copy link
Member

Kerollmops commented Mar 5, 2023

I tried to introduce more variants to the Error enum to represent the key encoding/decoding and data encoding/decoding possible errors.

Unfortunately, it's not that easy to convert the generic Error types between them. Some functions return a Result<Infallible, Infallible, Infallible, Infallible> and it is not easy to convert that into a Result<KE, KD, DE, DD>.

@Kerollmops Kerollmops modified the milestones: v0.20.0, v0.21.0 Feb 18, 2024
@antonilol
Copy link
Contributor

antonilol commented Aug 20, 2024

For some methods like the following one, we can return an error while encoding the key OR the value + while decoding the key OR the value:

Instead of adding two variants to the enum, a new enum can be made that can hold two (different) encoding errors (or use a crate like either). The method in your example can then return Result<Option<(KC::DItem, DC::DItem)>, KC::Err, Either<KC::Err, DC::Err>>.

I came up with this here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type the error in the traits BytesEncode and BytesDecode
3 participants