Skip to content

Commit 47a58dd

Browse files
KodrAusrusscam
KodrAus
authored and
russcam
committed
Make the top-level error type opaque (#41)
This commit refactors the top-level Error enum so that it's an opaque struct instead of an open enum. This allows changes to the internals of the error without breaking callers relying on the enum having a particular shape. As an example change, we might want to replace the Lib(String) variant with something strongly typed one day. It also means a Backtrace could be introduced, because it'll have a natural place to live within the struct.
1 parent 07a62fe commit 47a58dd

File tree

4 files changed

+59
-52
lines changed

4 files changed

+59
-52
lines changed

‎elasticsearch/src/error.rs

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,12 @@ use std::io;
2525
/// Errors that can occur include IO and parsing errors, as well as specific
2626
/// errors from Elasticsearch and internal errors from this library
2727
#[derive(Debug)]
28-
pub enum Error {
28+
pub struct Error {
29+
kind: Kind,
30+
}
31+
32+
#[derive(Debug)]
33+
enum Kind {
2934
/// An error building the client
3035
Build(BuildError),
3136

@@ -44,65 +49,72 @@ pub enum Error {
4449

4550
impl From<io::Error> for Error {
4651
fn from(err: io::Error) -> Error {
47-
Error::Io(err)
52+
Error {
53+
kind: Kind::Io(err),
54+
}
4855
}
4956
}
5057

5158
impl From<reqwest::Error> for Error {
5259
fn from(err: reqwest::Error) -> Error {
53-
Error::Http(err)
60+
Error {
61+
kind: Kind::Http(err)
62+
}
5463
}
5564
}
5665

5766
impl From<serde_json::error::Error> for Error {
5867
fn from(err: serde_json::error::Error) -> Error {
59-
Error::Json(err)
68+
Error {
69+
kind: Kind::Json(err)
70+
}
6071
}
6172
}
6273

6374
impl From<url::ParseError> for Error {
6475
fn from(err: url::ParseError) -> Error {
65-
Error::Lib(err.to_string())
76+
Error {
77+
kind: Kind::Lib(err.to_string())
78+
}
6679
}
6780
}
6881

6982
impl From<BuildError> for Error {
7083
fn from(err: BuildError) -> Error {
71-
Error::Build(err)
84+
Error {
85+
kind: Kind::Build(err)
86+
}
7287
}
7388
}
7489

75-
impl error::Error for Error {
76-
#[allow(warnings)]
77-
fn description(&self) -> &str {
78-
match *self {
79-
Error::Build(ref err) => err.description(),
80-
Error::Lib(ref err) => err,
81-
Error::Http(ref err) => err.description(),
82-
Error::Io(ref err) => err.description(),
83-
Error::Json(ref err) => err.description(),
90+
impl Error {
91+
pub(crate) fn lib(err: impl Into<String>) -> Self {
92+
Error {
93+
kind: Kind::Lib(err.into())
8494
}
8595
}
96+
}
8697

87-
fn cause(&self) -> Option<&dyn error::Error> {
88-
match *self {
89-
Error::Build(ref err) => Some(err as &dyn error::Error),
90-
Error::Lib(_) => None,
91-
Error::Http(ref err) => Some(err as &dyn error::Error),
92-
Error::Io(ref err) => Some(err as &dyn error::Error),
93-
Error::Json(ref err) => Some(err as &dyn error::Error),
98+
impl error::Error for Error {
99+
fn source(&self) -> Option<&(dyn error::Error + 'static)> {
100+
match &self.kind {
101+
Kind::Build(err) => Some(err),
102+
Kind::Lib(_) => None,
103+
Kind::Http(err) => Some(err),
104+
Kind::Io(err) => Some(err),
105+
Kind::Json(err) => Some(err),
94106
}
95107
}
96108
}
97109

98110
impl fmt::Display for Error {
99111
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
100-
match *self {
101-
Error::Build(ref err) => fmt::Display::fmt(err, f),
102-
Error::Lib(ref s) => fmt::Display::fmt(s, f),
103-
Error::Http(ref err) => fmt::Display::fmt(err, f),
104-
Error::Io(ref err) => fmt::Display::fmt(err, f),
105-
Error::Json(ref err) => fmt::Display::fmt(err, f),
112+
match &self.kind {
113+
Kind::Build(err) => err.fmt(f),
114+
Kind::Lib(err) => err.fmt(f),
115+
Kind::Http(err) => err.fmt(f),
116+
Kind::Io(err) => err.fmt(f),
117+
Kind::Json(err) => err.fmt(f),
106118
}
107119
}
108120
}

‎elasticsearch/src/http/request.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,9 @@ where
4242
{
4343
fn write(&self, bytes: &mut BytesMut) -> Result<(), Error> {
4444
let writer = bytes.writer();
45-
match serde_json::to_writer(writer, &self.0) {
46-
Ok(_) => Ok(()),
47-
Err(e) => Err(Error::Json(e)),
48-
}
45+
serde_json::to_writer(writer, &self.0)?;
46+
47+
Ok(())
4948
}
5049
}
5150

‎elasticsearch/src/http/transport.rs

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -282,9 +282,9 @@ impl Transport {
282282
Ok(r) => Ok(Response::new(r)),
283283
Err(e) => {
284284
if e.is_timeout() {
285-
Err(Error::Lib(format!("Request timed out to {:?}", e.url())))
285+
Err(Error::lib(format!("Request timed out to {:?}", e.url())))
286286
} else {
287-
Err(Error::Http(e))
287+
Err(e.into())
288288
}
289289
}
290290
}
@@ -357,21 +357,21 @@ impl CloudId {
357357
/// web console
358358
pub fn parse(cloud_id: &str) -> Result<CloudId, Error> {
359359
if cloud_id.is_empty() || !cloud_id.contains(':') {
360-
return Err(Error::Lib(
361-
"cloud_id should be of the form '<cluster name>:<base64 data>'".into(),
360+
return Err(Error::lib(
361+
"cloud_id should be of the form '<cluster name>:<base64 data>'"
362362
));
363363
}
364364

365365
let parts: Vec<&str> = cloud_id.splitn(2, ':').collect();
366366
let name: String = parts[0].into();
367367
if name.is_empty() {
368-
return Err(Error::Lib("cloud_id cluster name cannot be empty".into()));
368+
return Err(Error::lib("cloud_id cluster name cannot be empty"));
369369
}
370370

371371
let data = parts[1];
372372
let decoded_result = base64::decode(data);
373373
if decoded_result.is_err() {
374-
return Err(Error::Lib(format!("cannot base 64 decode '{}'", data)));
374+
return Err(Error::lib(format!("cannot base 64 decode '{}'", data)));
375375
}
376376

377377
let decoded = decoded_result.unwrap();
@@ -385,20 +385,19 @@ impl CloudId {
385385
Ok(s) => {
386386
domain_name = s.trim();
387387
if domain_name.is_empty() {
388-
return Err(Error::Lib("decoded '<base64 data>' must contain a domain name as the first part".into()));
388+
return Err(Error::lib("decoded '<base64 data>' must contain a domain name as the first part"));
389389
}
390390
}
391391
Err(_) => {
392-
return Err(Error::Lib(
392+
return Err(Error::lib(
393393
"decoded '<base64 data>' must contain a domain name as the first part"
394-
.into(),
395394
))
396395
}
397396
}
398397
}
399398
None => {
400-
return Err(Error::Lib(
401-
"decoded '<base64 data>' must contain a domain name as the first part".into(),
399+
return Err(Error::lib(
400+
"decoded '<base64 data>' must contain a domain name as the first part"
402401
));
403402
}
404403
}
@@ -408,20 +407,20 @@ impl CloudId {
408407
Ok(s) => {
409408
uuid = s.trim();
410409
if uuid.is_empty() {
411-
return Err(Error::Lib(
412-
"decoded '<base64 data>' must contain a uuid as the second part".into(),
410+
return Err(Error::lib(
411+
"decoded '<base64 data>' must contain a uuid as the second part"
413412
));
414413
}
415414
}
416415
Err(_) => {
417-
return Err(Error::Lib(
418-
"decoded '<base64 data>' must contain a uuid as the second part".into(),
416+
return Err(Error::lib(
417+
"decoded '<base64 data>' must contain a uuid as the second part"
419418
))
420419
}
421420
},
422421
None => {
423-
return Err(Error::Lib(
424-
"decoded '<base64 data>' must contain at least two parts".into(),
422+
return Err(Error::lib(
423+
"decoded '<base64 data>' must contain at least two parts"
425424
));
426425
}
427426
}

‎elasticsearch/src/lib.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -306,9 +306,6 @@ type _DoctestReadme = ();
306306

307307
#[macro_use]
308308
extern crate dyn_clone;
309-
extern crate reqwest;
310-
extern crate serde;
311-
extern crate serde_json;
312309

313310
pub mod auth;
314311
pub mod http;

0 commit comments

Comments
 (0)