mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 09:25:06 +03:00
Kunal IngawaleandKaterina Skroumpelou
73260dfaba
## Problem [The built-in retry guide](https://supabase.com/docs/guides/api/automatic-retries-in-supabase-js) describes retry behaviour that `supabase-js` doesn't have. Three claims in the "Built-in retries for PostgREST queries" section don't match the code: | The page says | The code does | | --- | --- | | "POST requests (used by PostgREST) are retried" | POST is **never** retried | | Retries cover "408, 409, 503 and 504" | Only `503` and `520` | | "exponential backoff with jitter" | No jitter — the delay is deterministic | **The POST claim is the serious one.** It tells a reader that their writes are retried when they aren't, which invites exactly the wrong conclusion about how to handle a failed insert. Someone reading this page would reasonably skip their own retry or idempotency handling on writes, on the strength of a guarantee the library doesn't make. The source of truth, from `packages/core/postgrest-js/src/types/common/common.ts` on `supabase-js` master: ```ts export const RETRYABLE_STATUS_CODES = [520, 503] as const export const RETRYABLE_METHODS = ['GET', 'HEAD', 'OPTIONS'] as const export const DEFAULT_MAX_RETRIES = 3 export const getRetryDelay = (attemptIndex: number): number => Math.min(1000 * 2 ** attemptIndex, 30000) ``` `shouldRetry` in `packages/core/postgrest-js/src/fetchWithRetry.ts` gates on both constants, so a request is retried only when the method is in `RETRYABLE_METHODS` *and* the status is in `RETRYABLE_STATUS_CODES`. `getRetryDelay` is a pure function of the attempt index, with no random component — hence no jitter. There's a fourth, subtler consequence. The intro says built-in retries apply to `.from()` and `.rpc()`, but `.rpc()` sends POST unless you pass `{ get: true }` or `{ head: true }`, so RPC calls aren't retried by default. A reader who takes the intro at face value would expect retries on exactly the calls that don't get them. ## Solution Corrected the three claims in place. No restructuring, no new sections, no change to the `fetch-retry` half of the page. - **Methods.** Replaced the sentence claiming POST is retried with the actual rule, and stated the consequence plainly — a write is never sent twice. - **Status codes.** `503 Service Unavailable` and `520 Unknown Error` in place of 408, 409, 503 and 504. - **Jitter.** Dropped the word, since the backoff has none. - **`.rpc()`.** Added one sentence noting that RPC sends POST by default, so it isn't retried unless called with `{ get: true }`. The diff is 3 changed lines and 2 added, all in one paragraph group. This is a technical correction only — I deliberately left the page's style and structure alone, so the diff stays readable as a single change of one kind. ### Note on an incoming change supabase/supabase-js#2699 proposes adding `521`, `522`, `523` and `524` to `RETRYABLE_STATUS_CODES`. It is open, not merged. This PR documents what `master` does today, and if that one lands the status sentence here needs `520-524` rather than `520`. Happy to follow up with that change once it merges, or to fold it in if you'd rather wait and land both together. ## Review instructions 1. Open `packages/core/postgrest-js/src/types/common/common.ts` in `supabase/supabase-js` on `master` and read `RETRYABLE_STATUS_CODES` and `RETRYABLE_METHODS`. 2. Compare them against the live page's second paragraph. The status list and the method list both differ. 3. Read `shouldRetry` in `packages/core/postgrest-js/src/fetchWithRetry.ts` and confirm it returns `false` for any method outside `RETRYABLE_METHODS`, POST included. 4. Read `getRetryDelay` in the same `common.ts` and confirm there is no random component, so "with jitter" doesn't hold. 5. Check `rpc()` in `packages/core/postgrest-js/src/PostgrestClient.ts` and confirm the method is POST unless `get` or `head` is passed. 6. Read the preview page and confirm the corrected paragraphs say the same thing the code does. ## Checklist Check all before review: - [x] I have read [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) - [x] If I wrote a new docs topic or edited an existing topic, I used the `/write-the-docs` or `/edit-the-docs` skill, which references [WORD_LIST](https://github.com/supabase/supabase/blob/master/apps/docs/WORD_LIST.md) and the docs [CONTRIBUTING](https://github.com/supabase/supabase/blob/master/apps/docs/CONTRIBUTING.md) guide ## Additional context One suggestion, which I've deliberately left out of the diff because it's an addition rather than a correction — your call whether it belongs on this page. The guide sets no request timeout anywhere, in either the built-in section or the `fetch-retry` examples. That matters because a retry only fires once a request has failed. A request that is merely hanging never fails, so it never triggers a retry, and the caller waits for whatever the platform's own timeout turns out to be. This isn't hypothetical. During a Cloudflare edge incident on 22 September 2026, PostgREST calls from Edge Functions stalled for 20 to 60 seconds and returned `522`. Supabase Support confirmed the elevated 522s were platform-wide at the time rather than specific to one project. The built-in retry fired on none of them — partly because `522` isn't in the list, but also because a stalled request never reaches the retry check at all. A sentence pointing readers at `AbortSignal.timeout` alongside the retry would close that gap: ```javascript const { data, error } = await supabase .from('your_table') .select('*') .abortSignal(AbortSignal.timeout(10_000)) ``` Happy to write that up as a short subsection if you want it — tell me where you'd like it to sit and I'll open a separate PR so this correction stays reviewable on its own. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified that automatic retries apply only to idempotent GET, HEAD, and OPTIONS requests that encounter HTTP 503 or 520 responses, or network failures. * Documented that RPC calls use POST by default and are not retried, and that `{ get: true }` or `{ head: true }` can use retryable methods. * Added guidance for using `AbortSignal.timeout(10_000)` to limit requests that hang before retry handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Katerina Skroumpelou <sk.katherine@gmail.com>