diff --git a/internal/handlers/get_state_workaround.go b/internal/handlers/get_state_workaround.go index 1519b05..35ec84f 100644 --- a/internal/handlers/get_state_workaround.go +++ b/internal/handlers/get_state_workaround.go @@ -11,7 +11,6 @@ import ( "time" "github.com/steemit/jussi/internal/request" - "github.com/steemit/jussi/internal/upstream" "github.com/steemit/jussi/internal/urn" ) @@ -628,14 +627,7 @@ func (p *RequestProcessor) callSteemd( } headers := originalReq.UpstreamHeaders() - retryCfg := &upstream.RetryConfig{ - MaxRetries: 1, - InitialBackoff: 100 * time.Millisecond, - MaxBackoff: 1 * time.Second, - BackoffMultiplier: 2.0, - } - - return p.httpClient.RequestWithRetry(subCtx, upstreamURL, payload, headers, retryCfg) + return p.httpClient.Request(subCtx, upstreamURL, payload, headers) } // deepCopyMap creates a deep copy of a map[string]interface{} by diff --git a/internal/handlers/processor.go b/internal/handlers/processor.go index d6f2a29..9621ddd 100644 --- a/internal/handlers/processor.go +++ b/internal/handlers/processor.go @@ -328,12 +328,16 @@ func (p *RequestProcessor) callHTTPUpstream(ctx context.Context, jsonrpcReq *req headers := jsonrpcReq.UpstreamHeaders() // Create timeout context + // A timeout of 0 in the upstream config means "use default". + // Previously 0 meant "no timeout" which could cause requests to + // hang indefinitely (e.g. broadcast_transaction_synchronous). timeout := time.Duration(jsonrpcReq.Upstream.Timeout) * time.Second - if timeout > 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, timeout) - defer cancel() + if timeout == 0 { + timeout = 3 * time.Second // safe default for all requests } + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, timeout) + defer cancel() return p.httpClient.Request(ctx, url, payload, headers) } diff --git a/internal/upstream/http.go b/internal/upstream/http.go index 248046e..966dc7f 100644 --- a/internal/upstream/http.go +++ b/internal/upstream/http.go @@ -34,24 +34,12 @@ func NewHTTPClient() *HTTPClient { } } -// Request sends an HTTP POST request to upstream +// Request sends an HTTP POST request to upstream (fail-fast, no retry). +// As a proxy/routing layer, jussi should return errors immediately and +// let the caller decide whether to retry. Retrying at the proxy level +// is dangerous for non-idempotent requests (e.g. broadcast_transaction) +// and adds latency for all requests. func (c *HTTPClient) Request(ctx context.Context, url string, payload map[string]interface{}, headers map[string]string) (map[string]interface{}, error) { - return c.RequestWithRetry(ctx, url, payload, headers, nil) -} - -// RequestWithRetry sends an HTTP POST request to upstream with retry logic -func (c *HTTPClient) RequestWithRetry(ctx context.Context, url string, payload map[string]interface{}, headers map[string]string, retryConfig *RetryConfig) (map[string]interface{}, error) { - if retryConfig == nil { - retryConfig = DefaultRetryConfig() - } - - return RetryWithResult(ctx, retryConfig, func() (map[string]interface{}, error) { - return c.doRequest(ctx, url, payload, headers) - }) -} - -// doRequest performs a single HTTP request -func (c *HTTPClient) doRequest(ctx context.Context, url string, payload map[string]interface{}, headers map[string]string) (map[string]interface{}, error) { // Marshal payload body, err := json.Marshal(payload) if err != nil { @@ -80,13 +68,13 @@ func (c *HTTPClient) doRequest(ctx context.Context, url string, payload map[stri // Send request resp, err := c.client.Do(req) if err != nil { - return nil, &RetryableError{Err: fmt.Errorf("request failed: %w", err)} + return nil, fmt.Errorf("upstream request failed: %w", err) } defer resp.Body.Close() - // Check for retryable status codes + // Check for server errors if resp.StatusCode >= 500 { - return nil, &RetryableError{Err: fmt.Errorf("server error: %d", resp.StatusCode)} + return nil, fmt.Errorf("upstream server error: %d", resp.StatusCode) } // Read response diff --git a/internal/upstream/retry.go b/internal/upstream/retry.go deleted file mode 100644 index 60eab33..0000000 --- a/internal/upstream/retry.go +++ /dev/null @@ -1,150 +0,0 @@ -package upstream - -import ( - "context" - "fmt" - "math" - "strings" - "time" -) - -// RetryConfig holds retry configuration -type RetryConfig struct { - MaxRetries int - InitialBackoff time.Duration - MaxBackoff time.Duration - BackoffMultiplier float64 -} - -// DefaultRetryConfig returns default retry configuration -func DefaultRetryConfig() *RetryConfig { - return &RetryConfig{ - MaxRetries: 3, - InitialBackoff: 100 * time.Millisecond, - MaxBackoff: 5 * time.Second, - BackoffMultiplier: 2.0, - } -} - -// RetryableError indicates an error that can be retried -type RetryableError struct { - Err error -} - -func (e *RetryableError) Error() string { - return fmt.Sprintf("retryable error: %v", e.Err) -} - -func (e *RetryableError) Unwrap() error { - return e.Err -} - -// IsRetryableError checks if an error is retryable -func IsRetryableError(err error) bool { - if err == nil { - return false - } - - // Network errors, timeouts, and 5xx errors are retryable - errStr := err.Error() - retryablePatterns := []string{ - "timeout", - "connection refused", - "connection reset", - "no such host", - "network is unreachable", - "temporary failure", - "502", - "503", - "504", - } - - for _, pattern := range retryablePatterns { - if contains(errStr, pattern) { - return true - } - } - - return false -} - -// Retry executes a function with retry logic -func Retry(ctx context.Context, config *RetryConfig, fn func() error) error { - var lastErr error - - for attempt := 0; attempt <= config.MaxRetries; attempt++ { - if attempt > 0 { - // Calculate backoff - backoff := time.Duration(float64(config.InitialBackoff) * math.Pow(config.BackoffMultiplier, float64(attempt-1))) - if backoff > config.MaxBackoff { - backoff = config.MaxBackoff - } - - // Wait before retry - select { - case <-ctx.Done(): - return ctx.Err() - case <-time.After(backoff): - } - } - - // Execute function - err := fn() - if err == nil { - return nil - } - - lastErr = err - - // Check if error is retryable - if !IsRetryableError(err) { - return err - } - } - - return fmt.Errorf("max retries exceeded: %w", lastErr) -} - -// RetryWithResult executes a function with retry logic and returns a result -func RetryWithResult[T any](ctx context.Context, config *RetryConfig, fn func() (T, error)) (T, error) { - var zero T - var lastErr error - - for attempt := 0; attempt <= config.MaxRetries; attempt++ { - if attempt > 0 { - // Calculate backoff - backoff := time.Duration(float64(config.InitialBackoff) * math.Pow(config.BackoffMultiplier, float64(attempt-1))) - if backoff > config.MaxBackoff { - backoff = config.MaxBackoff - } - - // Wait before retry - select { - case <-ctx.Done(): - return zero, ctx.Err() - case <-time.After(backoff): - } - } - - // Execute function - result, err := fn() - if err == nil { - return result, nil - } - - lastErr = err - - // Check if error is retryable - if !IsRetryableError(err) { - return zero, err - } - } - - return zero, fmt.Errorf("max retries exceeded: %w", lastErr) -} - -// contains checks if a string contains a substring (case-insensitive) -func contains(s, substr string) bool { - return strings.Contains(strings.ToLower(s), strings.ToLower(substr)) -} -