feat(postgrest): add automatic retries for transient errors#2072
feat(postgrest): add automatic retries for transient errors#2072
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/core/postgrest-js/src/PostgrestBuilder.ts`:
- Around line 193-244: The retry loop in executeWithRetry currently retries on
520 responses without consuming the response body and also may retry on
AbortError; update executeWithRetry so that when shouldRetry(...) returns true
you first fully drain the response body (e.g., await res.arrayBuffer() or
res.text() to consume streams) before sleeping/retrying, and in the catch block
immediately rethrow if fetchError.name === 'AbortError' (do not treat as
retryable), otherwise keep existing logic using RETRYABLE_METHODS,
DEFAULT_MAX_RETRIES, getRetryDelay, sleep, and continue calling _fetch and
processResponse as before.
🧹 Nitpick comments (3)
packages/core/postgrest-js/test/retry.test.ts (1)
210-235: Consider a more robust approach for verifying backoff delays.The current implementation spies on
setTimeoutand immediately executes callbacks withdelay=0. While functional, this approach bypasses the actual timing behavior and could mask issues if the retry implementation changes how it schedules delays.A more robust alternative would use
vi.getTimerCount()or advance timers by specific amounts and verify fetch call counts at each interval.packages/core/postgrest-js/src/PostgrestQueryBuilder.ts (2)
24-25: Add JSDoc documentation for theretryproperty.The
retryproperty is part of the public API but lacks JSDoc documentation. Other public properties in this class could benefit from similar documentation, but at minimum the newretryproperty should be documented to explain its purpose and default behavior.📝 Suggested documentation
+ /** + * Enable or disable automatic retries for transient errors. + * When enabled, idempotent requests (GET/HEAD/OPTIONS) that fail with 520 errors + * will be automatically retried with exponential backoff. + */ // Retry configuration retry?: booleanAs per coding guidelines: "Use JSDoc comments for all public APIs in each library".
40-59: Addretryparameter to the constructor's JSDoc.The constructor example and parameter documentation should include the new
retryoption for completeness.📝 Suggested documentation update
/** * Creates a query builder scoped to a Postgres table or view. * * `@example` * ```ts * import PostgrestQueryBuilder from '@supabase/postgrest-js' * * const query = new PostgrestQueryBuilder( * new URL('https://xyzcompany.supabase.co/rest/v1/users'), - * { headers: { apikey: 'public-anon-key' } } + * { headers: { apikey: 'public-anon-key' }, retry: true } * ) * ``` + * + * `@param` url - The URL for the query + * `@param` options - Named parameters + * `@param` options.headers - Custom headers + * `@param` options.schema - Postgres schema + * `@param` options.fetch - Custom fetch function + * `@param` options.retry - Enable automatic retries for transient errors */
Implement automatic retry logic for PostgREST SDK requests that fail with 520 status codes (Cloudflare timeout/connection errors). Key features: - Enabled by default for GET/HEAD/OPTIONS requests - 3 retry attempts with exponential backoff (1s, 2s, 4s) - X-Retry-Count header sent for observability - Can be disabled globally or per-request via .retry(false) This addresses customer feedback about perceived reliability issues when transient 520 errors occur under load. Related: https://linear.app/supabase/project/automatic-retries-for-postgrest-sdk-c6fa4eaf163a Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
3734cba to
8c9359d
Compare
@supabase/auth-js
@supabase/functions-js
@supabase/postgrest-js
@supabase/realtime-js
@supabase/storage-js
@supabase/supabase-js
commit: |
8c9359d to
63fd316
Compare
274617c to
7f6bc9b
Compare
The lack of retry header is concerning, without a retry header it will be too hard to debug at the origin server. Can that be added in this PR? |
Summary
Implements automatic retry logic for
@supabase/postgrest-jsto handle transient errors transparently.Problem
Two classes of transient errors cause unnecessary failures:
Solution
Usage
Key Design Decisions
Test plan
.retry(false)overrides global default.retry(true)overrides globalretry: false🤖 Generated with Claude Code