From eaad809abfc47c5f3634da6624ef90a013a784a8 Mon Sep 17 00:00:00 2001 From: Monica Khoury <99693443+monicakh@users.noreply.github.com> Date: Tue, 13 Oct 2026 19:44:21 +0300 Subject: [PATCH] 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. --- .../Webhooks/PlatformWebhooksPage.tsx | 69 ++++++++++++++----- ...latform-webhook-delivery-retry-mutation.ts | 2 +- ...platform-webhook-endpoint-test-mutation.ts | 2 +- .../platform-webhooks-fetchers.ts | 27 +++++++- 4 files changed, 76 insertions(+), 24 deletions(-) diff --git a/apps/studio/components/interfaces/Platform/Webhooks/PlatformWebhooksPage.tsx b/apps/studio/components/interfaces/Platform/Webhooks/PlatformWebhooksPage.tsx index 6a154c2a18a..14b0a8b431d 100644 --- a/apps/studio/components/interfaces/Platform/Webhooks/PlatformWebhooksPage.tsx +++ b/apps/studio/components/interfaces/Platform/Webhooks/PlatformWebhooksPage.tsx @@ -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') } diff --git a/apps/studio/data/platform-webhooks/platform-webhook-delivery-retry-mutation.ts b/apps/studio/data/platform-webhooks/platform-webhook-delivery-retry-mutation.ts index 6dfc069c0a4..7fac3a11b82 100644 --- a/apps/studio/data/platform-webhooks/platform-webhook-delivery-retry-mutation.ts +++ b/apps/studio/data/platform-webhooks/platform-webhook-delivery-retry-mutation.ts @@ -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}`) } diff --git a/apps/studio/data/platform-webhooks/platform-webhook-endpoint-test-mutation.ts b/apps/studio/data/platform-webhooks/platform-webhook-endpoint-test-mutation.ts index fba46f1f6a7..6e3741d035d 100644 --- a/apps/studio/data/platform-webhooks/platform-webhook-endpoint-test-mutation.ts +++ b/apps/studio/data/platform-webhooks/platform-webhook-endpoint-test-mutation.ts @@ -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}`) } diff --git a/apps/studio/data/platform-webhooks/platform-webhooks-fetchers.ts b/apps/studio/data/platform-webhooks/platform-webhooks-fetchers.ts index 3f649323048..6477a90233e 100644 --- a/apps/studio/data/platform-webhooks/platform-webhooks-fetchers.ts +++ b/apps/studio/data/platform-webhooks/platform-webhooks-fetchers.ts @@ -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 } =