From 4f95162e133dafc29766e811e1ec583e7e61c9ac Mon Sep 17 00:00:00 2001 From: Victor Babenko Date: Wed, 29 Oct 2025 20:34:23 -0700 Subject: [PATCH 1/2] Fix HTTP 412 errors to be retried instead of returned to caller Modified error handling logic in read() and write() operations to treat HTTP 412 (Precondition Failed) responses as retryable errors. Previously, all 4xx errors were passed through to the caller without retry attempts. Now 412 errors will throw VaultException to trigger the retry mechanism, while other 4xx errors continue to be returned as-is for backward compatibility. This ensures that transient precondition failures can be automatically recovered through the existing retry logic. --- .../java/io/github/jopenlibs/vault/api/Logical.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/github/jopenlibs/vault/api/Logical.java b/src/main/java/io/github/jopenlibs/vault/api/Logical.java index a8a8977e..9a4bd129 100644 --- a/src/main/java/io/github/jopenlibs/vault/api/Logical.java +++ b/src/main/java/io/github/jopenlibs/vault/api/Logical.java @@ -98,9 +98,10 @@ private LogicalResponse read(final String path, final logicalOperations operatio .sslContext(config.getSslConfig().getSslContext()) .get(); - // Validate response - don't treat 4xx class errors as exceptions, we want to return an error as the response + // Validate response - don't treat 4xx class errors as exceptions (except 412), we want to return an error as the response + // 412 (Precondition Failed) should be retried if (restResponse.getStatus() != 200 && !(restResponse.getStatus() >= 400 - && restResponse.getStatus() < 500)) { + && restResponse.getStatus() < 500 && restResponse.getStatus() != 412)) { throw new VaultException( "Vault responded with HTTP status code: " + restResponse.getStatus() + "\nResponse body: " + new String(restResponse.getBody(), @@ -157,9 +158,10 @@ public LogicalResponse read(final String path, Boolean shouldRetry, final Intege .sslContext(config.getSslConfig().getSslContext()) .get(); - // Validate response - don't treat 4xx class errors as exceptions, we want to return an error as the response + // Validate response - don't treat 4xx class errors as exceptions (except 412), we want to return an error as the response + // 412 (Precondition Failed) should be retried if (restResponse.getStatus() != 200 && !(restResponse.getStatus() >= 400 - && restResponse.getStatus() < 500)) { + && restResponse.getStatus() < 500 && restResponse.getStatus() != 412)) { throw new VaultException( "Vault responded with HTTP status code: " + restResponse.getStatus() + "\nResponse body: " + new String(restResponse.getBody(), @@ -297,9 +299,10 @@ private LogicalResponse write(final String path, final Map nameV .post(); // HTTP Status should be either 200 (with content - e.g. PKI write) or 204 (no content) + // 412 (Precondition Failed) should be retried, so exclude it from the 4xx pass-through final int restStatus = restResponse.getStatus(); if (restStatus == 200 || restStatus == 204 || (restResponse.getStatus() >= 400 - && restResponse.getStatus() < 500)) { + && restResponse.getStatus() < 500 && restResponse.getStatus() != 412)) { return new LogicalResponse(restResponse, attempt, operation); } else { throw new VaultException( From 742c47e7dfb9e888be2eba9a54dc464a4da498e4 Mon Sep 17 00:00:00 2001 From: Victor Babenko Date: Wed, 29 Oct 2025 20:46:19 -0700 Subject: [PATCH 2/2] Add unit tests for HTTP 412 retry behavior and enhance RetriesMockVault Enhanced RetriesMockVault to accept configurable failure status codes, allowing it to simulate any HTTP error code for retry testing. This eliminates the need for separate mock classes for each error code. Added four new test cases to verify retry behavior: - testRetries_Read_412: Verifies read operations retry on 412 errors - testRetries_Write_412: Verifies write operations retry on 412 errors - testNoRetries_Read_404: Confirms 404 errors are not retried on reads - testNoRetries_Write_400: Confirms 400 errors are not retried on writes The tests confirm that 412 (Precondition Failed) triggers retry logic while other 4xx errors are returned to the caller without retries. --- .../io/github/jopenlibs/vault/RetryTests.java | 72 +++++++++++++++++++ .../vault/vault/mock/RetriesMockVault.java | 13 +++- 2 files changed, 82 insertions(+), 3 deletions(-) diff --git a/src/test/java/io/github/jopenlibs/vault/RetryTests.java b/src/test/java/io/github/jopenlibs/vault/RetryTests.java index d1f88d40..c160cd23 100644 --- a/src/test/java/io/github/jopenlibs/vault/RetryTests.java +++ b/src/test/java/io/github/jopenlibs/vault/RetryTests.java @@ -53,4 +53,76 @@ public void testRetries_Write() throws Exception { VaultTestUtils.shutdownMockVault(server); } + @Test + public void testRetries_Read_412() throws Exception { + final RetriesMockVault retriesMockVault = new RetriesMockVault(3, 412, 200, + "{\"lease_id\":\"12345\",\"renewable\":false,\"lease_duration\":10000,\"data\":{\"value\":\"mock\"}}"); + final Server server = VaultTestUtils.initHttpMockVault(retriesMockVault); + server.start(); + + final VaultConfig vaultConfig = new VaultConfig().address("http://127.0.0.1:8999") + .token("mock_token").engineVersion(1).build(); + final Vault vault = Vault.create(vaultConfig); + final LogicalResponse response = vault.withRetries(5, 100).logical().read("secret/hello"); + assertEquals(3, response.getRetries()); + assertEquals("mock", response.getData().get("value")); + + VaultTestUtils.shutdownMockVault(server); + } + + @Test + public void testRetries_Write_412() throws Exception { + final RetriesMockVault retriesMockVault = new RetriesMockVault(3, 412, 204, null); + final Server server = VaultTestUtils.initHttpMockVault(retriesMockVault); + server.start(); + + final VaultConfig vaultConfig = new VaultConfig().address("http://127.0.0.1:8999") + .token("mock_token").build(); + final Vault vault = Vault.create(vaultConfig); + final LogicalResponse response = vault.withRetries(5, 100).logical() + .write("secret/hello", new HashMap() {{ + put("value", "world"); + }}); + assertEquals(3, response.getRetries()); + + VaultTestUtils.shutdownMockVault(server); + } + + @Test + public void testNoRetries_Read_404() throws Exception { + final RetriesMockVault retriesMockVault = new RetriesMockVault(1, 404, 404, + "{\"errors\":[\"Not found\"]}"); + final Server server = VaultTestUtils.initHttpMockVault(retriesMockVault); + server.start(); + + final VaultConfig vaultConfig = new VaultConfig().address("http://127.0.0.1:8999") + .token("mock_token").engineVersion(1).build(); + final Vault vault = Vault.create(vaultConfig); + final LogicalResponse response = vault.withRetries(5, 100).logical().read("secret/hello"); + assertEquals(0, response.getRetries()); + assertEquals(404, response.getRestResponse().getStatus()); + + VaultTestUtils.shutdownMockVault(server); + } + + @Test + public void testNoRetries_Write_400() throws Exception { + final RetriesMockVault retriesMockVault = new RetriesMockVault(1, 400, 400, + "{\"errors\":[\"Bad request\"]}"); + final Server server = VaultTestUtils.initHttpMockVault(retriesMockVault); + server.start(); + + final VaultConfig vaultConfig = new VaultConfig().address("http://127.0.0.1:8999") + .token("mock_token").build(); + final Vault vault = Vault.create(vaultConfig); + final LogicalResponse response = vault.withRetries(5, 100).logical() + .write("secret/hello", new HashMap() {{ + put("value", "world"); + }}); + assertEquals(0, response.getRetries()); + assertEquals(400, response.getRestResponse().getStatus()); + + VaultTestUtils.shutdownMockVault(server); + } + } diff --git a/src/test/java/io/github/jopenlibs/vault/vault/mock/RetriesMockVault.java b/src/test/java/io/github/jopenlibs/vault/vault/mock/RetriesMockVault.java index e33fa72f..52596681 100644 --- a/src/test/java/io/github/jopenlibs/vault/vault/mock/RetriesMockVault.java +++ b/src/test/java/io/github/jopenlibs/vault/vault/mock/RetriesMockVault.java @@ -15,7 +15,7 @@ * *
    *
  1. - * RetriesMockVault responds with HTTP 500 status codes to a designated number of requests (which + * RetriesMockVault responds with a specified HTTP status code to a designated number of requests (which * can be zero). This can be used to test retry logic. *
    2. *
  2. @@ -46,11 +46,18 @@ public class RetriesMockVault extends MockVault { private final int mockStatus; private final String mockResponse; + private final int failureStatus; private int failureCount; public RetriesMockVault(final int failureCount, final int mockStatus, final String mockResponse) { + this(failureCount, 500, mockStatus, mockResponse); + } + + public RetriesMockVault(final int failureCount, final int failureStatus, + final int mockStatus, final String mockResponse) { this.failureCount = failureCount; + this.failureStatus = failureStatus; this.mockStatus = mockStatus; this.mockResponse = mockResponse; } @@ -66,8 +73,8 @@ public void handle( baseRequest.setHandled(true); if (failureCount > 0) { failureCount = failureCount - 1; - response.setStatus(500); - System.out.println("RetriesMockVault is sending an HTTP 500 code, to cause a retry..."); + response.setStatus(failureStatus); + System.out.println("RetriesMockVault is sending an HTTP " + failureStatus + " code, to trigger retry logic..."); } else { System.out.println("RetriesMockVault is sending an HTTP " + mockStatus + " code, with expected success payload...");