Skip to content

Use retryable HTTP requests for Vault API#1375

Merged
hexedpackets merged 1 commit intomainfrom
vault-retries
Oct 29, 2025
Merged

Use retryable HTTP requests for Vault API#1375
hexedpackets merged 1 commit intomainfrom
vault-retries

Conversation

@hexedpackets
Copy link
Contributor

Description

By default API requests will immediately fail. FGA endpoints are the exception that use a wrapper to retry some failures. This adds Vault paths to the same retry logic. It also adds 408 response codes to the list of retryable codes - 408 is set by the SDK when catching/rethrowing a request timeout.

Documentation

Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.

[ ] Yes

If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.

@hexedpackets hexedpackets requested a review from a team as a code owner October 29, 2025 15:41
@hexedpackets hexedpackets requested a review from awolfden October 29, 2025 15:41
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

Extends retry logic to Vault API endpoints and adds 408 timeout status to retryable codes.

  • Centralized retry path detection into HttpClient.isPathRetryable() method that checks for /fga/ or /vault/ path prefixes
  • Added 408 (Request Timeout) to the list of retryable HTTP status codes alongside existing 500, 502, 504
  • Applied retry logic consistently across both FetchHttpClient and NodeHttpClient implementations
  • Comprehensive test coverage added for all Vault HTTP methods (GET, POST, PUT, DELETE)

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Clean refactoring with comprehensive test coverage, consistent implementation across both HTTP clients, and sensible addition of timeout handling to retry logic
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
src/common/net/http-client.ts 5/5 Added isPathRetryable helper method to centralize retry path logic for /fga/ and /vault/ endpoints, added 408 to retryable status codes
src/common/net/fetch-client.ts 5/5 Replaced hardcoded /fga/ checks with HttpClient.isPathRetryable() in GET, POST, PUT, DELETE methods
src/common/net/node-client.ts 5/5 Replaced hardcoded /fga/ checks with HttpClient.isPathRetryable() in GET, POST, PUT, DELETE methods

Sequence Diagram

sequenceDiagram
    participant Client
    participant HttpClient
    participant FetchClient
    participant API

    Client->>HttpClient: Request to /vault/v1/kv
    HttpClient->>HttpClient: isPathRetryable(path)
    Note over HttpClient: Returns true for /fga/ and /vault/
    
    alt Path is retryable
        HttpClient->>FetchClient: fetchRequestWithRetry()
        loop Up to 3 retry attempts
            FetchClient->>API: Make HTTP request
            alt Success (2xx)
                API-->>FetchClient: Response
                FetchClient-->>Client: Return response
            else Retryable error (408, 500, 502, 504, TypeError)
                API-->>FetchClient: Error response
                FetchClient->>FetchClient: sleep(backoff * jitter)
                Note over FetchClient: Backoff: 500ms * 1.5^attempt
            end
        end
        alt Max retries exceeded
            FetchClient-->>Client: Throw error
        end
    else Path not retryable
        HttpClient->>FetchClient: fetchRequest()
        FetchClient->>API: Make HTTP request (no retry)
        API-->>FetchClient: Response
        FetchClient-->>Client: Return response
    end
Loading

5 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@hexedpackets hexedpackets merged commit bfe17e3 into main Oct 29, 2025
5 checks passed
@hexedpackets hexedpackets deleted the vault-retries branch October 29, 2025 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants