mirror of
https://github.com/supabase/supabase.git
synced 2026-10-12 12:55:06 +03:00
<!-- ccr-slack-attribution --> _Requested by **Ivan Vasilov** · [Slack thread](https://supabase.slack.com/archives/C063LNYJJKS/p1790710416069689?thread_ts=1790710416.069689&cid=C063LNYJJKS)_ **Before:** When a Studio API GET comes back as a 200 with an empty body, openapi-fetch hands the caller `{}` and we only see the downstream crash, with no record of the response that caused it. **After:** The first time this happens for an endpoint in a page session, Studio sends one Sentry warning, `Empty response body on successful API request`. It carries the response metadata, browser state, resource timing, and the result of a single `cache: 'no-store'` refetch. What the caller receives is unchanged. ## Problem Studio crashes trace back to GET requests that return 200 with an empty body, which openapi-fetch turns into `{}`. They are heavily skewed to Firefox and Safari. The leading hypothesis is browser cache revalidation (Express weak ETags, no `Cache-Control` on api.supabase.com), but nothing confirms it yet. The `no-store` probe tells the two cases apart: if the refetch has a body, the browser cache is the likely culprit; if it is also empty, the server or the edge is sending empty bodies. This data should show whether the fix belongs on the API side or the Cloudflare side. Context: #51041 (closed) tried to guard the crashing call sites instead. ## Needs API-side change to be fully useful Cross-origin, Studio can only read CORS-safelisted response headers, and resource timing sizes read as zero. If api.supabase.com sends `Access-Control-Expose-Headers: ETag, cf-ray, cf-cache-status, x-request-id` and `Timing-Allow-Origin: <studio origin>`, this event will also carry the ETag, cf-ray, and cache status, plus the real transfer and body sizes and the negotiated protocol. Until then, those fields read as `null` or `0`. ## Solution - `data/empty-body-diagnostics.ts` (new): `reportEmptyBodyResponse({ request, response, schemaPath })`. - Runs only for `GET` and only when `IS_PLATFORM`. Empty POST/201 bodies are legitimate. - Reports at most once per templated endpoint per page session (module-level `Set`). - Endpoint: openapi-fetch's `schemaPath` (e.g. `/platform/projects/{ref}/settings`), passed through `templateEndpointPath`. That function drops the query string and hash, replaces the segment after `projects`/`organizations`/`branches` with `{ref}`/`{slug}`/`{branch}`, and replaces UUIDs, numeric IDs, and 20+ character alphanumeric IDs with `{id}`. I used `schemaPath` rather than the request URL so user-chosen names (bucket names, function slugs) never end up in tags or fingerprints. - Probe: one plain `fetch(new Request(request, { cache: 'no-store', ... }))` with a fresh `X-Request-Id` and a 10s `AbortController` timeout. `AbortSignal.timeout` isn't available in older Safari. The probe bypasses the openapi-fetch middleware, so it can't recurse. Only the body's byte length is recorded, never its contents. - Event: `level: 'warning'`, `fingerprint: ['empty-body-response', endpoint]`, `tags: { endpoint, probe_has_body, empty_body_diagnostic: 'true' }`, where `probe_has_body` is `true` / `false` / `error`. `extra` holds: - the request: method, status, `response.type`, `redirected`, and the original `X-Request-Id` (for API log lookup) - response headers: `content-type`, `cache-control`, `last-modified`, `expires`, `content-length`, `etag`, `cf-ray`, `cf-cache-status`, `x-request-id` - browser state: `visibilityState`, `navigator.onLine`, the navigation type, ms since navigation start, and whether the page was restored from bfcache - the latest `PerformanceResourceTiming` for the URL (transfer, encoded, and decoded size, `nextHopProtocol`, `responseStatus`) - the probe: status, request ID, body length, `content-length`, `content-type`, or the error name - Fire-and-forget: everything is wrapped in a `try`/`catch`, and the caller does not await it. - `data/fetchers.ts`: the `onResponse` middleware passes `{ request, schemaPath }` to `normalizeEmptyBodyResponse`, which calls the reporter in its empty-body branch and also for a 200 that carries `Content-Length: 0`. openapi-fetch short-circuits that case to `{}` the same way, so it is the same symptom. The return value is unchanged in every branch. - `packages/common/sentry.ts`: `filterSentryEvent` normally keeps only 1% of events that aren't page crashes. It now sends events tagged `empty_body_diagnostic` unsampled, with `codeSampleRate: '1'`. A once-per-session warning would barely show up at 1%. Consent and platform gating and the third-party filter still apply. www and docs also use `filterSentryEvent`, but only Studio's reporter sets this tag, so sampling for them and for every other Studio event is unchanged. Sentry config: Studio's `beforeSend` doesn't otherwise drop this message. It has no exception values, so the no-stack-trace filter doesn't apply, and it matches no `ignoreErrors` entry. ## Review instructions 1. Check `normalizeEmptyBodyResponse` in `data/fetchers.ts`: the reporter is `void`-called and its return value is untouched. 2. Check `probe()` in `data/empty-body-diagnostics.ts`: only `byteLength` is read from the body. The probe reuses the original request's headers and credentials (same auth as the original GET). 3. Check `filterSentryEvent` in `packages/common/sentry.ts`: only the `empty_body_diagnostic` tag skips sampling. 4. Tests: `data/empty-body-diagnostics.test.ts` and `packages/common/sentry.test.ts`. ## Verification - Unit tests (`data/empty-body-diagnostics.test.ts`, new): - path templating cases - `probe_has_body` `true` / `false` / `error` - the secret body content never appears in the Sentry call - one report per endpoint - non-GET and non-platform requests are skipped - no throw when `fetch` or Sentry throws - end-to-end through `client.GET`: still resolves `{}` and reports the `schemaPath`, for both a missing `Content-Length` and `Content-Length: 0` - `packages/common/sentry.test.ts`: tagged diagnostics are sent unsampled, untagged or false-tagged ones are still sampled, and they're still dropped without consent. These tests, plus the existing `normalizeEmptyBodyResponse.test.ts`, `handleError.test.ts`, and the rest of `sentry.test.ts`, pass (67 tests) under vitest 5 + jsdom. I ran them in a minimal harness, not the full `pnpm install` workspace, because the local checkout is sparse. - I ran TypeScript 7.0.2 (`--strict`) on the five touched files against the real `api-types`, with stubbed `common`/Sentry types. No errors in the touched files. - Prettier `--check` with the repo config passes, with and without `SORT_IMPORTS=false`. - Not run locally: the full studio typecheck, `lint:ratchet`, and knip. CI covers them. The change adds no `any`, no default exports, and no deps. ## Checklist - [x] I have read [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01UefDak8XYLMi9aiEjDPXc5 --------- Co-authored-by: Claude <noreply@anthropic.com>
239 lines
8.2 KiB
TypeScript
239 lines
8.2 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
|
|
|
const platform = vi.hoisted(() => ({ isPlatform: true }))
|
|
const sentry = vi.hoisted(() => ({ captureMessage: vi.fn(), captureException: vi.fn() }))
|
|
|
|
vi.mock('@sentry/nextjs', () => sentry)
|
|
vi.mock('common', () => ({
|
|
get IS_PLATFORM() {
|
|
return platform.isPlatform
|
|
},
|
|
getAccessToken: vi.fn(),
|
|
}))
|
|
vi.mock('@/lib/constants', () => ({ API_URL: 'http://localhost' }))
|
|
vi.mock('@/lib/helpers', () => ({ uuidv4: () => 'probe-uuid' }))
|
|
|
|
// Import after mocks are set up
|
|
const { reportEmptyBodyResponse, templateEndpointPath } = await import('./empty-body-diagnostics')
|
|
const { client } = await import('./fetchers')
|
|
|
|
const SECRET = 'service_role_secret_value'
|
|
|
|
function emptyGet(path: string, init?: RequestInit) {
|
|
const request = new Request(`http://localhost${path}?include=secret`, {
|
|
headers: { 'X-Request-Id': 'original-uuid', Authorization: 'Bearer token' },
|
|
...init,
|
|
})
|
|
const response = new Response(null, {
|
|
status: 200,
|
|
headers: { 'Content-Type': 'application/json', 'Cache-Control': 'private' },
|
|
})
|
|
return { request, response }
|
|
}
|
|
|
|
function lastCapture() {
|
|
const [message, context] = sentry.captureMessage.mock.lastCall ?? []
|
|
return { message, context }
|
|
}
|
|
|
|
describe('templateEndpointPath', () => {
|
|
it.each([
|
|
['/platform/projects/abcdefghijklmnopqrst/settings', '/platform/projects/{ref}/settings'],
|
|
['/platform/organizations/my-org/members', '/platform/organizations/{slug}/members'],
|
|
[
|
|
'/v1/projects/abcdefghijklmnopqrst/branches/dev/config',
|
|
'/v1/projects/{ref}/branches/{branch}/config',
|
|
],
|
|
['/platform/pg-meta/abcdefghijklmnopqrst/tables', '/platform/pg-meta/{id}/tables'],
|
|
[
|
|
'/platform/organizations/{slug}/members/3f2b8c1e-9d4a-4e6b-8c7d-1a2b3c4d5e6f',
|
|
'/platform/organizations/{slug}/members/{id}',
|
|
],
|
|
['/platform/projects/{ref}/backups/12345', '/platform/projects/{ref}/backups/{id}'],
|
|
['/platform/projects/{ref}/api-keys?reveal=true', '/platform/projects/{ref}/api-keys'],
|
|
['/platform/profile#section', '/platform/profile'],
|
|
['/platform/projects/{ref}/custom-hostname', '/platform/projects/{ref}/custom-hostname'],
|
|
['/platform/projects', '/platform/projects'],
|
|
['/platform/constructor/toString', '/platform/constructor/toString'],
|
|
['', ''],
|
|
])('templates %s as %s', (path, expected) => {
|
|
expect(templateEndpointPath(path)).toBe(expected)
|
|
})
|
|
})
|
|
|
|
describe('reportEmptyBodyResponse', () => {
|
|
beforeEach(() => {
|
|
platform.isPlatform = true
|
|
sentry.captureMessage.mockReset()
|
|
})
|
|
afterEach(() => vi.unstubAllGlobals())
|
|
|
|
it('reports with probe_has_body "true" when a no-store refetch returns a body', async () => {
|
|
const fetchMock = vi.fn<(request: Request) => Promise<Response>>(
|
|
async () => new Response(JSON.stringify({ secret: SECRET }))
|
|
)
|
|
vi.stubGlobal('fetch', fetchMock)
|
|
const { request, response } = emptyGet('/platform/projects/abcdefghijklmnopqrst/a')
|
|
|
|
await reportEmptyBodyResponse({ request, response, schemaPath: '/platform/projects/{ref}/a' })
|
|
|
|
const probeRequest = fetchMock.mock.lastCall?.[0]
|
|
expect(probeRequest).toBeInstanceOf(Request)
|
|
expect(probeRequest?.cache).toBe('no-store')
|
|
expect(probeRequest?.headers.get('X-Request-Id')).toBe('probe-uuid')
|
|
|
|
const { message, context } = lastCapture()
|
|
expect(message).toBe('Empty response body on successful API request')
|
|
expect(context).toMatchObject({
|
|
level: 'warning',
|
|
fingerprint: ['empty-body-response', '/platform/projects/{ref}/a'],
|
|
tags: {
|
|
endpoint: '/platform/projects/{ref}/a',
|
|
probe_has_body: 'true',
|
|
empty_body_diagnostic: 'true',
|
|
},
|
|
extra: {
|
|
method: 'GET',
|
|
status: 200,
|
|
request_id: 'original-uuid',
|
|
probe_request_id: 'probe-uuid',
|
|
header_content_type: 'application/json',
|
|
header_cache_control: 'private',
|
|
header_etag: null,
|
|
probe_status: 200,
|
|
probe_body_length: JSON.stringify({ secret: SECRET }).length,
|
|
},
|
|
})
|
|
expect(JSON.stringify(sentry.captureMessage.mock.calls)).not.toContain(SECRET)
|
|
})
|
|
|
|
it('reports probe_has_body "false" when the refetch is also empty', async () => {
|
|
vi.stubGlobal(
|
|
'fetch',
|
|
vi.fn(async () => new Response(null))
|
|
)
|
|
const { request, response } = emptyGet('/platform/b')
|
|
|
|
await reportEmptyBodyResponse({ request, response, schemaPath: '/platform/b' })
|
|
|
|
expect(lastCapture().context).toMatchObject({
|
|
tags: { probe_has_body: 'false' },
|
|
extra: { probe_body_length: 0 },
|
|
})
|
|
})
|
|
|
|
it('reports probe_has_body "error" and does not throw when the refetch rejects', async () => {
|
|
vi.stubGlobal(
|
|
'fetch',
|
|
vi.fn(async () => Promise.reject(new TypeError('Load failed')))
|
|
)
|
|
const { request, response } = emptyGet('/platform/c')
|
|
|
|
await expect(
|
|
reportEmptyBodyResponse({ request, response, schemaPath: '/platform/c' })
|
|
).resolves.toBeUndefined()
|
|
|
|
expect(lastCapture().context).toMatchObject({
|
|
tags: { probe_has_body: 'error' },
|
|
extra: { probe_error: 'TypeError' },
|
|
})
|
|
})
|
|
|
|
it('reports at most once per endpoint', async () => {
|
|
vi.stubGlobal(
|
|
'fetch',
|
|
vi.fn(async () => new Response(null))
|
|
)
|
|
|
|
for (const ref of ['abcdefghijklmnopqrst', 'tsrqponmlkjihgfedcba']) {
|
|
const { request, response } = emptyGet(`/platform/projects/${ref}/d`)
|
|
await reportEmptyBodyResponse({ request, response, schemaPath: '/platform/projects/{ref}/d' })
|
|
}
|
|
|
|
expect(sentry.captureMessage).toHaveBeenCalledTimes(1)
|
|
})
|
|
|
|
it('skips non-GET requests', async () => {
|
|
const fetchMock = vi.fn(async () => new Response(null))
|
|
vi.stubGlobal('fetch', fetchMock)
|
|
const { request, response } = emptyGet('/platform/e', { method: 'POST' })
|
|
|
|
await reportEmptyBodyResponse({ request, response, schemaPath: '/platform/e' })
|
|
|
|
expect(fetchMock).not.toHaveBeenCalled()
|
|
expect(sentry.captureMessage).not.toHaveBeenCalled()
|
|
})
|
|
|
|
it('skips when not on platform', async () => {
|
|
platform.isPlatform = false
|
|
const fetchMock = vi.fn(async () => new Response(null))
|
|
vi.stubGlobal('fetch', fetchMock)
|
|
const { request, response } = emptyGet('/platform/f')
|
|
|
|
await reportEmptyBodyResponse({ request, response, schemaPath: '/platform/f' })
|
|
|
|
expect(fetchMock).not.toHaveBeenCalled()
|
|
expect(sentry.captureMessage).not.toHaveBeenCalled()
|
|
})
|
|
|
|
it('does not throw when Sentry throws', async () => {
|
|
vi.spyOn(console, 'error').mockImplementation(() => {})
|
|
vi.stubGlobal(
|
|
'fetch',
|
|
vi.fn(async () => new Response(null))
|
|
)
|
|
sentry.captureMessage.mockImplementation(() => {
|
|
throw new Error('sentry down')
|
|
})
|
|
const { request, response } = emptyGet('/platform/g')
|
|
|
|
await expect(
|
|
reportEmptyBodyResponse({ request, response, schemaPath: '/platform/g' })
|
|
).resolves.toBeUndefined()
|
|
})
|
|
})
|
|
|
|
describe('openapi-fetch client — empty GET 200', () => {
|
|
beforeEach(() => {
|
|
platform.isPlatform = true
|
|
sentry.captureMessage.mockReset()
|
|
})
|
|
afterEach(() => vi.unstubAllGlobals())
|
|
|
|
it('still resolves with {} and reports the templated schema path', async () => {
|
|
vi.stubGlobal(
|
|
'fetch',
|
|
vi.fn(async () => new Response(null, { status: 200 }))
|
|
)
|
|
|
|
const { data, error } = await client.GET('/platform/projects/{ref}/settings', {
|
|
params: { path: { ref: 'abcdefghijklmnopqrst' } },
|
|
})
|
|
|
|
expect(error).toBeUndefined()
|
|
expect(data).toEqual({})
|
|
await vi.waitFor(() => expect(sentry.captureMessage).toHaveBeenCalledTimes(1))
|
|
expect(lastCapture().context).toMatchObject({
|
|
tags: { endpoint: '/platform/projects/{ref}/settings' },
|
|
})
|
|
})
|
|
|
|
it('reports an empty GET 200 that carries Content-Length: 0', async () => {
|
|
vi.stubGlobal(
|
|
'fetch',
|
|
vi.fn(async () => new Response(null, { status: 200, headers: { 'Content-Length': '0' } }))
|
|
)
|
|
|
|
const { data } = await client.GET('/platform/organizations/{slug}/members', {
|
|
params: { path: { slug: 'my-org' } },
|
|
})
|
|
|
|
expect(data).toEqual({})
|
|
await vi.waitFor(() => expect(sentry.captureMessage).toHaveBeenCalledTimes(1))
|
|
expect(lastCapture().context).toMatchObject({
|
|
tags: { endpoint: '/platform/organizations/{slug}/members' },
|
|
extra: { header_content_length: '0' },
|
|
})
|
|
})
|
|
})
|