Kong / unirest-java

Unirest in Java: Simplified, lightweight HTTP client library.
http://kong.github.io/unirest-java/
MIT License
2.61k stars 593 forks source link

"Connection Refused" should be caught in "iffailure" chain #327

Closed codeswayslay closed 4 years ago

codeswayslay commented 4 years ago

A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] Typically, this is how I would write my request.

Unirest.get("http://localhost:8080/mydata")
                .asJson()
                .ifSuccess(response -> someSuccessMethod(response))
                .ifFailure(response -> {
                    log.error("Oh No! Status" + response.getStatus());
                    response.getParsingError().ifPresent(e -> {
                        log.error("Parsing Exception: ", e);
                        log.error("Original body: " + e.getOriginalBody());
                    });
                });

However, I've noticed that, in the event that there is a "connection refused" exception, probably because the service on 8080 isn't running, or 8080 isn't open, the exception isn't caught in the iffailure chain.

I expect this should happen. Plus, it keeps our codes cleaner. The alternative is to start wrapping the whole thing in try-catch, which can be very messy.

ryber commented 4 years ago

We do have a solution to this problem but it might not be immediately apparent. a little background first:

Unirest has traditionally always thrown request failures as UnirestExceptions. I imagine there is already lots of code out there with Unirest in a try/catch so changing that by default would be a breaking change (maybe not a terrible one but one nonetheless)

ifFailure processes a failed HttpResponse, in the case of a connection failure we don't have a response, we have a failed request. We could construct a failed response that that would be a lie or we can just pass null but that gives you no information. So...

Unirest has an interceptor starting in 3.2.00 that can handle a failed request. You can choose to either return null, throw (the default) or return a FailedResponse which would then also be processed in ifFailure.

I've considered exposing config options for these with the default interceptor but right now you need to configure your own.

see https://github.com/Kong/unirest-java/blob/master/unirest/src/test/java/BehaviorTests/InterceptorTest.java#L170-L175

codeswayslay commented 4 years ago

You're absolutely right. I just went through the post you put up and tested it myself. Works like a charm. Thanks!