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

Add fine-grained HTTP error exceptions #825

Merged
merged 1 commit into from
Nov 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 136 additions & 1 deletion core/src/main/java/feign/FeignException.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,46 @@ public static FeignException errorStatus(String methodKey, Response response) {
} catch (IOException ignored) { // NOPMD
}

return new FeignException(response.status(), message, body);
return errorStatus(response.status(), message, body);
}

private static FeignException errorStatus(int status, String message, byte[] body) {
switch (status) {
case 400:
return new BadRequest(message, body);
case 401:
return new Unauthorized(message, body);
case 403:
return new Forbidden(message, body);
case 404:
return new NotFound(message, body);
case 405:
return new MethodNotAllowed(message, body);
case 406:
return new NotAcceptable(message, body);
case 409:
return new Conflict(message, body);
case 410:
return new Gone(message, body);
case 415:
return new UnsupportedMediaType(message, body);
case 429:
return new TooManyRequests(message, body);
case 422:
return new UnprocessableEntity(message, body);
case 500:
return new InternalServerError(message, body);
case 501:
return new NotImplemented(message, body);
case 502:
return new BadGateway(message, body);
case 503:
return new ServiceUnavailable(message, body);
case 504:
return new GatewayTimeout(message, body);
default:
return new FeignException(status, message, body);
}
}

static FeignException errorExecuting(Request request, IOException cause) {
Expand All @@ -85,4 +124,100 @@ static FeignException errorExecuting(Request request, IOException cause) {
cause,
null);
}

public static class BadRequest extends FeignException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like all the 4xx ones should extend some sort of 4xx exception (say FiengClientException) and the same with 5xx errors (maybe FeignServerException).

public BadRequest(String message, byte[] body) {
super(400, message, body);
}
}

public static class Unauthorized extends FeignException {
public Unauthorized(String message, byte[] body) {
super(401, message, body);
}
}

public static class Forbidden extends FeignException {
public Forbidden(String message, byte[] body) {
super(403, message, body);
}
}

public static class NotFound extends FeignException {
public NotFound(String message, byte[] body) {
super(404, message, body);
}
}

public static class MethodNotAllowed extends FeignException {
public MethodNotAllowed(String message, byte[] body) {
super(405, message, body);
}
}

public static class NotAcceptable extends FeignException {
public NotAcceptable(String message, byte[] body) {
super(406, message, body);
}
}

public static class Conflict extends FeignException {
public Conflict(String message, byte[] body) {
super(409, message, body);
}
}

public static class Gone extends FeignException {
public Gone(String message, byte[] body) {
super(410, message, body);
}
}

public static class UnsupportedMediaType extends FeignException {
public UnsupportedMediaType(String message, byte[] body) {
super(415, message, body);
}
}

public static class TooManyRequests extends FeignException {
public TooManyRequests(String message, byte[] body) {
super(429, message, body);
}
}

public static class UnprocessableEntity extends FeignException {
public UnprocessableEntity(String message, byte[] body) {
super(422, message, body);
}
}

public static class InternalServerError extends FeignException {
public InternalServerError(String message, byte[] body) {
super(500, message, body);
}
}

public static class NotImplemented extends FeignException {
public NotImplemented(String message, byte[] body) {
super(501, message, body);
}
}

public static class BadGateway extends FeignException {
public BadGateway(String message, byte[] body) {
super(502, message, body);
}
}

public static class ServiceUnavailable extends FeignException {
public ServiceUnavailable(String message, byte[] body) {
super(503, message, body);
}
}

public static class GatewayTimeout extends FeignException {
public GatewayTimeout(String message, byte[] body) {
super(504, message, body);
}
}
}
19 changes: 19 additions & 0 deletions core/src/test/java/feign/FeignBuilderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,22 @@ public void testUrlPathConcatNoPathOnRequestLine() throws Exception {
assertThat(server.takeRequest()).hasPath("/");
}

@Test
public void testHttpNotFoundError() {
server.enqueue(new MockResponse().setResponseCode(404));

String url = "http://localhost:" + server.getPort() + "/";
TestInterface api = Feign.builder().target(TestInterface.class, url);

try {
api.getBodyAsString();
failBecauseExceptionWasNotThrown(FeignException.class);
} catch (FeignException.NotFound e) {
assertThat(e.status()).isEqualTo(404);
}

}

@Test
public void testUrlPathConcatNoInitialSlashOnPath() throws Exception {
server.enqueue(new MockResponse().setBody("response data"));
Expand Down Expand Up @@ -433,6 +449,9 @@ interface TestInterface {
@RequestLine("GET api/thing")
Response getNoInitialSlashOnSlash();

@RequestLine("GET api/thing")
String getBodyAsString();

@RequestLine(value = "GET /api/querymap/object")
String queryMapEncoded(@QueryMap Object object);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/**
* Copyright 2012-2018 The Feign Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
package feign.codec;

import feign.FeignException;
import feign.Request;
import feign.Request.HttpMethod;
import feign.Response;
import feign.Util;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import java.util.Collection;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Map;
import static org.assertj.core.api.Assertions.assertThat;

@RunWith(Parameterized.class)
public class DefaultErrorDecoderHttpErrorTest {

@Parameterized.Parameters(name = "error: [{0}], exception: [{1}]")
public static Object[][] errorCodes() {
return new Object[][] {
{400, FeignException.BadRequest.class},
{401, FeignException.Unauthorized.class},
{403, FeignException.Forbidden.class},
{404, FeignException.NotFound.class},
{405, FeignException.MethodNotAllowed.class},
{406, FeignException.NotAcceptable.class},
{409, FeignException.Conflict.class},
{429, FeignException.TooManyRequests.class},
{422, FeignException.UnprocessableEntity.class},
{500, FeignException.InternalServerError.class},
{501, FeignException.NotImplemented.class},
{502, FeignException.BadGateway.class},
{503, FeignException.ServiceUnavailable.class},
{504, FeignException.GatewayTimeout.class},
{599, FeignException.class},
};
}

@Parameterized.Parameter
public int httpStatus;

@Parameterized.Parameter(1)
public Class expectedExceptionClass;

private ErrorDecoder errorDecoder = new ErrorDecoder.Default();

private Map<String, Collection<String>> headers = new LinkedHashMap<>();

@Test
public void testExceptionIsHttpSpecific() throws Throwable {
Response response = Response.builder()
.status(httpStatus)
.reason("anything")
.request(Request.create(HttpMethod.GET, "/api", Collections.emptyMap(), null, Util.UTF_8))
.headers(headers)
.build();

Exception exception = errorDecoder.decode("Service#foo()", response);

assertThat(exception).isInstanceOf(expectedExceptionClass);
assertThat(((FeignException) exception).status()).isEqualTo(httpStatus);
}

}