Skip to content
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

Having to unwrap exceptions from ExecutionException #111

Open
maciej opened this issue Jun 2, 2015 · 2 comments
Open

Having to unwrap exceptions from ExecutionException #111

maciej opened this issue Jun 2, 2015 · 2 comments

Comments

@maciej
Copy link

maciej commented Jun 2, 2015

(It's not strictly a bug, more of an API issue :-) )

Let's take the example code from the homepage and modify it slightly to get a 404 response from housetop.info.

import dispatch._, Defaults._
val svc = url("http://api.hostip.info/country.php-NOT-FOUND")
val country = Http(svc OK as.String)
country.recover { case e => println(e.getClass.getCanonicalName) } 

The last line will eventually print java.util.concurrent.ExecutionException. To get to the actual error (StatusCode) I first need to unwrap it.
It would seem intuitive that for a common use case like a non-OK status code returned from the HTTP server the future would fail with StatusCode straight away, which is not the case.

I'm using dispatch 0.11.2.

@farmdawgnation
Copy link
Member

This seems like a reasonable ask. Given that Dispatch 0.12 and 0.13 have already frozen their APIs, I think the earliest we'd be able to address this is Dispatch 0.14. Before I commit to it though, I want the Development of AHC 2.1 to settle down, since I'm also angling to get that into Dispatch 0.14.

@farmdawgnation farmdawgnation modified the milestone: 0.14.0 Jul 3, 2017
farmdawgnation added a commit that referenced this issue Jul 5, 2017
The execution exception itself isn't a terribly useful measure of what actually happened under the
hood. As a result, #111 proposed unwrapping them before finishing the future. I'm still not sure
what I think of this idea, but it was straightforward enough to implement and we have some time
before 0.14.x is declared final.
@farmdawgnation
Copy link
Member

I'm having second thoughts about this change.

The uniformity of knowing that you'll get an ExecutionException has some nice features for error handling, imo.

I'm going to continue considering this, but bump it from the 0.14.0 milestone.

@farmdawgnation farmdawgnation removed this from the 0.14.0 milestone Jul 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants