Made not treating 400 class status codes as errors the default behavior

Author: Barry Klawans

src/main/java/com/bettercloud/vault/VaultConfig.java

@@ -40,7 +40,6 @@ public class VaultConfig implements Serializable {
     private int retryIntervalMilliseconds;
     private Integer globalEngineVersion;
     private String nameSpace;
-    private Boolean treatInvalidRequestsAsErrors = true;
     private EnvironmentLoader environmentLoader;
 
     /**
@@ -208,26 +207,6 @@ public VaultConfig readTimeout(final Integer readTimeout) {
         return this;
     }
 
-    /**
-     * <p>Specifies whether 4xx class status codes should be treated as errors or invalid calls.</p>
-     *
-     * <p>If 4xx status codes are treated as errors, any call that returns them will retry (if specified)
-     * and will result in an exception being thrown.  If disabled, a 4xx return code does not retry, does
-     * not generate an exception, and instead returns the error in the response body</p>
-     *
-     * <p>If you set this to <code>false</code> you need to check the response code before using the data
-     * in a response</p>
-     *
-     * <p>The default behavior is to treat 4xx status codes as errors.</p>
-     *
-     * @param treatInvalidRequestsAsErrors Should 4xx class status codes be treated as errors.
-     * @return This object, with treatInvalidRequestsAsErrors populated, ready for additional builder-pattern method calls or else finalization with the build() method
-     */
-    public VaultConfig treatInvalidRequestsAsErrors(final Boolean treatInvalidRequestsAsErrors) {
-        this.treatInvalidRequestsAsErrors = treatInvalidRequestsAsErrors;
-        return this;
-    }
-
     /**
      * <p>Sets the maximum number of times that an API operation will retry upon failure.</p>
      *
@@ -335,10 +314,6 @@ public Integer getReadTimeout() {
         return readTimeout;
     }
 
-    public Boolean isTreatInvalidRequestsAsErrors() {
-        return treatInvalidRequestsAsErrors;
-    }
-
     public int getMaxRetries() {
         return maxRetries;
     }

src/main/java/com/bettercloud/vault/api/Logical.java

@@ -91,8 +91,8 @@ private LogicalResponse read(final String path, Boolean shouldRetry, final logic
                         .sslContext(config.getSslConfig().getSslContext())
                         .get();
 
-                // Validate response
-                if (restResponse.getStatus() != 200 && !(!config.isTreatInvalidRequestsAsErrors() && restResponse.getStatus() >= 400 && restResponse.getStatus() < 500)) {
+                // Validate response - don't treat 4xx class errors as exceptions, we want to return an error as the response
+                if (restResponse.getStatus() != 200 && !(restResponse.getStatus() >= 400 && restResponse.getStatus() < 500)) {
                     throw new VaultException("Vault responded with HTTP status code: " + restResponse.getStatus()
                             + "\nResponse body: " + new String(restResponse.getBody(), StandardCharsets.UTF_8),
                             restResponse.getStatus());
@@ -160,8 +160,8 @@ public LogicalResponse read(final String path, Boolean shouldRetry, final Intege
                         .sslContext(config.getSslConfig().getSslContext())
                         .get();
 
-                // Validate response
-                if (restResponse.getStatus() != 200 && !(!config.isTreatInvalidRequestsAsErrors() && restResponse.getStatus() >= 400 && restResponse.getStatus() < 500)) {
+                // Validate response - don't treat 4xx class errors as exceptions, we want to return an error as the response
+                if (restResponse.getStatus() != 200 && !(restResponse.getStatus() >= 400 && restResponse.getStatus() < 500)) {
                     throw new VaultException("Vault responded with HTTP status code: " + restResponse.getStatus()
                             + "\nResponse body: " + new String(restResponse.getBody(), StandardCharsets.UTF_8),
                             restResponse.getStatus());
@@ -261,7 +261,7 @@ private LogicalResponse write(final String path, final Map<String, Object> nameV
 
                 // HTTP Status should be either 200 (with content - e.g. PKI write) or 204 (no content)
                 final int restStatus = restResponse.getStatus();
-                if (restStatus == 200 || restStatus == 204 || (!config.isTreatInvalidRequestsAsErrors() && restResponse.getStatus() >= 400 && restResponse.getStatus() < 500)) {
+                if (restStatus == 200 || restStatus == 204 || (restResponse.getStatus() >= 400 && restResponse.getStatus() < 500)) {
                     return new LogicalResponse(restResponse, retryCount, operation);
                 } else {
                     throw new VaultException("Expecting HTTP status 204 or 200, but instead receiving " + restStatus

src/test/java/com/bettercloud/vault/VaultConfigTests.java

@@ -255,13 +255,4 @@ public void testConfigBuilder_WithNamespace() throws VaultException {
         VaultConfig vaultConfig = new VaultConfig().nameSpace("namespace").address("address").build();
         Assert.assertEquals(vaultConfig.getNameSpace(), "namespace");
     }
-
-    @Test
-    public void testConfigBuiler_WithInvalidRequestAsNonError() throws VaultException {
-        VaultConfig vaultConfig = new VaultConfig().address("address").token("token").treatInvalidRequestsAsErrors(false).build();
-        assertEquals("address", vaultConfig.getAddress());
-        assertEquals("token", vaultConfig.getToken());
-        assertEquals(Boolean.FALSE, vaultConfig.isTreatInvalidRequestsAsErrors());
-
-    }
 }

src/test/java/com/bettercloud/vault/VaultTests.java

@@ -1,11 +1,17 @@
 package com.bettercloud.vault;
 
+import com.bettercloud.vault.response.LogicalResponse;
+import com.bettercloud.vault.vault.VaultTestUtils;
+import com.bettercloud.vault.vault.mock.MockVault;
+import org.eclipse.jetty.server.Server;
 import org.junit.Assert;
 import org.junit.Test;
 
 import java.util.HashMap;
 import java.util.Map;
 
+import static junit.framework.TestCase.assertEquals;
+
 
 /**
  * Unit tests for the various <code>Vault</code> constructors.
@@ -88,4 +94,22 @@ public void kvEngineMapIsHonored() throws VaultException {
         Assert.assertEquals(String.valueOf(1), vault.logical().getEngineVersionForSecretPath("kv-v1").toString());
         Assert.assertEquals(String.valueOf(2), vault.logical().getEngineVersionForSecretPath("notInMap").toString());
     }
+
+    @Test
+    public void testConfigBuiler_WithInvalidRequestAsNonError() throws Exception {
+        final MockVault mockVault = new MockVault(403, "{\"errors\":[\"preflight capability check returned 403, please ensure client's policies grant access to path \"path/that/does/not/exist/\"]}");
+        final Server server = VaultTestUtils.initHttpMockVault(mockVault);
+        server.start();
+
+        final VaultConfig vaultConfig = new VaultConfig()
+                .address("http://127.0.0.1:8999")
+                .token("mock_token")
+                .build();
+        final Vault vault = new Vault(vaultConfig);
+
+        LogicalResponse response = vault.logical().read("path/that/does/not/exist/");
+        VaultTestUtils.shutdownMockVault(server);
+        Assert.assertEquals(403, response.getRestResponse().getStatus());
+        Assert.assertEquals(0, response.getRetries());
+    }
 }