Surface the source() fn in HandlerError

[whitfin]

This might fix #359.

Basically we just surface the source() function from the Error trait correctly, by delegating to the internal error.

[colinbankier]

Thanks @whitfin. Seems fine - but travis seems unhappy.

[whitfin]

@colinbankier the Travis unhappiness is due to #358 and the fact it has moved into beta :p

[fpgaminer]

I don't believe this fixes #359

Here's my minimal test:

use gotham::handler::{HandlerError, IntoHandlerError};
use gotham::state::State;


fn main() {
	pretty_env_logger::init();
	gotham::start("127.0.0.1:7879", || Ok(say_hello))
}

fn say_hello(state: State) -> (State, Result<String, HandlerError>) {
	let err = std::io::Error::new(std::io::ErrorKind::Other, "oh no!");
	(state, Err(err.into_handler_error()))
}

To be run with: RUST_LOG=debug cargo run

Using this patch the log shows:

 DEBUG gotham::handler::error      > [8b96448c-5a73-4c05-9ece-e26f95c5aa41] HandlerError generating 500 Internal Server Error response: (none)

Still shows (none) where the error should be; the same behavior as 0.4.0.

I'm no expert on this code base or implementing std:error::Error but I don't believe this line is right:

self.cause.source()

self.cause is the "source" of the HandlerError. So calling self.cause.source() actually returns the source of the source (which in my example usage is indeed None, hence the similar behavior).

My stab at it would be:

impl Error for HandlerError {
    fn description(&self) -> &str {
        "handler failed to process request"
    }

    fn cause(&self) -> Option<&dyn Error> {
        self.source()
    }

    fn source(&self) -> Option<&(dyn Error + 'static)> {
        Some(&*self.cause)
    }
}

Which appears to work correctly:

 DEBUG gotham::handler::error      > [4474b028-f506-4d12-a3ed-c8236f648d5d] HandlerError generating 500 Internal Server Error response: oh no!

Unless my assumptions are wrong, and the intention of source is to return the source of the source?

[whitfin]

@fpgaminer you might be right, but then I'm a little confused what the point is to having both of those exist in the trait. What if self.cause has a source that is not None? Should we ignore it?

Or should it be something along the lines of the following (written in a rush)?

self.cause.source().or_else(|| Some(&*self.cause))

[fpgaminer]

My understanding is that Error::source() and Error::cause() are supposed to be synonymous; exactly the same method. Error::source() only exists because the std devs had to patch a bug (users couldn't downcast the Error returned by cause() because it didn't specify a 'static lifetime). Beyond that, the intention of the method didn't change. The naming of source versus cause is not significant.

So as a user I would expect for all implementations of Error, some_error.source() == some_error.cause().

[nyarly]

Nudge, @fpgaminer and @whitfin - what's the status here?

[msrd0]

I think @fpgaminer is right - Error::cause() is marked deprecated in favour of Error::source(), so if we implement both, they should return the same result. Also I don't understand why this PR is making changes to the travis config.

[msrd0]

I believe this has been fixed in #438.