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

Derive Serialize and Deserialize for Error? #1437

Open
Hirrolot opened this issue Dec 16, 2022 · 3 comments
Open

Derive Serialize and Deserialize for Error? #1437

Hirrolot opened this issue Dec 16, 2022 · 3 comments
Labels

Comments

@Hirrolot
Copy link

Hirrolot commented Dec 16, 2022

Use Case:

We use sled::Error inside our own error enumeration that must implement ResponseError from actix_web. Inside impl ResponseError, we call bincode::serialize on the error, which requires it to be Serialize. Since sled::Error doesn't implement Serialize, we have to store it simply as a string.

Proposed Change:

Introduce a new feature serde and derive Serializable and Deserializable for sled::Error.

Who Benefits From The Change(s)?

sled users who want to use sled::Error in a (de)serializable context, e.g., in a microservice architecture.

Alternative Approaches

@dariusc93
Copy link

Couldnt you make a wrapper around sled::Error with your own matching enum that derives the traits you need from serde and possibly use From/Into to convert it? That make more sense to do than to request the error to support serde itself in my opinion.

@Hirrolot
Copy link
Author

I haven't tried, but possibly I could. However, it'd require quite a lot of boilerplate and would break backwards compatibility whenever a new variant to sled::Error is added. From the library side, it'd be much easier to just add #[derive(Serialize, Deserialize)].

@quietlychris
Copy link

I'm not actually sure it works this way, at least trivially using the NewType pattern

#[derive(Serialize, Deserialize)
struct SledError(sled::Error);

will still have issues. serde's API has been stable for years now as 1.0 + SemVer, and at least within the projects that I work on, it does seem pretty common to create an opt-in feature for exposing serde traits for external types like sled::Error. I just ran into this issue myself, and it would make things much easier. From a development point of view, it shouldn't be a big lift to either add or maintain as well, and I'll probably look into submitting a PR about this once the new bloodstone work that Tyler's merging gets more fully rolled out.

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

No branches or pull requests

3 participants