fix: update exception mapping on HTTP error responses (#1570)

* fix: update exception mapping on HTTP error responses.

Interprets HTTP error reponse codes as defined by the canonical error code mapping. Not marking as breaking changes as it affects only httpjson that is not GA-ed.

* chore: rename parameter

* chore: enhance test

Co-authored-by: Mike Eltsufin <meltsufin@google.com>

Author: chanseokoh

gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonExceptionCallable.java

@@ -95,7 +95,7 @@ public void onSuccess(ResponseT r) {
     public void onFailure(Throwable throwable) {
       if (throwable instanceof HttpResponseException) {
         HttpResponseException e = (HttpResponseException) throwable;
-        StatusCode statusCode = HttpJsonStatusCode.of(e.getStatusCode(), e.getMessage());
+        StatusCode statusCode = HttpJsonStatusCode.of(e.getStatusCode());
         boolean canRetry = retryableCodes.contains(statusCode.getCode());
         String message = e.getStatusMessage();
         ApiException newException =

gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshot.java

@@ -138,10 +138,9 @@ public Builder setResponse(Object response) {
       return this;
     }
 
-    public Builder setError(int errorCode, String errorMessage) {
+    public Builder setError(int httpStatus, String errorMessage) {
       this.errorCode =
-          HttpJsonStatusCode.of(
-              errorCode == 0 ? Code.OK.getHttpStatusCode() : errorCode, errorMessage);
+          httpStatus == 0 ? HttpJsonStatusCode.of(Code.OK) : HttpJsonStatusCode.of(httpStatus);
       this.errorMessage = errorMessage;
       return this;
     }

gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonStatusCode.java

@@ -32,26 +32,18 @@
 import com.google.api.core.BetaApi;
 import com.google.api.core.InternalExtensionOnly;
 import com.google.api.gax.rpc.StatusCode;
-import com.google.api.gax.rpc.StatusCode.Code;
-import com.google.common.base.Strings;
 import java.util.Objects;
 
 /** A failure code specific to an HTTP call. */
 @BetaApi
 @InternalExtensionOnly
 public class HttpJsonStatusCode implements StatusCode {
-  static final String FAILED_PRECONDITION = "FAILED_PRECONDITION";
-  static final String OUT_OF_RANGE = "OUT_OF_RANGE";
-  static final String ALREADY_EXISTS = "ALREADY_EXISTS";
-  static final String DATA_LOSS = "DATA_LOSS";
-  static final String UNKNOWN = "UNKNOWN";
-
   private final int httpStatus;
   private final Code statusCode;
 
   /** Creates a new instance with the given status code. */
-  public static HttpJsonStatusCode of(int httpStatus, String errorMessage) {
-    return new HttpJsonStatusCode(httpStatus, httpStatusToStatusCode(httpStatus, errorMessage));
+  public static HttpJsonStatusCode of(int httpStatus) {
+    return new HttpJsonStatusCode(httpStatus, httpStatusToStatusCode(httpStatus));
   }
 
   public static HttpJsonStatusCode of(Code statusCode) {
@@ -103,56 +95,43 @@ static Code rpcCodeToStatusCode(com.google.rpc.Code rpcCode) {
     }
   }
 
-  static Code httpStatusToStatusCode(int httpStatus, String errorMessage) {
-    String causeMessage = Strings.nullToEmpty(errorMessage).toUpperCase();
-    switch (httpStatus) {
-      case 200:
-        return Code.OK;
-      case 400:
-        if (causeMessage.contains(OUT_OF_RANGE)) {
-          return Code.OUT_OF_RANGE;
-        } else if (causeMessage.contains(FAILED_PRECONDITION)) {
-          return Code.FAILED_PRECONDITION;
-        } else {
+  static Code httpStatusToStatusCode(int httpStatus) {
+    if (200 <= httpStatus && httpStatus < 300) {
+      return Code.OK;
+    } else if (400 <= httpStatus && httpStatus < 500) {
+      switch (httpStatus) {
+        case 400:
           return Code.INVALID_ARGUMENT;
-        }
-      case 401:
-        return Code.UNAUTHENTICATED;
-      case 403:
-        return Code.PERMISSION_DENIED;
-      case 404:
-        return Code.NOT_FOUND;
-      case 409:
-        if (causeMessage.contains(ALREADY_EXISTS)) {
-          return Code.ALREADY_EXISTS;
-        } else {
+        case 401:
+          return Code.UNAUTHENTICATED;
+        case 403:
+          return Code.PERMISSION_DENIED;
+        case 404:
+          return Code.NOT_FOUND;
+        case 409:
           return Code.ABORTED;
-        }
-      case 411:
-        throw new IllegalStateException(
-            "411 status code received (Content-Length header not given.) Please file a bug against https://github.com/googleapis/gax-java/\n"
-                + httpStatus);
-      case 429:
-        return Code.RESOURCE_EXHAUSTED;
-      case 499:
-        return Code.CANCELLED;
-      case 500:
-        if (causeMessage.contains(DATA_LOSS)) {
-          return Code.DATA_LOSS;
-        } else if (causeMessage.contains(UNKNOWN)) {
-          return Code.UNKNOWN;
-        } else {
+        case 416:
+          return Code.OUT_OF_RANGE;
+        case 429:
+          return Code.RESOURCE_EXHAUSTED;
+        case 499:
+          return Code.CANCELLED;
+        default:
+          return Code.FAILED_PRECONDITION;
+      }
+    } else if (500 <= httpStatus && httpStatus < 600) {
+      switch (httpStatus) {
+        case 501:
+          return Code.UNIMPLEMENTED;
+        case 503:
+          return Code.UNAVAILABLE;
+        case 504:
+          return Code.DEADLINE_EXCEEDED;
+        default:
           return Code.INTERNAL;
-        }
-      case 501:
-        return Code.UNIMPLEMENTED;
-      case 503:
-        return Code.UNAVAILABLE;
-      case 504:
-        return Code.DEADLINE_EXCEEDED;
-      default:
-        throw new IllegalArgumentException("Unrecognized http status code: " + httpStatus);
+      }
     }
+    return Code.UNKNOWN;
   }
 
   @Override

gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshotTest.java

@@ -67,7 +67,7 @@ public void newBuilderTestWithError() {
             .build();
 
     assertEquals(testOperationSnapshot.getErrorMessage(), "Forbidden");
-    assertEquals(testOperationSnapshot.getErrorCode(), HttpJsonStatusCode.of(403, "Forbidden"));
+    assertEquals(testOperationSnapshot.getErrorCode(), HttpJsonStatusCode.of(403));
     assertTrue(testOperationSnapshot.isDone());
   }
 

gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonStatusCodeTest.java

@@ -29,12 +29,10 @@
  */
 package com.google.api.gax.httpjson;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
+import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.fail;
 
-import com.google.api.gax.rpc.StatusCode;
-import java.util.Arrays;
+import com.google.api.gax.rpc.StatusCode.Code;
 import java.util.HashSet;
 import java.util.Set;
 import org.junit.Test;
@@ -43,9 +41,9 @@ public class HttpJsonStatusCodeTest {
 
   @Test
   public void rpcCodeToStatusCodeTest() {
-    Set<StatusCode.Code> allCodes = new HashSet<>();
+    Set<Code> allCodes = new HashSet<>();
     for (com.google.rpc.Code rpcCode : com.google.rpc.Code.values()) {
-      StatusCode.Code statusCode;
+      Code statusCode;
       try {
         statusCode = HttpJsonStatusCode.rpcCodeToStatusCode(rpcCode);
       } catch (IllegalArgumentException e) {
@@ -55,77 +53,39 @@ public void rpcCodeToStatusCodeTest() {
         continue;
       }
 
-      assertNotNull(statusCode);
+      assertThat(statusCode).isNotNull();
       allCodes.add(statusCode);
     }
 
-    assertEquals(allCodes, new HashSet<>(Arrays.asList(StatusCode.Code.values())));
+    assertThat(Code.values()).asList().containsExactlyElementsIn(allCodes);
   }
 
   @Test
   public void httpStatusToStatusCodeTest() {
-    // The HTTP status code conversion logic is currently in the process of being standardized,
-    // the tested logic may change in nearest future.
-    final String defaultMessage = "anything";
-    assertEquals(
-        StatusCode.Code.OK, HttpJsonStatusCode.httpStatusToStatusCode(200, defaultMessage));
-    assertEquals(
-        StatusCode.Code.OUT_OF_RANGE,
-        HttpJsonStatusCode.httpStatusToStatusCode(400, HttpJsonStatusCode.OUT_OF_RANGE));
-    assertEquals(
-        StatusCode.Code.FAILED_PRECONDITION,
-        HttpJsonStatusCode.httpStatusToStatusCode(400, HttpJsonStatusCode.FAILED_PRECONDITION));
-    assertEquals(
-        StatusCode.Code.INVALID_ARGUMENT,
-        HttpJsonStatusCode.httpStatusToStatusCode(400, defaultMessage));
-    assertEquals(
-        StatusCode.Code.UNAUTHENTICATED,
-        HttpJsonStatusCode.httpStatusToStatusCode(401, defaultMessage));
-    assertEquals(
-        StatusCode.Code.PERMISSION_DENIED,
-        HttpJsonStatusCode.httpStatusToStatusCode(403, defaultMessage));
-    assertEquals(
-        StatusCode.Code.NOT_FOUND, HttpJsonStatusCode.httpStatusToStatusCode(404, defaultMessage));
-    assertEquals(
-        StatusCode.Code.ALREADY_EXISTS,
-        HttpJsonStatusCode.httpStatusToStatusCode(409, HttpJsonStatusCode.ALREADY_EXISTS));
-    assertEquals(
-        StatusCode.Code.ABORTED, HttpJsonStatusCode.httpStatusToStatusCode(409, defaultMessage));
-    assertEquals(
-        StatusCode.Code.RESOURCE_EXHAUSTED,
-        HttpJsonStatusCode.httpStatusToStatusCode(429, defaultMessage));
-    assertEquals(
-        StatusCode.Code.CANCELLED, HttpJsonStatusCode.httpStatusToStatusCode(499, defaultMessage));
-    assertEquals(
-        StatusCode.Code.DATA_LOSS,
-        HttpJsonStatusCode.httpStatusToStatusCode(500, HttpJsonStatusCode.DATA_LOSS));
-    assertEquals(
-        StatusCode.Code.UNKNOWN,
-        HttpJsonStatusCode.httpStatusToStatusCode(500, HttpJsonStatusCode.UNKNOWN));
-    assertEquals(
-        StatusCode.Code.INTERNAL, HttpJsonStatusCode.httpStatusToStatusCode(500, defaultMessage));
-    assertEquals(
-        StatusCode.Code.UNIMPLEMENTED,
-        HttpJsonStatusCode.httpStatusToStatusCode(501, defaultMessage));
-    assertEquals(
-        StatusCode.Code.UNAVAILABLE,
-        HttpJsonStatusCode.httpStatusToStatusCode(503, defaultMessage));
-    assertEquals(
-        StatusCode.Code.DEADLINE_EXCEEDED,
-        HttpJsonStatusCode.httpStatusToStatusCode(504, defaultMessage));
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(200)).isEqualTo(Code.OK);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(201)).isEqualTo(Code.OK);
 
-    try {
-      HttpJsonStatusCode.httpStatusToStatusCode(411, defaultMessage);
-      fail();
-    } catch (IllegalStateException e) {
-      // expected
-    }
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(400)).isEqualTo(Code.INVALID_ARGUMENT);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(401)).isEqualTo(Code.UNAUTHENTICATED);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(403)).isEqualTo(Code.PERMISSION_DENIED);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(404)).isEqualTo(Code.NOT_FOUND);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(409)).isEqualTo(Code.ABORTED);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(416)).isEqualTo(Code.OUT_OF_RANGE);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(429)).isEqualTo(Code.RESOURCE_EXHAUSTED);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(499)).isEqualTo(Code.CANCELLED);
 
-    try {
-      HttpJsonStatusCode.httpStatusToStatusCode(666, defaultMessage);
-      fail();
-    } catch (IllegalArgumentException e) {
-      // expected
-    }
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(405)).isEqualTo(Code.FAILED_PRECONDITION);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(408)).isEqualTo(Code.FAILED_PRECONDITION);
+
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(500)).isEqualTo(Code.INTERNAL);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(501)).isEqualTo(Code.UNIMPLEMENTED);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(502)).isEqualTo(Code.INTERNAL);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(503)).isEqualTo(Code.UNAVAILABLE);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(504)).isEqualTo(Code.DEADLINE_EXCEEDED);
+
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(100)).isEqualTo(Code.UNKNOWN);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(300)).isEqualTo(Code.UNKNOWN);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(302)).isEqualTo(Code.UNKNOWN);
+    assertThat(HttpJsonStatusCode.httpStatusToStatusCode(600)).isEqualTo(Code.UNKNOWN);
   }
 }