Skip to content

Commit 5d3c472

Browse files
seanmonstarseanmonstar
committed
feat(error): revamp hyper::Error type
**The `Error` is now an opaque struct**, which allows for more variants to be added freely, and the internal representation to change without being breaking changes. For inspecting an `Error`, there are several `is_*` methods to check for certain classes of errors, such as `Error::is_parse()`. The `cause` can also be inspected, like before. This likely seems like a downgrade, but more inspection can be added as needed! The `Error` now knows about more states, which gives much more context around when a certain error occurs. This is also expressed in the description and `fmt` messages. **Most places where a user would provide an error to hyper can now pass any error type** (`E: Into<Box<std::error::Error>>`). This error is passed back in relevant places, and can be useful for logging. This should make it much clearer about what error a user should provide to hyper: any it feels is relevant! Closes #1128 Closes #1130 Closes #1431 Closes #1338 BREAKING CHANGE: `Error` is no longer an enum to pattern match over, or to construct. Code will need to be updated accordingly. For body streams or `Service`s, inference might be unable to determine what error type you mean to return. Starting in Rust 1.26, you could just label that as `!` if you never return an error.
1 parent 33874f9 commit 5d3c472

22 files changed

+514
-402
lines changed

‎benches/server.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ fn throughput_fixedsize_large_payload(b: &mut test::Bencher) {
7878
fn throughput_fixedsize_many_chunks(b: &mut test::Bencher) {
7979
bench_server!(b, ("content-length", "1000000"), || {
8080
static S: &'static [&'static [u8]] = &[&[b'x'; 1_000] as &[u8]; 1_000] as _;
81-
Body::wrap_stream(stream::iter_ok(S.iter()).map(|&s| s))
81+
Body::wrap_stream(stream::iter_ok::<_, String>(S.iter()).map(|&s| s))
8282
})
8383
}
8484

@@ -96,7 +96,7 @@ fn throughput_chunked_large_payload(b: &mut test::Bencher) {
9696
fn throughput_chunked_many_chunks(b: &mut test::Bencher) {
9797
bench_server!(b, ("transfer-encoding", "chunked"), || {
9898
static S: &'static [&'static [u8]] = &[&[b'x'; 1_000] as &[u8]; 1_000] as _;
99-
Body::wrap_stream(stream::iter_ok(S.iter()).map(|&s| s))
99+
Body::wrap_stream(stream::iter_ok::<_, String>(S.iter()).map(|&s| s))
100100
})
101101
}
102102

‎examples/client.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,9 @@ fn main() {
4040
println!("Response: {}", res.status());
4141
println!("Headers: {:#?}", res.headers());
4242

43-
res.into_parts().1.into_stream().for_each(|chunk| {
44-
io::stdout().write_all(&chunk).map_err(From::from)
43+
res.into_body().into_stream().for_each(|chunk| {
44+
io::stdout().write_all(&chunk)
45+
.map_err(|e| panic!("example expects stdout is open, error={}", e))
4546
})
4647
}).map(|_| {
4748
println!("\n\nDone.");

‎examples/hello.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ fn main() {
1717
let addr = ([127, 0, 0, 1], 3000).into();
1818

1919
let new_service = const_service(service_fn(|_| {
20-
Ok(Response::new(Body::from(PHRASE)))
20+
//TODO: when `!` is stable, replace error type
21+
Ok::<_, hyper::Error>(Response::new(Body::from(PHRASE)))
2122
}));
2223

2324
tokio::run(lazy(move || {

‎examples/send_file.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use futures::future::lazy;
99
use futures::sync::oneshot;
1010

1111
use hyper::{Body, /*Chunk,*/ Method, Request, Response, StatusCode};
12-
use hyper::error::Error;
1312
use hyper::server::{Http, Service};
1413

1514
use std::fs::File;
@@ -19,7 +18,7 @@ use std::thread;
1918
static NOTFOUND: &[u8] = b"Not Found";
2019
static INDEX: &str = "examples/send_file_index.html";
2120

22-
fn simple_file_send(f: &str) -> Box<Future<Item = Response<Body>, Error = hyper::Error> + Send> {
21+
fn simple_file_send(f: &str) -> Box<Future<Item = Response<Body>, Error = io::Error> + Send> {
2322
// Serve a file by reading it entirely into memory. As a result
2423
// this is limited to serving small files, but it is somewhat
2524
// simpler with a little less overhead.
@@ -56,15 +55,15 @@ fn simple_file_send(f: &str) -> Box<Future<Item = Response<Body>, Error = hyper:
5655
};
5756
});
5857

59-
Box::new(rx.map_err(|e| Error::from(io::Error::new(io::ErrorKind::Other, e))))
58+
Box::new(rx.map_err(|e| io::Error::new(io::ErrorKind::Other, e)))
6059
}
6160

6261
struct ResponseExamples;
6362

6463
impl Service for ResponseExamples {
6564
type Request = Request<Body>;
6665
type Response = Response<Body>;
67-
type Error = hyper::Error;
66+
type Error = io::Error;
6867
type Future = Box<Future<Item = Self::Response, Error = Self::Error> + Send>;
6968

7069
fn call(&self, req: Request<Body>) -> Self::Future {
@@ -119,7 +118,7 @@ impl Service for ResponseExamples {
119118
*/
120119
});
121120

122-
Box::new(rx.map_err(|e| Error::from(io::Error::new(io::ErrorKind::Other, e))))
121+
Box::new(rx.map_err(|e| io::Error::new(io::ErrorKind::Other, e)))
123122
},
124123
(&Method::GET, "/no_file.html") => {
125124
// Test what happens when file cannot be be found

‎src/client/conn.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ pub struct SendRequest<B> {
4545
pub struct Connection<T, B>
4646
where
4747
T: AsyncRead + AsyncWrite,
48-
B: Entity<Error=::Error> + 'static,
48+
B: Entity + 'static,
4949
{
5050
inner: proto::dispatch::Dispatcher<
5151
proto::dispatch::Client<B>,
@@ -138,7 +138,7 @@ impl<B> SendRequest<B>
138138

139139
impl<B> SendRequest<B>
140140
where
141-
B: Entity<Error=::Error> + 'static,
141+
B: Entity + 'static,
142142
{
143143
/// Sends a `Request` on the associated connection.
144144
///
@@ -262,7 +262,7 @@ impl<B> fmt::Debug for SendRequest<B> {
262262
impl<T, B> Connection<T, B>
263263
where
264264
T: AsyncRead + AsyncWrite,
265-
B: Entity<Error=::Error> + 'static,
265+
B: Entity + 'static,
266266
{
267267
/// Return the inner IO object, and additional information.
268268
pub fn into_parts(self) -> Parts<T> {
@@ -289,7 +289,7 @@ where
289289
impl<T, B> Future for Connection<T, B>
290290
where
291291
T: AsyncRead + AsyncWrite,
292-
B: Entity<Error=::Error> + 'static,
292+
B: Entity + 'static,
293293
{
294294
type Item = ();
295295
type Error = ::Error;
@@ -302,7 +302,7 @@ where
302302
impl<T, B> fmt::Debug for Connection<T, B>
303303
where
304304
T: AsyncRead + AsyncWrite + fmt::Debug,
305-
B: Entity<Error=::Error> + 'static,
305+
B: Entity + 'static,
306306
{
307307
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
308308
f.debug_struct("Connection")
@@ -331,7 +331,7 @@ impl Builder {
331331
pub fn handshake<T, B>(&self, io: T) -> Handshake<T, B>
332332
where
333333
T: AsyncRead + AsyncWrite,
334-
B: Entity<Error=::Error> + 'static,
334+
B: Entity + 'static,
335335
{
336336
Handshake {
337337
inner: HandshakeInner {
@@ -345,7 +345,7 @@ impl Builder {
345345
pub(super) fn handshake_no_upgrades<T, B>(&self, io: T) -> HandshakeNoUpgrades<T, B>
346346
where
347347
T: AsyncRead + AsyncWrite,
348-
B: Entity<Error=::Error> + 'static,
348+
B: Entity + 'static,
349349
{
350350
HandshakeNoUpgrades {
351351
inner: HandshakeInner {
@@ -362,7 +362,7 @@ impl Builder {
362362
impl<T, B> Future for Handshake<T, B>
363363
where
364364
T: AsyncRead + AsyncWrite,
365-
B: Entity<Error=::Error> + 'static,
365+
B: Entity + 'static,
366366
{
367367
type Item = (SendRequest<B>, Connection<T, B>);
368368
type Error = ::Error;
@@ -387,7 +387,7 @@ impl<T, B> fmt::Debug for Handshake<T, B> {
387387
impl<T, B> Future for HandshakeNoUpgrades<T, B>
388388
where
389389
T: AsyncRead + AsyncWrite,
390-
B: Entity<Error=::Error> + 'static,
390+
B: Entity + 'static,
391391
{
392392
type Item = (SendRequest<B>, proto::dispatch::Dispatcher<
393393
proto::dispatch::Client<B>,
@@ -406,7 +406,7 @@ where
406406
impl<T, B, R> Future for HandshakeInner<T, B, R>
407407
where
408408
T: AsyncRead + AsyncWrite,
409-
B: Entity<Error=::Error> + 'static,
409+
B: Entity + 'static,
410410
R: proto::Http1Transaction<
411411
Incoming=StatusCode,
412412
Outgoing=proto::RequestLine,
@@ -470,15 +470,15 @@ impl<B: Send> AssertSendSync for SendRequest<B> {}
470470
impl<T: Send, B: Send> AssertSend for Connection<T, B>
471471
where
472472
T: AsyncRead + AsyncWrite,
473-
B: Entity<Error=::Error> + 'static,
473+
B: Entity + 'static,
474474
B::Data: Send + 'static,
475475
{}
476476

477477
#[doc(hidden)]
478478
impl<T: Send + Sync, B: Send + Sync> AssertSendSync for Connection<T, B>
479479
where
480480
T: AsyncRead + AsyncWrite,
481-
B: Entity<Error=::Error> + 'static,
481+
B: Entity + 'static,
482482
B::Data: Send + Sync + 'static,
483483
{}
484484

‎src/client/connect.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ pub trait Connect: Send + Sync {
3636
/// The connected IO Stream.
3737
type Transport: AsyncRead + AsyncWrite + Send + 'static;
3838
/// An error occured when trying to connect.
39-
type Error;
39+
type Error: Into<Box<StdError + Send + Sync>>;
4040
/// A Future that will resolve to the connected Transport.
4141
type Future: Future<Item=(Self::Transport, Connected), Error=Self::Error> + Send;
4242
/// Connect to a destination.

‎src/client/dispatch.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@ impl<T, U> Sender<T, U> {
3939
// there's room in the queue, but does the Connection
4040
// want a message yet?
4141
self.giver.poll_want()
42-
.map_err(|_| ::Error::Closed)
42+
.map_err(|_| ::Error::new_closed())
4343
},
4444
Ok(Async::NotReady) => Ok(Async::NotReady),
45-
Err(_) => Err(::Error::Closed),
45+
Err(_) => Err(::Error::new_closed()),
4646
}
4747
}
4848

@@ -184,9 +184,12 @@ mod tests {
184184
drop(rx);
185185

186186
promise.then(|fulfilled| {
187-
let res = fulfilled.expect("fulfilled");
188-
match res.unwrap_err() {
189-
(::Error::Cancel(_), Some(_)) => (),
187+
let err = fulfilled
188+
.expect("fulfilled")
189+
.expect_err("promise should error");
190+
191+
match (err.0.kind(), err.1) {
192+
(&::error::Kind::Canceled, Some(_)) => (),
190193
e => panic!("expected Error::Cancel(_), found {:?}", e),
191194
}
192195

‎src/client/mod.rs

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,10 @@ impl<C, B> Client<C, B> {
9696
}
9797

9898
impl<C, B> Client<C, B>
99-
where C: Connect<Error=io::Error> + Sync + 'static,
99+
where C: Connect + Sync + 'static,
100100
C::Transport: 'static,
101101
C::Future: 'static,
102-
B: Entity<Error=::Error> + Send + 'static,
102+
B: Entity + Send + 'static,
103103
B::Data: Send,
104104
{
105105

@@ -139,13 +139,14 @@ where C: Connect<Error=io::Error> + Sync + 'static,
139139
Version::HTTP_11 => (),
140140
other => {
141141
error!("Request has unsupported version \"{:?}\"", other);
142-
return FutureResponse(Box::new(future::err(::Error::Version)));
142+
//TODO: replace this with a proper variant
143+
return FutureResponse(Box::new(future::err(::Error::new_user_unsupported_version())));
143144
}
144145
}
145146

146147
if req.method() == &Method::CONNECT {
147148
debug!("Client does not support CONNECT requests");
148-
return FutureResponse(Box::new(future::err(::Error::Method)));
149+
return FutureResponse(Box::new(future::err(::Error::new_user_unsupported_request_method())));
149150
}
150151

151152
let uri = req.uri().clone();
@@ -154,7 +155,8 @@ where C: Connect<Error=io::Error> + Sync + 'static,
154155
format!("{}://{}", scheme, auth)
155156
}
156157
_ => {
157-
return FutureResponse(Box::new(future::err(::Error::Io(
158+
//TODO: replace this with a proper variant
159+
return FutureResponse(Box::new(future::err(::Error::new_io(
158160
io::Error::new(
159161
io::ErrorKind::InvalidInput,
160162
"invalid URI for Client Request"
@@ -203,13 +205,13 @@ where C: Connect<Error=io::Error> + Sync + 'static,
203205
};
204206
future::lazy(move || {
205207
connector.connect(dst)
206-
.from_err()
208+
.map_err(::Error::new_connect)
207209
.and_then(move |(io, connected)| {
208210
conn::Builder::new()
209211
.h1_writev(h1_writev)
210212
.handshake_no_upgrades(io)
211213
.and_then(move |(tx, conn)| {
212-
executor.execute(conn.map_err(|e| debug!("client connection error: {}", e)))?;
214+
executor.execute(conn.map_err(|e| debug!("client connection error: {}", e)));
213215
Ok(pool.pooled(pool_key, PoolClient {
214216
is_proxied: connected.is_proxied,
215217
tx: tx,
@@ -260,9 +262,7 @@ where C: Connect<Error=io::Error> + Sync + 'static,
260262
} else if !res.body().is_empty() {
261263
let (delayed_tx, delayed_rx) = oneshot::channel();
262264
res.body_mut().delayed_eof(delayed_rx);
263-
// If the executor doesn't have room, oh well. Things will likely
264-
// be blowing up soon, but this specific task isn't required.
265-
let _ = executor.execute(
265+
executor.execute(
266266
future::poll_fn(move || {
267267
pooled.tx.poll_ready()
268268
})
@@ -277,7 +277,6 @@ where C: Connect<Error=io::Error> + Sync + 'static,
277277
Ok(res)
278278
});
279279

280-
281280
fut
282281
});
283282

@@ -290,9 +289,9 @@ where C: Connect<Error=io::Error> + Sync + 'static,
290289
}
291290

292291
impl<C, B> Service for Client<C, B>
293-
where C: Connect<Error=io::Error> + 'static,
292+
where C: Connect + 'static,
294293
C::Future: 'static,
295-
B: Entity<Error=::Error> + Send + 'static,
294+
B: Entity + Send + 'static,
296295
B::Data: Send,
297296
{
298297
type Request = Request<B>;
@@ -353,9 +352,9 @@ struct RetryableSendRequest<C, B> {
353352

354353
impl<C, B> Future for RetryableSendRequest<C, B>
355354
where
356-
C: Connect<Error=io::Error> + 'static,
355+
C: Connect + 'static,
357356
C::Future: 'static,
358-
B: Entity<Error=::Error> + Send + 'static,
357+
B: Entity + Send + 'static,
359358
B::Data: Send,
360359
{
361360
type Item = Response<Body>;
@@ -564,10 +563,10 @@ impl<C, B> Config<C, B> {
564563
}
565564

566565
impl<C, B> Config<C, B>
567-
where C: Connect<Error=io::Error>,
566+
where C: Connect,
568567
C::Transport: 'static,
569568
C::Future: 'static,
570-
B: Entity<Error=::Error> + Send,
569+
B: Entity + Send,
571570
B::Data: Send,
572571
{
573572
/// Construct the Client with this configuration.
@@ -590,7 +589,7 @@ where C: Connect<Error=io::Error>,
590589

591590
impl<B> Config<UseDefaultConnector, B>
592591
where
593-
B: Entity<Error=::Error> + Send,
592+
B: Entity + Send,
594593
B::Data: Send,
595594
{
596595
/// Construct the Client with this configuration.
@@ -640,7 +639,6 @@ impl<C: Clone, B> Clone for Config<C, B> {
640639
}
641640
}
642641

643-
644642
// ===== impl Exec =====
645643

646644
#[derive(Clone)]
@@ -655,24 +653,19 @@ impl Exec {
655653
Exec::Executor(Arc::new(executor))
656654
}
657655

658-
fn execute<F>(&self, fut: F) -> io::Result<()>
656+
fn execute<F>(&self, fut: F)
659657
where
660658
F: Future<Item=(), Error=()> + Send + 'static,
661659
{
662660
match *self {
663661
Exec::Default => spawn(fut),
664662
Exec::Executor(ref e) => {
665-
e.execute(bg(Box::new(fut)))
663+
let _ = e.execute(bg(Box::new(fut)))
666664
.map_err(|err| {
667-
debug!("executor error: {:?}", err.kind());
668-
io::Error::new(
669-
io::ErrorKind::Other,
670-
"executor error",
671-
)
672-
})?
665+
panic!("executor error: {:?}", err.kind());
666+
});
673667
},
674668
}
675-
Ok(())
676669
}
677670
}
678671

‎src/client/pool.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ impl<T: Closed + Send + 'static> Pool<T> {
245245
interval: interval,
246246
pool: Arc::downgrade(&self.inner),
247247
pool_drop_notifier: rx,
248-
}).unwrap();
248+
});
249249
}
250250
}
251251

0 commit comments

Comments
 (0)