Add HTTP method to RequestNotOkException (#98)

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.

Author: dziemba

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<Response> processPossibleRedirects(

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.

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);
   }
 }

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
    *

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);

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"));
     }