From 68aa07ec6331270ec3904447f92127a6e524d1db Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 13:48:36 +0000 Subject: [PATCH] fix(tickets): recover from a GraphQL field skew instead of a raw error page One field the deployed schema does not declare fails validation for the whole tickets document. The table and board rendered the raw server error full-page with no retry, blocking a core flow for the rest of the session. - Prune an undeclared leaf field and retry once in `TicketService`, so a single field skew (e.g. `escalatedByUser`) degrades to a missing badge, not an empty board or list. - Replace the raw `PageError` on the table and board with an inline `LoadError` that keeps the page chrome and offers a working retry; never show the raw server payload. - Skip react-query retries for deterministic schema-validation errors. - Wrap the tickets route in `ContentErrorBoundary`, matching sibling routes. Generated-By: PostHog Desktop Task-Id: fd82f8ce-ef9a-454e-994c-518cf8907ec2 --- .../components/board-column-subscriber.tsx | 5 +- .../tickets/components/tickets-board.tsx | 37 +++++++-- .../tickets/components/tickets-table.tsx | 24 +++++- .../(app)/tickets/hooks/use-tickets-query.ts | 28 +++---- src/app/(app)/tickets/page.tsx | 8 +- .../(app)/tickets/queries/ticket-queries.ts | 9 +- .../(app)/tickets/services/ticket-service.ts | 58 +++++++------ src/app/(app)/tickets/utils/graphql.test.ts | 73 +++++++++++++++++ src/app/(app)/tickets/utils/graphql.ts | 82 ++++++++++++++++++- 9 files changed, 266 insertions(+), 58 deletions(-) create mode 100644 src/app/(app)/tickets/utils/graphql.test.ts diff --git a/src/app/(app)/tickets/components/board-column-subscriber.tsx b/src/app/(app)/tickets/components/board-column-subscriber.tsx index 962640a7..bfd6266a 100644 --- a/src/app/(app)/tickets/components/board-column-subscriber.tsx +++ b/src/app/(app)/tickets/components/board-column-subscriber.tsx @@ -5,6 +5,7 @@ import { useEffect } from 'react'; import { BOARD_PAGE_SIZE, type BoardColumnState } from '../hooks/use-tickets-board-query'; import { ticketService } from '../services'; import type { TicketsPage } from '../services/ticket-service.types'; +import { isGraphQlValidationError } from '../utils/graphql'; import { dialogsQueryKeys } from '../utils/query-keys'; export interface BoardColumnUpdate { @@ -73,7 +74,9 @@ export function BoardColumnSubscriber({ statusId, params, onUpdate, registerLoad lastPage.pageInfo.hasNextPage ? (lastPage.pageInfo.endCursor ?? undefined) : undefined, staleTime: 60_000, gcTime: 5 * 60_000, - retry: 2, + // A schema-validation failure is deterministic — retrying it only delays the + // error. Transient failures still get the two attempts. + retry: (count, error) => !isGraphQlValidationError(error) && count < 2, retryDelay: 1000, refetchInterval: 15_000, }); diff --git a/src/app/(app)/tickets/components/tickets-board.tsx b/src/app/(app)/tickets/components/tickets-board.tsx index 2e804059..74416b09 100644 --- a/src/app/(app)/tickets/components/tickets-board.tsx +++ b/src/app/(app)/tickets/components/tickets-board.tsx @@ -8,13 +8,14 @@ import { type BoardTicket, } from '@flamingo-stack/openframe-frontend-core/components/features'; import { Filter02Icon } from '@flamingo-stack/openframe-frontend-core/components/icons-v2'; -import { Button, PageError, PageLayout } from '@flamingo-stack/openframe-frontend-core/components/ui'; +import { Button, LoadError, PageLayout } from '@flamingo-stack/openframe-frontend-core/components/ui'; import { useDebounce, useToast } from '@flamingo-stack/openframe-frontend-core/hooks'; import { type InfiniteData, useQueryClient } from '@tanstack/react-query'; import { type ReactNode, useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useUserStatusMap } from '@/app/hooks/use-user-status-map'; import { featureFlags } from '@/lib/feature-flags'; import { appendImageHash } from '@/lib/image-url'; +import { isOfflineError, loadErrorProps } from '@/lib/query-state'; import { routes } from '@/lib/routes'; import { useApprovalRequests } from '../hooks/use-approval-requests'; import { useMoveTicket, useMovingTicketIds } from '../hooks/use-move-ticket'; @@ -153,7 +154,12 @@ export function TicketsBoard({ const debouncedSearch = useDebounce(search, 300); const [mobileFiltersOpen, setMobileFiltersOpen] = useState(false); - const { data: statusesData, isLoading: statusesLoading, error: statusesError } = useTicketStatusesQuery(); + const { + data: statusesData, + isLoading: statusesLoading, + error: statusesError, + refetch: refetchStatuses, + } = useTicketStatusesQuery(); const { data: transitionRules } = useTicketStatusTransitionRules(); const { mutate: moveTicket } = useMoveTicket(); const movingIds = useMovingTicketIds(); @@ -392,11 +398,28 @@ export function TicketsBoard({ const actions = useMemo(() => emphasizeNewTicketAction(baseActions, showEmptyState), [baseActions, showEmptyState]); - if (statusesError) { - return ; - } - if (columnError) { - return ; + // Clearing the column updates drops `columnError` and remounts the column + // subscribers, which refetch; the statuses query is retried alongside. + const retryLoad = () => { + setColumnUpdates({}); + queryClient.resetQueries({ queryKey: dialogsQueryKeys.boardColumns() }); + refetchStatuses(); + }; + + // Keep the page chrome and offer a retry instead of a bare banner, and never + // show the raw server payload the failed load carried. + const loadError = statusesError ?? columnError; + if (loadError) { + return ( + + + + ); } return ( diff --git a/src/app/(app)/tickets/components/tickets-table.tsx b/src/app/(app)/tickets/components/tickets-table.tsx index 4da20f49..16a7f863 100644 --- a/src/app/(app)/tickets/components/tickets-table.tsx +++ b/src/app/(app)/tickets/components/tickets-table.tsx @@ -7,13 +7,14 @@ import { type ColumnFiltersState, DataTable, FilterModal, + LoadError, type OnChangeFn, - PageError, PageLayout, } from '@flamingo-stack/openframe-frontend-core/components/ui'; import { useDebounce } from '@flamingo-stack/openframe-frontend-core/hooks'; import { type ReactNode, useCallback, useMemo, useState } from 'react'; import { useStickyToolbar } from '@/app/hooks/use-sticky-toolbar'; +import { loadErrorProps } from '@/lib/query-state'; import { emphasizeNewTicketAction, useTicketsActions } from '../hooks/use-tickets-actions'; import { useTicketsQuery } from '../hooks/use-tickets-query'; import { useTicketStatusesQuery } from '../statuses/hooks/use-ticket-statuses-query'; @@ -60,7 +61,9 @@ export function TicketsTable({ isFetchingNextPage, hasNextPage, fetchNextPage, - error, + isError, + isOffline, + refetch, } = useTicketsQuery({ archived: isArchived, search: debouncedSearch, @@ -151,8 +154,21 @@ export function TicketsTable({ const actions = useMemo(() => emphasizeNewTicketAction(baseActions, showEmptyState), [baseActions, showEmptyState]); - if (error) { - return ; + // Keep the page chrome — title, back button, view switch — so a failed load + // leaves a working page with a retry, not a bare banner. The thrown message is + // never shown: it is a raw server payload the user cannot act on. + if (isError) { + return ( + + refetch())} /> + + ); } return ( diff --git a/src/app/(app)/tickets/hooks/use-tickets-query.ts b/src/app/(app)/tickets/hooks/use-tickets-query.ts index a769ba59..25e6aa84 100644 --- a/src/app/(app)/tickets/hooks/use-tickets-query.ts +++ b/src/app/(app)/tickets/hooks/use-tickets-query.ts @@ -1,11 +1,12 @@ 'use client'; -import { useToast } from '@flamingo-stack/openframe-frontend-core/hooks'; import { useInfiniteQuery, useQueryClient } from '@tanstack/react-query'; -import { useCallback, useEffect, useMemo } from 'react'; +import { useCallback, useMemo } from 'react'; +import { queryState } from '@/lib/query-state'; import { ticketService } from '../services'; import type { TicketsPage } from '../services/ticket-service.types'; import { useTicketStatusesQuery } from '../statuses/hooks/use-ticket-statuses-query'; +import { isGraphQlValidationError } from '../utils/graphql'; import { type DialogsQueryParams, dialogsQueryKeys } from '../utils/query-keys'; const TICKETS_PAGE_SIZE = 20; @@ -21,7 +22,6 @@ export function useTicketsQuery({ tagIds, pageSize = TICKETS_PAGE_SIZE, }: DialogsQueryParams) { - const { toast } = useToast(); const queryClient = useQueryClient(); const statusesQuery = useTicketStatusesQuery({ enabled: true }); @@ -77,20 +77,12 @@ export function useTicketsQuery({ initialPageParam: undefined as string | undefined, staleTime: 60_000, gcTime: 5 * 60_000, - retry: 2, + // A schema-validation failure is deterministic — retrying it only delays the + // error. Transient failures still get the two attempts. + retry: (count, error) => !isGraphQlValidationError(error) && count < 2, retryDelay: 1000, }); - useEffect(() => { - if (query.error) { - toast({ - title: 'Failed to Load Tickets', - description: query.error.message, - variant: 'destructive', - }); - } - }, [query.error, toast]); - const dialogs = useMemo(() => query.data?.pages.flatMap(page => page.dialogs) ?? [], [query.data?.pages]); const resetToFirstPage = useCallback(() => { @@ -108,13 +100,19 @@ export function useTicketsQuery({ }); }, [queryClient, archived, search, statusFilters, statusIds, organizationIds, assigneeIds, tagIds, pageSize]); + // Only a first-load failure with no rows is an error state; a background + // refetch that fails behind cached rows keeps the stale rows on screen. + const state = queryState(query); + return { dialogs, isLoading: query.isLoading || waitingForStatusIds, isFetchingNextPage: query.isFetchingNextPage, hasNextPage: query.hasNextPage ?? false, fetchNextPage: query.fetchNextPage, - error: query.error?.message ?? null, + isError: state.error !== null, + isOffline: state.isOffline, + refetch: query.refetch, resetToFirstPage, }; } diff --git a/src/app/(app)/tickets/page.tsx b/src/app/(app)/tickets/page.tsx index 206544a9..6b02b7a7 100644 --- a/src/app/(app)/tickets/page.tsx +++ b/src/app/(app)/tickets/page.tsx @@ -1,8 +1,8 @@ 'use client'; -import { ContentPageContainer } from '@flamingo-stack/openframe-frontend-core'; import { useRouter } from 'next/navigation'; import { useEffect } from 'react'; +import { ContentErrorBoundary } from '@/app/components/shared'; import { isSaasTenantMode } from '@/lib/app-mode'; import { routes } from '@/lib/routes'; import { TicketsView } from './components/tickets-view'; @@ -22,5 +22,9 @@ export default function Tickets() { return null; } - return ; + return ( + + + + ); } diff --git a/src/app/(app)/tickets/queries/ticket-queries.ts b/src/app/(app)/tickets/queries/ticket-queries.ts index 4c178f07..711fea03 100644 --- a/src/app/(app)/tickets/queries/ticket-queries.ts +++ b/src/app/(app)/tickets/queries/ticket-queries.ts @@ -275,10 +275,11 @@ export const GET_TICKETS_QUERY = ` /** * `escalatedByUser` ships with the escalation backend, so it rides the - * `ai-escalation` flag: a field the server's schema does not declare fails - * validation for the entire document, and `extractGraphQlData` throws on the - * first GraphQL error — every board column would come back empty rather than - * merely missing a badge. + * `ai-escalation` flag to avoid an extra round-trip in the common case: a field + * the server's schema does not declare fails validation for the whole document. + * The flag can still lead the backend by a deploy, so `TicketService.fetchGraphQl` + * is the safety net — it prunes an undeclared leaf field and retries once, and + * the board loads with the badge simply absent rather than empty. */ const boardCardTicketFragment = () => ` fragment BoardCardTicket on Ticket { diff --git a/src/app/(app)/tickets/services/ticket-service.ts b/src/app/(app)/tickets/services/ticket-service.ts index bdd0a034..7b6a255a 100644 --- a/src/app/(app)/tickets/services/ticket-service.ts +++ b/src/app/(app)/tickets/services/ticket-service.ts @@ -18,7 +18,7 @@ import { } from '../queries/ticket-queries'; import type { Dialog, DialogStatus, Message } from '../types/dialog.types'; import type { GraphQlResponse } from '../utils/graphql'; -import { extractGraphQlData } from '../utils/graphql'; +import { extractGraphQlData, pruneLeafFields, undefinedFieldNames } from '../utils/graphql'; import type { FetchBoardColumnByStatusIdParams, FetchMessagesParams, @@ -210,6 +210,28 @@ function normalizeTicketToDialog(ticket: TicketNode): Dialog { } export class TicketService implements TicketServiceInterface { + /** + * Post a query and tolerate one field skew. A single field the deployed schema + * does not declare fails validation for the whole document, which would blank + * the list or every board column. When that happens, prune the undeclared leaf + * fields and retry once, so the surface loads with that value simply absent + * (e.g. no escalation badge) instead of empty. + */ + private async fetchGraphQl(query: string, variables: Record): Promise { + const response = await apiClient.post>(API_ENDPOINTS.GRAPHQL, { query, variables }); + + const undefinedFields = response.data?.errors ? undefinedFieldNames(response.data.errors) : []; + if (undefinedFields.length > 0) { + const pruned = pruneLeafFields(query, undefinedFields); + if (pruned !== query) { + const retried = await apiClient.post>(API_ENDPOINTS.GRAPHQL, { query: pruned, variables }); + return extractGraphQlData(retried); + } + } + + return extractGraphQlData(response); + } + private async mutateTicketStatus(ticketId: string, mutation: string, responseKey: string): Promise { const response = await apiClient.post>>( API_ENDPOINTS.GRAPHQL, @@ -252,16 +274,11 @@ export class TicketService implements TicketServiceInterface { filter.tagIds = params.tagIds; } - const response = await apiClient.post>(API_ENDPOINTS.GRAPHQL, { - query: GET_TICKETS_QUERY, - variables: { - filter, - pagination: paginationVars, - search: params.search || undefined, - }, + const data = await this.fetchGraphQl(GET_TICKETS_QUERY, { + filter, + pagination: paginationVars, + search: params.search || undefined, }); - - const data = extractGraphQlData(response); const connection = data.tickets; return { @@ -277,20 +294,15 @@ export class TicketService implements TicketServiceInterface { } async fetchBoardColumnByStatusId(params: FetchBoardColumnByStatusIdParams): Promise { - const response = await apiClient.post>(API_ENDPOINTS.GRAPHQL, { - query: getBoardColumnTicketsQuery(), - variables: { - statusId: params.statusId, - limit: params.limit, - cursor: params.cursor, - search: params.search || undefined, - organizationIds: params.organizationIds?.length ? params.organizationIds : undefined, - assigneeIds: params.assigneeIds?.length ? params.assigneeIds : undefined, - tagIds: params.tagIds?.length ? params.tagIds : undefined, - }, + const data = await this.fetchGraphQl(getBoardColumnTicketsQuery(), { + statusId: params.statusId, + limit: params.limit, + cursor: params.cursor, + search: params.search || undefined, + organizationIds: params.organizationIds?.length ? params.organizationIds : undefined, + assigneeIds: params.assigneeIds?.length ? params.assigneeIds : undefined, + tagIds: params.tagIds?.length ? params.tagIds : undefined, }); - - const data = extractGraphQlData(response); const connection = data.tickets; return { diff --git a/src/app/(app)/tickets/utils/graphql.test.ts b/src/app/(app)/tickets/utils/graphql.test.ts new file mode 100644 index 00000000..8213d94f --- /dev/null +++ b/src/app/(app)/tickets/utils/graphql.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from 'vitest'; +import { + extractGraphQlData, + GraphQlResponseError, + isGraphQlValidationError, + pruneLeafFields, + undefinedFieldNames, +} from './graphql'; + +const UNDEFINED_ESCALATED = + "Validation error of type FieldUndefined: Field 'escalatedByUser' in type 'Ticket' is undefined"; + +describe('undefinedFieldNames', () => { + it('reads the field name from a FieldUndefined validation error', () => { + expect(undefinedFieldNames([{ message: UNDEFINED_ESCALATED }])).toEqual(['escalatedByUser']); + }); + + it('de-duplicates a field reported across several errors', () => { + expect(undefinedFieldNames([{ message: UNDEFINED_ESCALATED }, { message: UNDEFINED_ESCALATED }])).toEqual([ + 'escalatedByUser', + ]); + }); + + it('ignores errors that are not about an undefined field', () => { + expect(undefinedFieldNames([{ message: 'Internal server error' }])).toEqual([]); + }); +}); + +describe('pruneLeafFields', () => { + it('removes a plain leaf selection', () => { + const query = `{ ticket { id escalatedByUser title } }`; + expect(pruneLeafFields(query, ['escalatedByUser'])).not.toContain('escalatedByUser'); + }); + + it('keeps a field that carries a sub-selection', () => { + const query = `{ ticket { tags { id } } }`; + expect(pruneLeafFields(query, ['tags'])).toContain('tags {'); + }); + + it('keeps a field that carries arguments', () => { + const query = `{ ticket { notes(first: 5) } }`; + expect(pruneLeafFields(query, ['notes'])).toContain('notes(first: 5)'); + }); +}); + +describe('extractGraphQlData', () => { + it('throws a validation error carrying the undefined field', () => { + const call = () => + extractGraphQlData({ + ok: true, + data: { errors: [{ message: UNDEFINED_ESCALATED, extensions: { classification: 'ValidationError' } }] }, + }); + expect(call).toThrow(GraphQlResponseError); + try { + call(); + } catch (error) { + expect(isGraphQlValidationError(error)).toBe(true); + expect((error as GraphQlResponseError).undefinedFields).toEqual(['escalatedByUser']); + } + }); + + it('does not flag a plain GraphQL error as a validation error', () => { + try { + extractGraphQlData({ ok: true, data: { errors: [{ message: 'boom' }] } }); + } catch (error) { + expect(isGraphQlValidationError(error)).toBe(false); + } + }); + + it('returns the data when the response has no errors', () => { + expect(extractGraphQlData({ ok: true, data: { data: { value: 1 } } })).toEqual({ value: 1 }); + }); +}); diff --git a/src/app/(app)/tickets/utils/graphql.ts b/src/app/(app)/tickets/utils/graphql.ts index d913c53e..4511f77d 100644 --- a/src/app/(app)/tickets/utils/graphql.ts +++ b/src/app/(app)/tickets/utils/graphql.ts @@ -1,6 +1,80 @@ +export interface GraphQlError { + message: string; + extensions?: unknown; +} + export interface GraphQlResponse { data?: T; - errors?: Array<{ message: string; extensions?: unknown }>; + errors?: GraphQlError[]; +} + +/** + * A GraphQL response that carried `errors`. `classification` is graphql-java's + * error category — `ValidationError` when the deployed schema rejects the + * document — and `undefinedFields` are the field names a `FieldUndefined` + * validation error reports, so a caller can prune them and retry. + * + * The raw `message` stays off the user-facing path: a validation error names + * internal types and is not copy a user can act on. + */ +export class GraphQlResponseError extends Error { + readonly classification?: string; + readonly undefinedFields: string[]; + + constructor(message: string, options?: { classification?: string; undefinedFields?: string[] }) { + super(message); + this.name = 'GraphQlResponseError'; + this.classification = options?.classification; + this.undefinedFields = options?.undefinedFields ?? []; + } +} + +// graphql-java phrasing for a field the schema does not declare, e.g. +// "Field 'escalatedByUser' in type 'Ticket' is undefined". +const UNDEFINED_FIELD_PATTERN = /Field '([^']+)' in type '[^']+' is undefined/g; + +function classificationOf(error: GraphQlError): string | undefined { + const classification = (error.extensions as { classification?: unknown } | undefined)?.classification; + return typeof classification === 'string' ? classification : undefined; +} + +/** Field names the response's validation errors report as undefined by the schema. */ +export function undefinedFieldNames(errors: GraphQlError[]): string[] { + const names = new Set(); + for (const error of errors) { + for (const match of error.message.matchAll(UNDEFINED_FIELD_PATTERN)) names.add(match[1]); + } + return [...names]; +} + +/** True for a deterministic schema-validation failure — retrying it cannot help. */ +export function isGraphQlValidationError(error: unknown): boolean { + return ( + error instanceof GraphQlResponseError && + (error.classification === 'ValidationError' || error.undefinedFields.length > 0) + ); +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +/** + * Remove leaf field selections from a GraphQL document, so a document with one + * field the deployed schema does not declare can be retried without it. Only a + * plain scalar leaf is removed — a field with arguments, an alias or a + * sub-selection is left in place, because it cannot be dropped without knowing + * the shape it carries. + */ +export function pruneLeafFields(query: string, fields: string[]): string { + let pruned = query; + for (const field of fields) { + // The field as a whole-word selection, not followed by `(`, `{` or `:` + // (arguments, a sub-selection or an alias). + const pattern = new RegExp(`\\b${escapeRegExp(field)}\\b(?!\\s*[({:])`, 'g'); + pruned = pruned.replace(pattern, ''); + } + return pruned; } export function extractGraphQlData(response: { @@ -15,7 +89,11 @@ export function extractGraphQlData(response: { const gql = response.data; if (gql?.errors && gql.errors.length > 0) { - throw new Error(gql.errors[0].message || 'GraphQL error occurred'); + const first = gql.errors[0]; + throw new GraphQlResponseError(first.message || 'GraphQL error occurred', { + classification: classificationOf(first), + undefinedFields: undefinedFieldNames(gql.errors), + }); } if (!gql?.data) {