mirror of
https://github.com/supabase/supabase.git
synced 2026-10-15 06:09:08 +03:00
fix: webhook retry error handling (nested error envelope + unhandled rejections) (#51591)
## What
Fixes two bugs found while writing component tests for the platform
webhooks page (follow-up to FE-3749):
1. The webhooks API nests error details under an `error` key (`{ error:
{ code, message } }`), but the shared `handleError` only reads a
top-level `message`. This meant the friendly rate-limit message ("wait a
minute and try again") never actually fired, it always fell back to a
generic error. Fixed with a local unwrapping `handleError` in
`platform-webhooks-fetchers.ts`.
2. Every mutation handler on the page (`mutateAsync` for
create/update/delete/regenerate-secret/send-test-event/retry) left the
re-thrown error from React Query uncaught, producing an unhandled
promise rejection on every failure, even though the error toast itself
was already correct. Fixed by wrapping each call in try/catch.
## Why
Found while writing the stacked test PR for FE-3749; filed as
[FE-4620](https://linear.app/supabase/issue/FE-4620/two-webhook-error-handling-bugs-found-while-writing-retryrate-limit)
since it's a correctness fix independent of the test work.
## Testing
Manually verified both fixes on localhost: rate-limit retry now shows
the friendly message, and no `Uncaught (in promise)` errors appear in
the console on any mutation failure.
This commit is contained in:
1 parent
700365ffba
commit
eaad809abf
4 files changed
+76
-24
No files matched your search
@@ -27,7 +27,7 @@ import { PageContainer } from 'ui-patterns/PageContainer'
|
||||
import { PageSection, PageSectionContent } from 'ui-patterns/PageSection'
|
||||
|
||||
import { PLATFORM_WEBHOOK_EVENT_TYPES } from './PlatformWebhooks.constants'
|
||||
import type { WebhookScope } from './PlatformWebhooks.types'
|
||||
import type { WebhookEndpoint, WebhookScope } from './PlatformWebhooks.types'
|
||||
import {
|
||||
filterWebhookDeliveries,
|
||||
filterWebhookEndpoints,
|
||||
@@ -286,7 +286,12 @@ export const PlatformWebhooksPage = ({ scope, endpointId }: PlatformWebhooksPage
|
||||
|
||||
const handleDeleteEndpoint = async () => {
|
||||
if (!endpointPendingDelete) return
|
||||
await deleteEndpointMutation.mutateAsync({ scope: apiScope, id: endpointPendingDelete.id })
|
||||
try {
|
||||
await deleteEndpointMutation.mutateAsync({ scope: apiScope, id: endpointPendingDelete.id })
|
||||
} catch (error) {
|
||||
console.error('Failed to delete webhook endpoint', error)
|
||||
return
|
||||
}
|
||||
if (endpointPendingDelete.id === endpointId) {
|
||||
router.push(webhooksHref)
|
||||
setDeliverySearch('')
|
||||
@@ -298,7 +303,13 @@ export const PlatformWebhooksPage = ({ scope, endpointId }: PlatformWebhooksPage
|
||||
const handleUpsertEndpoint = async (values: EndpointFormValues) => {
|
||||
if (panel === 'create') {
|
||||
const input = toEndpointPayload(values)
|
||||
const created = await createEndpointMutation.mutateAsync({ scope: apiScope, input })
|
||||
let created: WebhookEndpoint
|
||||
try {
|
||||
created = await createEndpointMutation.mutateAsync({ scope: apiScope, input })
|
||||
} catch (error) {
|
||||
console.error('Failed to create webhook endpoint', error)
|
||||
return
|
||||
}
|
||||
// The server never echoes the secret back — we already know it, since the
|
||||
// user (or the "Generate" button) chose it client-side before submitting.
|
||||
setPendingCreatedEndpointId(created.id)
|
||||
@@ -315,11 +326,16 @@ export const PlatformWebhooksPage = ({ scope, endpointId }: PlatformWebhooksPage
|
||||
}
|
||||
|
||||
if (panel === 'edit' && selectedEndpoint) {
|
||||
await updateEndpointMutation.mutateAsync({
|
||||
scope: apiScope,
|
||||
id: selectedEndpoint.id,
|
||||
input: toEndpointPayload(values),
|
||||
})
|
||||
try {
|
||||
await updateEndpointMutation.mutateAsync({
|
||||
scope: apiScope,
|
||||
id: selectedEndpoint.id,
|
||||
input: toEndpointPayload(values),
|
||||
})
|
||||
} catch (error) {
|
||||
console.error('Failed to update webhook endpoint', error)
|
||||
return
|
||||
}
|
||||
setPanel(null)
|
||||
setEditEnabledOverride(null)
|
||||
toast.success('Endpoint updated')
|
||||
@@ -329,11 +345,16 @@ export const PlatformWebhooksPage = ({ scope, endpointId }: PlatformWebhooksPage
|
||||
const handleRegenerateSecret = async () => {
|
||||
if (!selectedEndpoint) return
|
||||
const nextSecret = generateSigningSecret()
|
||||
await regenerateSecretMutation.mutateAsync({
|
||||
scope: apiScope,
|
||||
id: selectedEndpoint.id,
|
||||
signingSecret: nextSecret,
|
||||
})
|
||||
try {
|
||||
await regenerateSecretMutation.mutateAsync({
|
||||
scope: apiScope,
|
||||
id: selectedEndpoint.id,
|
||||
signingSecret: nextSecret,
|
||||
})
|
||||
} catch (error) {
|
||||
console.error('Failed to regenerate webhook signing secret', error)
|
||||
return
|
||||
}
|
||||
setSigningSecretReveal({ signingSecret: nextSecret })
|
||||
setShowRegenerateSecretConfirm(false)
|
||||
toast.success('Signing secret regenerated')
|
||||
@@ -341,7 +362,12 @@ export const PlatformWebhooksPage = ({ scope, endpointId }: PlatformWebhooksPage
|
||||
|
||||
const handleSendTestEvent = async () => {
|
||||
if (!selectedEndpoint) return
|
||||
await testEndpointMutation.mutateAsync({ scope: apiScope, id: selectedEndpoint.id })
|
||||
try {
|
||||
await testEndpointMutation.mutateAsync({ scope: apiScope, id: selectedEndpoint.id })
|
||||
} catch (error) {
|
||||
console.error('Failed to send webhook test event', error)
|
||||
return
|
||||
}
|
||||
toast.success('Test event sent — check deliveries shortly')
|
||||
}
|
||||
|
||||
@@ -350,11 +376,16 @@ export const PlatformWebhooksPage = ({ scope, endpointId }: PlatformWebhooksPage
|
||||
const delivery = deliveries.find((item) => item.id === deliveryId)
|
||||
if (!delivery || delivery.status === 'success') return
|
||||
|
||||
await retryDeliveryMutation.mutateAsync({
|
||||
scope: apiScope,
|
||||
endpointId: selectedEndpoint.id,
|
||||
id: deliveryId,
|
||||
})
|
||||
try {
|
||||
await retryDeliveryMutation.mutateAsync({
|
||||
scope: apiScope,
|
||||
endpointId: selectedEndpoint.id,
|
||||
id: deliveryId,
|
||||
})
|
||||
} catch (error) {
|
||||
console.error('Failed to retry webhook delivery', error)
|
||||
return
|
||||
}
|
||||
toast.success('Delivery queued for retry')
|
||||
}
|
||||
|
||||
|
||||
@@ -43,7 +43,7 @@ export const useWebhookDeliveryRetryMutation = ({
|
||||
async onError(data, variables, context) {
|
||||
if (onError === undefined) {
|
||||
if (isWebhookRateLimitError(data)) {
|
||||
toast.error('Too many retries — wait a minute and try again (limit: 10 per minute).')
|
||||
toast.error('Too many requests. Wait a minute and try again (limit: 10 per minute).')
|
||||
} else {
|
||||
toast.error(`Failed to retry delivery: ${data.message}`)
|
||||
}
|
||||
|
||||
@@ -43,7 +43,7 @@ export const useWebhookEndpointTestMutation = ({
|
||||
async onError(data, variables, context) {
|
||||
if (onError === undefined) {
|
||||
if (isWebhookRateLimitError(data)) {
|
||||
toast.error('Too many test events — wait a minute and try again (limit: 10 per minute).')
|
||||
toast.error('Too many requests. Wait a minute and try again (limit: 10 per minute).')
|
||||
} else {
|
||||
toast.error(`Failed to send test event: ${data.message}`)
|
||||
}
|
||||
|
||||
@@ -1,6 +1,22 @@
|
||||
import type { operations } from 'api-types'
|
||||
|
||||
import { del, get, handleError, patch, post } from '@/data/fetchers'
|
||||
import { del, get, patch, post, handleError as sharedHandleError } from '@/data/fetchers'
|
||||
|
||||
// The real webhooks API nests error details under an `error` key
|
||||
// (`{ error: { code, message } }`), but the shared `handleError` only reads a
|
||||
// top-level `message`/`msg`. Flatten the nested fields up so it finds one.
|
||||
// Rate limiting is expected and already has dedicated UI handling, so it's the
|
||||
// only case excluded from Sentry capture.
|
||||
const handleError = (error: unknown): never => {
|
||||
if (error && typeof error === 'object' && 'error' in error) {
|
||||
const { error: nested, ...rest } = error
|
||||
if (nested && typeof nested === 'object') {
|
||||
const isRateLimit = 'code' in nested && nested.code === 'too_many_requests'
|
||||
return sharedHandleError({ ...rest, ...nested }, { alwaysCapture: !isRateLimit })
|
||||
}
|
||||
}
|
||||
return sharedHandleError(error)
|
||||
}
|
||||
|
||||
export type WebhookScopeParams =
|
||||
| { scope: 'organization'; orgSlug: string }
|
||||
@@ -195,9 +211,14 @@ export async function sendTestWebhookEvent(scope: WebhookScopeParams, id: string
|
||||
return data
|
||||
}
|
||||
|
||||
// Retry and test-event are rate-limited to 10 requests/60s.
|
||||
// Retry and test-event are rate-limited to 10 requests/60s. The documented
|
||||
// message (api_v2_openapi.json) is "Too Many Requests", but the locally
|
||||
// running API currently sends "Rate limit exceeded" instead. Accept both
|
||||
// until that's reconciled.
|
||||
const RATE_LIMIT_MESSAGES = ['Too Many Requests', 'Rate limit exceeded']
|
||||
|
||||
export const isWebhookRateLimitError = (error: { message?: string }) =>
|
||||
error.message === 'Rate limit exceeded'
|
||||
!!error.message && RATE_LIMIT_MESSAGES.includes(error.message)
|
||||
|
||||
export async function retryWebhookDelivery(scope: WebhookScopeParams, id: string) {
|
||||
const { data, error } =
|
||||
|
||||
Reference in new issue
Block a user