Fix: Retry HTTP 412 errors to support eventual consistency on standbys #68
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
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.
vbabenkoru
changed the title
|
Thank you @henryx! Could you make a new release? Then we'll be able to switch back to upstream in our app. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[AI-generated but fully reviewed & edited by me]
Starting in Vault Enterprise v1.10, Vault returns
HTTP 412 (Precondition Failed)when a performance standby or performance replication secondary node doesn't have the necessary WAL index to ensure read-after-write consistency. According to the Vault Enterprise Consistency documentation, clients should automatically retry 412 responses to allow nodes to catch up with replication. The official Vault Go API handles this automatically.This Java driver was treating 412 like other 4xx errors and returning them to the caller without retry, occasionally returning errors in deployments with replication. This PR modifies error handling in
Logicalto treat HTTP 412 as a retryable error while preserving existing behavior for other 4xx codes (400-411, 413-499).Added unit tests to verify 412 errors are retried for read and write operations while other 4xx errors maintain backward-compatible behavior. This change aligns the Java driver with the official Vault Go API and ensures proper support for Vault Enterprise eventual consistency.