From 2877a2fe0f8ff6a5cd98e05a726630ff44eb7c5e Mon Sep 17 00:00:00 2001 From: Niko Dziemba Date: Wed, 11 May 2022 11:06:21 +0200 Subject: [PATCH] Add HTTP method to RequestNotOkException Adding the HTTP method to the failure exception allows for better visibility into which call exactly failed. We already had the HTTP path in the exception but there are usually multiple HTTP methods that are valid for a single path. --- .../github/v3/clients/GitHubClient.java | 4 +-- .../github/v3/clients/RepositoryClient.java | 4 +-- .../ReadOnlyRepositoryException.java | 10 ++++--- .../v3/exceptions/RequestNotOkException.java | 26 ++++++++++++++----- .../github/opencensus/OpenCensusSpanTest.java | 4 +-- .../github/v3/clients/GitHubClientTest.java | 4 +++ 6 files changed, 36 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/spotify/github/v3/clients/GitHubClient.java b/src/main/java/com/spotify/github/v3/clients/GitHubClient.java index 40170820..9a5b6a62 100644 --- a/src/main/java/com/spotify/github/v3/clients/GitHubClient.java +++ b/src/main/java/com/spotify/github/v3/clients/GitHubClient.java @@ -741,10 +741,10 @@ private RequestNotOkException mapException(final Response res, final Request req String bodyString = res.body() != null ? res.body().string() : ""; if (res.code() == FORBIDDEN) { if (bodyString.contains("Repository was archived so is read-only")) { - return new ReadOnlyRepositoryException(request.url().encodedPath(), res.code(), bodyString); + return new ReadOnlyRepositoryException(request.method(), request.url().encodedPath(), res.code(), bodyString); } } - return new RequestNotOkException(request.url().encodedPath(), res.code(), bodyString); + return new RequestNotOkException(request.method(), request.url().encodedPath(), res.code(), bodyString); } CompletableFuture processPossibleRedirects( diff --git a/src/main/java/com/spotify/github/v3/clients/RepositoryClient.java b/src/main/java/com/spotify/github/v3/clients/RepositoryClient.java index 74fe1808..9727d4e9 100644 --- a/src/main/java/com/spotify/github/v3/clients/RepositoryClient.java +++ b/src/main/java/com/spotify/github/v3/clients/RepositoryClient.java @@ -7,9 +7,9 @@ * 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. diff --git a/src/main/java/com/spotify/github/v3/exceptions/ReadOnlyRepositoryException.java b/src/main/java/com/spotify/github/v3/exceptions/ReadOnlyRepositoryException.java index 29769561..91919091 100644 --- a/src/main/java/com/spotify/github/v3/exceptions/ReadOnlyRepositoryException.java +++ b/src/main/java/com/spotify/github/v3/exceptions/ReadOnlyRepositoryException.java @@ -7,9 +7,9 @@ * 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. @@ -25,11 +25,13 @@ public class ReadOnlyRepositoryException extends RequestNotOkException { /** * Instantiates a new Read only repository exception. * + * @param method HTTP method * @param path the path * @param statusCode the status code * @param msg the msg */ - public ReadOnlyRepositoryException(final String path, final int statusCode, final String msg) { - super(path, statusCode, msg); + public ReadOnlyRepositoryException( + final String method, final String path, final int statusCode, final String msg) { + super(method, path, statusCode, msg); } } diff --git a/src/main/java/com/spotify/github/v3/exceptions/RequestNotOkException.java b/src/main/java/com/spotify/github/v3/exceptions/RequestNotOkException.java index d3ac8674..c5852a54 100644 --- a/src/main/java/com/spotify/github/v3/exceptions/RequestNotOkException.java +++ b/src/main/java/com/spotify/github/v3/exceptions/RequestNotOkException.java @@ -7,9 +7,9 @@ * 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. @@ -38,23 +38,28 @@ public class RequestNotOkException extends GithubException { private final int statusCode; + private final String method; private final String path; private final String msg; - private static String decoratedMessage(final String path, final int statusCode, final String msg) { - return String.format("%s %d: %s", path, statusCode, msg); + private static String decoratedMessage( + final String method, final String path, final int statusCode, final String msg) { + return String.format("%s %s %d: %s", method, path, statusCode, msg); } /** * Response to request came back with non-2xx status code * + * @param method HTTP method * @param path URI path * @param statusCode status of repsonse * @param msg response body */ - public RequestNotOkException(final String path, final int statusCode, final String msg) { - super(decoratedMessage(path, statusCode, msg)); + public RequestNotOkException( + final String method, final String path, final int statusCode, final String msg) { + super(decoratedMessage(method, path, statusCode, msg)); this.statusCode = statusCode; + this.method = method; this.path = path; this.msg = msg; } @@ -77,6 +82,15 @@ public int statusCode() { return statusCode; } + /** + * Get request HTTP method + * + * @return method + */ + public String method() { + return method; + } + /** * Get request URI path * diff --git a/src/test/java/com/spotify/github/opencensus/OpenCensusSpanTest.java b/src/test/java/com/spotify/github/opencensus/OpenCensusSpanTest.java index 59428b7e..73f9c354 100644 --- a/src/test/java/com/spotify/github/opencensus/OpenCensusSpanTest.java +++ b/src/test/java/com/spotify/github/opencensus/OpenCensusSpanTest.java @@ -46,7 +46,7 @@ public void succeed() { @Test public void fail() { final Span span = new OpenCensusSpan(wrapped); - span.failure(new RequestNotOkException("path", 404, "Not found")); + span.failure(new RequestNotOkException("method", "path", 404, "Not found")); span.close(); verify(wrapped).setStatus(Status.UNKNOWN); @@ -57,7 +57,7 @@ public void fail() { @Test public void failOnServerError() { final Span span = new OpenCensusSpan(wrapped); - span.failure(new RequestNotOkException("path", 500, "Internal Server Error")); + span.failure(new RequestNotOkException("method", "path", 500, "Internal Server Error")); span.close(); verify(wrapped).setStatus(Status.UNKNOWN); diff --git a/src/test/java/com/spotify/github/v3/clients/GitHubClientTest.java b/src/test/java/com/spotify/github/v3/clients/GitHubClientTest.java index fc1e5a2c..57eba069 100644 --- a/src/test/java/com/spotify/github/v3/clients/GitHubClientTest.java +++ b/src/test/java/com/spotify/github/v3/clients/GitHubClientTest.java @@ -130,6 +130,10 @@ public void testRequestNotOkException() throws Throwable { assertThat(e.getCause() instanceof RequestNotOkException, is(true)); RequestNotOkException e1 = (RequestNotOkException) e.getCause(); assertThat(e1.statusCode(), is(409)); + assertThat(e1.method(), is("POST")); + assertThat(e1.path(), is("/repos/testorg/testrepo/merges")); + assertThat(e1.getMessage(), containsString("POST")); + assertThat(e1.getMessage(), containsString("/repos/testorg/testrepo/merges")); assertThat(e1.getMessage(), containsString("Merge Conflict")); assertThat(e1.getRawMessage(), containsString("Merge Conflict")); }