From a75414a05e340a3f5d54d3d9d361bd1ddf6d20b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Thu, 2 Jul 2026 20:59:42 +0200 Subject: [PATCH] fix(sse): treat expected event-stream coordination errors as 403s and never crash the sync loop (#22475) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Rationale `NOT_AUTHORIZED` and `EVENT_STREAM_ALREADY_EXISTS` on the event-stream mutations are **expected coordination outcomes** — the frontend explicitly recognizes both (`isGracefullyHandledEventStreamError`) and recovers by recreating its stream. But `EventStreamExceptionFilter` rethrows them as `InternalServerError`, which (a) Sentry captures on every single occurrence, and (b) counts as a 500 in operation metrics. **Production evidence (Sentry):** this turned a March client-regression into a 119,510-event / 2,607-user flood ([TWENTY-SERVER-FP3](https://twenty-v7.sentry.io/issues/TWENTY-SERVER-FP3), plus FP0 at ~29k) that buried real errors. The trigger was fixed back then, but the amplifier — error-level capture of an expected signal — is still in place, and a residual trickle still fires today. Second defect, client side: for any *non-graceful* server error (e.g. a lock-acquisition timeout), `SSEQuerySubscribeEffect.handleError` **threw** from inside a debounced callback — an unhandled rejection ([TWENTY-FRONT-62M](https://twenty-v7.sentry.io/issues/TWENTY-FRONT-62M), 233 users; [6MM](https://twenty-v7.sentry.io/issues/TWENTY-FRONT-6MM), 65 users) that left the tab's query listeners permanently out of sync with the server (no more live updates until reload). ## Why this is the root cause, not a symptom patch The protocol design already says these are recoverable client-coordination signals — the bug is purely that the server encodes them with 500 semantics and the client punishes unexpected errors by giving up instead of resetting. This PR aligns both ends with the existing design rather than adding new machinery: - Server: `ForbiddenError` (403) with the same `subCode` — the client's graceful check already accepts `code === 'FORBIDDEN'`, so this is compatible by construction; `FORBIDDEN` is already in `graphQLErrorCodesToFilter`, so monitoring capture stops with no new filtering logic. - Client: the non-graceful path now does exactly what the graceful path does (reset listeners + recreate stream) and *additionally* reports the unexpected error — visibility without a crash. ## User impact Tabs that hit any event-stream error now always self-heal back to live updates instead of silently going stale until reload (~300 users hit the crash path over 90d). On the ops side: expected coordination noise leaves error monitoring, and 403/500 metrics become truthful. ## Test plan - [x] Behavior preserved for graceful codes (same reset path, client check already includes FORBIDDEN) - [ ] CI green https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38 --- _Generated by [Claude Code](https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38)_ Review in cubic --- .../components/SSEQuerySubscribeEffect.tsx | 15 +++++++++------ .../event-stream-exception.filter.ts | 4 ++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/twenty-front/src/modules/sse-db-event/components/SSEQuerySubscribeEffect.tsx b/packages/twenty-front/src/modules/sse-db-event/components/SSEQuerySubscribeEffect.tsx index 53c38e46b8..3f5e046ee0 100644 --- a/packages/twenty-front/src/modules/sse-db-event/components/SSEQuerySubscribeEffect.tsx +++ b/packages/twenty-front/src/modules/sse-db-event/components/SSEQuerySubscribeEffect.tsx @@ -8,6 +8,7 @@ import { sseEventStreamReadyState } from '@/sse-db-event/states/sseEventStreamRe import { isGracefullyHandledEventStreamError } from '@/sse-db-event/utils/isGracefullyHandledEventStreamError'; import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue'; import { CombinedGraphQLErrors } from '@apollo/client/errors'; +import { captureException } from '@sentry/react'; import { useMutation } from '@apollo/client/react'; import { isNonEmptyString } from '@sniptt/guards'; import { useStore } from 'jotai'; @@ -47,18 +48,20 @@ export const SSEQuerySubscribeEffect = () => { const extensions = getGraphqlErrorExtensionsFromError(error); if ( - isGracefullyHandledEventStreamError({ + !isGracefullyHandledEventStreamError({ subCode: extensions?.subCode, code: extensions?.code, }) ) { - store.set(activeQueryListenersState.atom, []); - store.set(shouldDestroyEventStreamState.atom, true); - - return; + captureException( + new Error(`Unhandled error for event stream: ${error.message}`, { + cause: error, + }), + ); } - throw new Error(`Unhandled error for event stream: ${error.message}`); + store.set(activeQueryListenersState.atom, []); + store.set(shouldDestroyEventStreamState.atom, true); } }, [store], diff --git a/packages/twenty-server/src/engine/subscriptions/event-stream-exception.filter.ts b/packages/twenty-server/src/engine/subscriptions/event-stream-exception.filter.ts index 0715f964f5..7fd3b7a730 100644 --- a/packages/twenty-server/src/engine/subscriptions/event-stream-exception.filter.ts +++ b/packages/twenty-server/src/engine/subscriptions/event-stream-exception.filter.ts @@ -3,7 +3,7 @@ import { GqlExceptionFilter } from '@nestjs/graphql'; import { assertUnreachable } from 'twenty-shared/utils'; -import { InternalServerError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; +import { ForbiddenError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; import { EventStreamException, EventStreamExceptionCode, @@ -15,7 +15,7 @@ export class EventStreamExceptionFilter implements GqlExceptionFilter { switch (exception.code) { case EventStreamExceptionCode.EVENT_STREAM_ALREADY_EXISTS: case EventStreamExceptionCode.NOT_AUTHORIZED: - throw new InternalServerError(exception.message, { + throw new ForbiddenError(exception.message, { subCode: exception.code, userFriendlyMessage: exception.userFriendlyMessage, });