Skip to content

Use thiserror to handle errors #311

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

Closed
bidoubiwa opened this issue Aug 3, 2022 · 2 comments · Fixed by #356
Closed

Use thiserror to handle errors #311

bidoubiwa opened this issue Aug 3, 2022 · 2 comments · Fixed by #356
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@bidoubiwa
Copy link
Contributor

⚠️ Do not start contributing to this issue before this PR is merged: #297

Currently we are implementing all our errors by hand.

Exemple:

impl From<MeilisearchError> for Error {
fn from(error: MeilisearchError) -> Self {
Self::Meilisearch(error)
}
}
impl From<jsonwebtoken::errors::Error> for Error {
fn from(error: jsonwebtoken::errors::Error) -> Error {
Error::InvalidTenantToken(error)
}
}

This would be way more simple using the thiserror library.

@bidoubiwa bidoubiwa added good first issue Good for newcomers enhancement New feature or request labels Aug 3, 2022
@romilpunetha
Copy link
Contributor

I can take this up.

@bidoubiwa
Copy link
Contributor Author

Thanks for your interest in this project 🔥 You are definitely more than welcome to open a PR for this!

We prefer not assigning people to our issues because sometimes people ask to be assigned and never come back, which discourages the real volunteers contributors from opening a PR to fix this issue.
We will accept and merge the first PR that fixes correctly and well implements the issue following our contributing guidelines.

We are looking forward to reviewing your PR 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants