Refactor error types to use thiserror and internalize BuildError
#228
Conversation
Xtansia
force-pushed
the
refactor/error
branch
from
72a4132
to
88455f6
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #228 +/- ##
==========================================
+ Coverage 74.00% 74.07% +0.06%
==========================================
Files 403 403
Lines 64198 64138 -60
==========================================
Hits 47508 47508
+ Misses 16690 16630 -60
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Xtansia
force-pushed
the
refactor/error
branch
2 times, most recently
from
6f305b2
to
f52b6bc
Compare
opensearch/src/cert.rs
Outdated
| certs.push(reqwest::Certificate::from_pem(cert.join("\n").as_bytes())?); | ||
| certs.push( | ||
| reqwest::Certificate::from_pem(cert.join("\n").as_bytes()) | ||
| .map_err(CertificateError::MalformedCertificate)?, |
There was a problem hiding this comment.
Super nit:
| .map_err(CertificateError::MalformedCertificate)?, | |
| .map_err(CertificateError::MalformedCertificate)? |
There was a problem hiding this comment.
This is imposed by cargo fmt due to how it decides to break the line. Only way around it is to restructure the line entirely.
opensearch/src/error.rs
Outdated
| } | ||
| } | ||
| #[error("reqwest error: {0}")] | ||
| Reqwest(#[from] reqwest::Error), |
There was a problem hiding this comment.
I would suggest to keep Http here, Reqwest is just client implementation, I don't think we should expose it
| Reqwest(#[from] reqwest::Error), | |
| Http(#[from] reqwest::Error), |
There was a problem hiding this comment.
Ah I see, we use it at build time, may be ClientBuilder would be better one?
There was a problem hiding this comment.
We already expose reqwest types externally so that's not a new problem as such. But these variants aren't actually exposed other than via the descriptor text or debug formatting. But certainly happy to rename to something that makes more sense to specific cases
Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Xtansia
force-pushed
the
refactor/error
branch
from
f52b6bc
to
f24b729
Compare
Description
Refactors the error types to use
thiserrorand internalize theBuildErrortype.Split from #132.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.