mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 09:25:06 +03:00
[FE-3034] feat(studio): render multi-line SQL error HINTs (#45038)
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>
This commit is contained in:
1 parent
19027e73f8
commit
45fc609471
7 files changed
+156
-14
No files matched your search
@@ -5,6 +5,7 @@ import { forwardRef } from 'react'
|
||||
import { Button, cn, Tooltip, TooltipContent, TooltipTrigger } from 'ui'
|
||||
|
||||
import Results from './Results'
|
||||
import { getSqlErrorLines } from './UtilityTabResults.utils'
|
||||
import { subscriptionHasHipaaAddon } from '@/components/interfaces/Billing/Subscription/Subscription.utils'
|
||||
import { AiAssistantDropdown } from '@/components/ui/AiAssistantDropdown'
|
||||
import CopyButton from '@/components/ui/CopyButton'
|
||||
@@ -55,9 +56,7 @@ const UtilityTabResults = forwardRef<HTMLDivElement, UtilityTabResultsProps>(
|
||||
</div>
|
||||
)
|
||||
} else if (result?.error) {
|
||||
const formattedError = (result.error?.formattedError?.split('\n') ?? []).filter(
|
||||
(x: string) => x.length > 0
|
||||
)
|
||||
const errorLines = getSqlErrorLines(result.error)
|
||||
const readReplicaError =
|
||||
state.selectedDatabaseId !== ref &&
|
||||
result.error.message.includes('in a read-only transaction')
|
||||
@@ -96,8 +95,8 @@ const UtilityTabResults = forwardRef<HTMLDivElement, UtilityTabResultsProps>(
|
||||
</div>
|
||||
) : (
|
||||
<div className="flex flex-col gap-y-1">
|
||||
{formattedError.length > 0 ? (
|
||||
formattedError.map((x: string, i: number) => (
|
||||
{errorLines.length > 0 ? (
|
||||
errorLines.map((x: string, i: number) => (
|
||||
<pre key={`error-${i}`} className="font-mono text-sm text-wrap">
|
||||
{x}
|
||||
</pre>
|
||||
@@ -146,10 +145,10 @@ const UtilityTabResults = forwardRef<HTMLDivElement, UtilityTabResultsProps>(
|
||||
Switch to primary database
|
||||
</Button>
|
||||
)}
|
||||
{formattedError.length > 0 && (
|
||||
{errorLines.length > 0 && (
|
||||
<Tooltip>
|
||||
<TooltipTrigger>
|
||||
<CopyButton iconOnly type="default" text={formattedError.join('\n')} />
|
||||
<CopyButton iconOnly type="default" text={errorLines.join('\n')} />
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="bottom" align="center">
|
||||
<span>Copy error</span>
|
||||
|
||||
+80
@@ -0,0 +1,80 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { getSqlErrorLines } from './UtilityTabResults.utils'
|
||||
|
||||
describe('getSqlErrorLines', () => {
|
||||
it('returns formattedError lines when present', () => {
|
||||
const lines = getSqlErrorLines({
|
||||
message: 'permission denied for table users',
|
||||
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(lines).toEqual([
|
||||
'ERROR: 42501: permission denied for table users',
|
||||
'HINT: To grant access to anon on a specific table:',
|
||||
' GRANT SELECT ON TABLE public.users TO anon;',
|
||||
])
|
||||
})
|
||||
|
||||
it('strips empty lines from formattedError', () => {
|
||||
const lines = getSqlErrorLines({
|
||||
formattedError: 'ERROR: boom\n\nHINT: retry\n',
|
||||
})
|
||||
|
||||
expect(lines).toEqual(['ERROR: boom', 'HINT: retry'])
|
||||
})
|
||||
|
||||
it('falls back to message lines when formattedError is missing and message is multi-line', () => {
|
||||
const lines = getSqlErrorLines({
|
||||
message:
|
||||
'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(lines).toEqual([
|
||||
'ERROR: 42501: permission denied for table users',
|
||||
'HINT: To grant access to anon on a specific table:',
|
||||
' GRANT SELECT ON TABLE public.users TO anon;',
|
||||
])
|
||||
})
|
||||
|
||||
it('returns empty array for a single-line message so callers render the fallback', () => {
|
||||
const lines = getSqlErrorLines({ message: 'permission denied for table users' })
|
||||
expect(lines).toEqual([])
|
||||
})
|
||||
|
||||
it('returns empty array when both fields are missing', () => {
|
||||
expect(getSqlErrorLines({})).toEqual([])
|
||||
})
|
||||
|
||||
it('returns empty array when message is an empty string', () => {
|
||||
expect(getSqlErrorLines({ message: '' })).toEqual([])
|
||||
})
|
||||
|
||||
it('returns empty array when message only contains whitespace newlines', () => {
|
||||
// Only empty segments after filtering — treated as single-line
|
||||
expect(getSqlErrorLines({ message: '\n\n' })).toEqual([])
|
||||
})
|
||||
|
||||
it('prefers formattedError even when message is also multi-line', () => {
|
||||
const lines = getSqlErrorLines({
|
||||
message: 'message line 1\nmessage line 2',
|
||||
formattedError: 'formatted line 1\nformatted line 2',
|
||||
})
|
||||
|
||||
expect(lines).toEqual(['formatted line 1', 'formatted line 2'])
|
||||
})
|
||||
|
||||
it('falls through to message when formattedError is empty string', () => {
|
||||
const lines = getSqlErrorLines({
|
||||
message: 'ERROR: line 1\nHINT: line 2',
|
||||
formattedError: '',
|
||||
})
|
||||
|
||||
expect(lines).toEqual(['ERROR: line 1', 'HINT: line 2'])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,18 @@
|
||||
/**
|
||||
* Pick which lines to render for a SQL editor error.
|
||||
*
|
||||
* pg-meta returns `formattedError` with multi-line ERROR/HINT/LINE output from Postgres.
|
||||
* Historically only `message` was reliably populated end-to-end, which is why the UI also
|
||||
* falls back to splitting `message` on newlines — e.g. the enhanced permission-denied HINT
|
||||
* added by supabase/postgres#2084 arrives in the message body on some paths.
|
||||
*
|
||||
* Returns an empty array when the error is single-line (message only) — callers fall back to
|
||||
* a plain "Error: {message}" rendering in that case.
|
||||
*/
|
||||
export function getSqlErrorLines(error: { message?: string; formattedError?: string }): string[] {
|
||||
const formattedLines = (error.formattedError?.split('\n') ?? []).filter((x) => x.length > 0)
|
||||
if (formattedLines.length > 0) return formattedLines
|
||||
|
||||
const messageLines = (error.message?.split('\n') ?? []).filter((x) => x.length > 0)
|
||||
return messageLines.length > 1 ? messageLines : []
|
||||
}
|
||||
@@ -172,6 +172,10 @@ export const handleError = (error: unknown, options: HandleErrorOptions = {}): n
|
||||
'metadata' in error && typeof error.metadata === 'object' && !!error.metadata
|
||||
? (error.metadata as ErrorMetadata)
|
||||
: undefined
|
||||
const formattedError =
|
||||
'formattedError' in error && typeof error.formattedError === 'string'
|
||||
? error.formattedError
|
||||
: undefined
|
||||
|
||||
if (errorMessage) {
|
||||
const matched = ERROR_PATTERNS.find(({ pattern }) => pattern.test(errorMessage))
|
||||
@@ -182,7 +186,8 @@ export const handleError = (error: unknown, options: HandleErrorOptions = {}): n
|
||||
requestId,
|
||||
retryAfter,
|
||||
requestPathname,
|
||||
metadata
|
||||
metadata,
|
||||
formattedError
|
||||
)
|
||||
: new UnknownAPIResponseError(
|
||||
errorMessage,
|
||||
@@ -190,7 +195,8 @@ export const handleError = (error: unknown, options: HandleErrorOptions = {}): n
|
||||
requestId,
|
||||
retryAfter,
|
||||
requestPathname,
|
||||
metadata
|
||||
metadata,
|
||||
formattedError
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -95,5 +95,39 @@ describe('handleError — error classification', () => {
|
||||
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()
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -12,9 +12,10 @@ export class ConnectionTimeoutError extends ResponseError {
|
||||
requestId?: string,
|
||||
retryAfter?: number,
|
||||
requestPathname?: string,
|
||||
metadata?: ErrorMetadata
|
||||
metadata?: ErrorMetadata,
|
||||
formattedError?: string
|
||||
) {
|
||||
super(message, code, requestId, retryAfter, requestPathname, metadata)
|
||||
super(message, code, requestId, retryAfter, requestPathname, metadata, formattedError)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -27,9 +28,10 @@ export class UnknownAPIResponseError extends ResponseError {
|
||||
requestId?: string,
|
||||
retryAfter?: number,
|
||||
requestPathname?: string,
|
||||
metadata?: ErrorMetadata
|
||||
metadata?: ErrorMetadata,
|
||||
formattedError?: string
|
||||
) {
|
||||
super(message, code, requestId, retryAfter, requestPathname, metadata)
|
||||
super(message, code, requestId, retryAfter, requestPathname, metadata, formattedError)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -102,6 +102,7 @@ export class ResponseError extends Error {
|
||||
requestPathname?: string
|
||||
metadata?: CostMetadata
|
||||
errorType?: string
|
||||
formattedError?: string
|
||||
|
||||
constructor(
|
||||
message: string | undefined,
|
||||
@@ -109,7 +110,8 @@ export class ResponseError extends Error {
|
||||
requestId?: string,
|
||||
retryAfter?: number,
|
||||
requestPathname?: string,
|
||||
metadata?: CostMetadata
|
||||
metadata?: CostMetadata,
|
||||
formattedError?: string
|
||||
) {
|
||||
super(message || 'API error happened while trying to communicate with the server.')
|
||||
this.code = code
|
||||
@@ -117,6 +119,7 @@ export class ResponseError extends Error {
|
||||
this.retryAfter = retryAfter
|
||||
this.requestPathname = requestPathname
|
||||
this.metadata = metadata
|
||||
this.formattedError = formattedError
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user