mirror of
https://github.com/supabase/supabase.git
synced 2026-05-06 17:00:27 -04:00
45fc609471
Preserve `formattedError` through the `ResponseError` path and fall back to splitting `error.message` on newlines so enhanced permission-denied HINTs from supabase/postgres#2084 render as separate lines in the SQL editor — users can actually read the GRANT example now. **Context:** postgres#2084 adds a multi-line HINT to SQLSTATE 42501 errors, telling users exactly how to grant access per-table. Today the SQL editor rendered the whole thing on one line because `formattedError` was stripped by the fetchers' error handling and the `message` fallback didn't split on `\n`. This PR fixes both. Blocks [FE-3023](https://linear.app/supabase/issue/FE-3023) — the project-creation toggle that flips default privileges; without readable HINTs users land on RLS debugging rabbit holes when they hit a permission denied. **Changed:** - `ResponseError` now carries an optional `formattedError` field; `ConnectionTimeoutError` / `UnknownAPIResponseError` thread it through. - `handleError` in `data/fetchers.ts` extracts `formattedError` from the raw error body and forwards it to the thrown subclass. - `UtilityTabResults.tsx` uses a new `getSqlErrorLines` helper — prefers `formattedError`, falls back to splitting `message` on newlines when it's multi-line (defense in depth since the exact field pg-meta populates for the HINT depends on the path). Copy button now uses the same lines. **Added:** - `getSqlErrorLines` pure helper + 9 unit tests. - 5 new tests in `handleError.test.ts` covering `formattedError` preservation on classified and unclassified errors. ## To test 1. Pull the branch, run `pnpm dev:studio`, open any project's SQL editor. 2. Run a query that triggers the enhanced HINT (requires postgres#2084 deployed on the DB — currently staging-only). Example: `select * from some_table_you_cant_read;` as a role without grants. 3. Expect the ERROR line, HINT line, and the `GRANT ...` example to each render on their own `<pre>` line, plus the Copy button to copy the full multi-line text. 4. Sanity check existing single-line errors (e.g. `select * from nonexistent_table`) still render as `Error: relation "nonexistent_table" does not exist` in the `<p>` fallback. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Improved SQL error message formatting in the editor for better readability and clarity. * **Refactor** * Centralized error formatting logic for more consistent error presentation across the application. * **Tests** * Added comprehensive test coverage for SQL error message parsing and formatting. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Alaister Young <10985857+alaister@users.noreply.github.com>
134 lines
5.1 KiB
TypeScript
134 lines
5.1 KiB
TypeScript
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
|
|
|
import { ConnectionTimeoutError, UnknownAPIResponseError } from '@/types/api-errors'
|
|
import { ResponseError } from '@/types/base'
|
|
|
|
vi.mock('@sentry/nextjs', () => ({ captureException: vi.fn() }))
|
|
vi.mock('common', () => ({ IS_PLATFORM: false, getAccessToken: vi.fn() }))
|
|
vi.mock('@/lib/constants', () => ({ API_URL: 'http://localhost' }))
|
|
vi.mock('@/lib/helpers', () => ({ uuidv4: () => 'test-uuid' }))
|
|
|
|
// Import after mocks are set up
|
|
const { handleError } = await import('./fetchers')
|
|
|
|
function throwAndCatch(error: unknown): ResponseError {
|
|
try {
|
|
handleError(error)
|
|
} catch (e) {
|
|
return e as ResponseError
|
|
}
|
|
throw new Error('handleError did not throw')
|
|
}
|
|
|
|
describe('handleError — error classification', () => {
|
|
beforeEach(() => vi.clearAllMocks())
|
|
|
|
describe('known patterns', () => {
|
|
it('classifies connection timeout via message field', () => {
|
|
const err = throwAndCatch({ message: 'CONNECTION TERMINATED DUE TO CONNECTION TIMEOUT' })
|
|
expect(err).toBeInstanceOf(ConnectionTimeoutError)
|
|
expect((err as ConnectionTimeoutError).errorType).toBe('connection-timeout')
|
|
})
|
|
|
|
it('classifies connection timeout via msg field', () => {
|
|
const err = throwAndCatch({ msg: 'ERROR: CONNECTION TERMINATED DUE TO CONNECTION TIMEOUT' })
|
|
expect(err).toBeInstanceOf(ConnectionTimeoutError)
|
|
})
|
|
|
|
it('classification is case-insensitive', () => {
|
|
const err = throwAndCatch({ message: 'connection terminated due to connection timeout' })
|
|
expect(err).toBeInstanceOf(ConnectionTimeoutError)
|
|
})
|
|
|
|
it('classified error is still instanceof ResponseError', () => {
|
|
const err = throwAndCatch({ message: 'CONNECTION TERMINATED DUE TO CONNECTION TIMEOUT' })
|
|
expect(err).toBeInstanceOf(ResponseError)
|
|
})
|
|
})
|
|
|
|
describe('unclassified errors', () => {
|
|
it('throws UnknownAPIResponseError for unmatched messages', () => {
|
|
const err = throwAndCatch({ message: 'something went wrong' })
|
|
expect(err).toBeInstanceOf(UnknownAPIResponseError)
|
|
expect(err).toBeInstanceOf(ResponseError)
|
|
})
|
|
|
|
it('throws UnknownAPIResponseError for empty message', () => {
|
|
const err = throwAndCatch({ message: '' })
|
|
expect(err).toBeInstanceOf(UnknownAPIResponseError)
|
|
})
|
|
|
|
it('throws UnknownAPIResponseError for null', () => {
|
|
const err = throwAndCatch(null)
|
|
expect(err).toBeInstanceOf(UnknownAPIResponseError)
|
|
})
|
|
|
|
it('throws UnknownAPIResponseError for non-object', () => {
|
|
const err = throwAndCatch('raw string error')
|
|
expect(err).toBeInstanceOf(UnknownAPIResponseError)
|
|
})
|
|
})
|
|
|
|
describe('field preservation', () => {
|
|
it('preserves all ResponseError fields on classified errors', () => {
|
|
const err = throwAndCatch({
|
|
message: 'CONNECTION TERMINATED DUE TO CONNECTION TIMEOUT',
|
|
code: 503,
|
|
requestId: 'req-abc',
|
|
retryAfter: 30,
|
|
requestPathname: '/rest/v1/table',
|
|
})
|
|
expect(err.message).toBe('CONNECTION TERMINATED DUE TO CONNECTION TIMEOUT')
|
|
expect(err.code).toBe(503)
|
|
expect(err.requestId).toBe('req-abc')
|
|
expect(err.retryAfter).toBe(30)
|
|
expect(err.requestPathname).toBe('/rest/v1/table')
|
|
})
|
|
|
|
it('preserves all ResponseError fields on unclassified errors', () => {
|
|
const err = throwAndCatch({ message: 'some error', code: 500, requestId: 'req-xyz' })
|
|
expect(err.code).toBe(500)
|
|
expect(err.requestId).toBe('req-xyz')
|
|
})
|
|
|
|
it('msg field takes priority over message field for error text', () => {
|
|
const err = throwAndCatch({ msg: 'from msg field', message: 'from message field' })
|
|
expect(err.message).toBe('from msg field')
|
|
})
|
|
|
|
it('preserves formattedError on classified errors', () => {
|
|
const err = throwAndCatch({
|
|
message: 'CONNECTION TERMINATED DUE TO CONNECTION TIMEOUT',
|
|
formattedError: 'ERROR: 08000: CONNECTION TERMINATED\nHINT: retry later',
|
|
})
|
|
expect(err).toBeInstanceOf(ConnectionTimeoutError)
|
|
expect(err.formattedError).toBe('ERROR: 08000: CONNECTION TERMINATED\nHINT: retry later')
|
|
})
|
|
|
|
it('preserves formattedError on unclassified errors (permission denied with HINT)', () => {
|
|
const err = throwAndCatch({
|
|
message: 'permission denied for table users',
|
|
code: 400,
|
|
formattedError:
|
|
'ERROR: 42501: permission denied for table users\n' +
|
|
'HINT: To grant access to anon on a specific table:\n' +
|
|
' GRANT SELECT ON TABLE public.users TO anon;',
|
|
})
|
|
expect(err).toBeInstanceOf(UnknownAPIResponseError)
|
|
expect(err.formattedError).toContain('ERROR:')
|
|
expect(err.formattedError).toContain('HINT:')
|
|
expect(err.formattedError?.split('\n').length).toBeGreaterThan(1)
|
|
})
|
|
|
|
it('leaves formattedError undefined when raw error omits it', () => {
|
|
const err = throwAndCatch({ message: 'some error' })
|
|
expect(err.formattedError).toBeUndefined()
|
|
})
|
|
|
|
it('ignores non-string formattedError values', () => {
|
|
const err = throwAndCatch({ message: 'some error', formattedError: 42 })
|
|
expect(err.formattedError).toBeUndefined()
|
|
})
|
|
})
|
|
})
|