From 186be705bf87d8f4722d2e1dee1e40e8deab13ef Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:29:42 -0500 Subject: [PATCH 01/19] extract shared list and job detail components The Jobs list owns its empty state and its Fewer/More controls, the job detail page owns its whole layout, and `JobNotFound` owns the 404 layout. Another pane that shows jobs has to copy that markup, and the copies drift. Move the empty state, the pagination controls, and the 404 layout into their own components. Split the job detail page into `JobDetailView`, which takes its actions as a prop, so a pane with different actions can reuse the same layout. Jobs renders the same as before. --- src/components/JobDetail.tsx | 48 +++++++++++++++++++---- src/components/JobList.tsx | 64 ++++++------------------------- src/components/JobNotFound.tsx | 38 ++++++++---------- src/components/ListEmptyState.tsx | 37 ++++++++++++++++++ src/components/ListPagination.tsx | 39 +++++++++++++++++++ src/components/NotFoundPage.tsx | 40 +++++++++++++++++++ 6 files changed, 183 insertions(+), 83 deletions(-) create mode 100644 src/components/ListEmptyState.tsx create mode 100644 src/components/ListPagination.tsx create mode 100644 src/components/NotFoundPage.tsx diff --git a/src/components/JobDetail.tsx b/src/components/JobDetail.tsx index 3040348e..f366892a 100644 --- a/src/components/JobDetail.tsx +++ b/src/components/JobDetail.tsx @@ -17,7 +17,7 @@ import { Job, JobWithKnownMetadata } from "@services/jobs"; import { JobState } from "@services/types"; import { Link } from "@tanstack/react-router"; import { capitalize } from "@utils/string"; -import { FormEvent, useState } from "react"; +import { FormEvent, ReactNode, useState } from "react"; type JobDetailProps = { cancel: () => void; @@ -26,12 +26,44 @@ type JobDetailProps = { retry: () => void; }; +type JobDetailViewProps = { + actions: ReactNode; + job: Job; + // Shown under the heading, for context that applies to the whole job. + notice?: ReactNode; + title: string; +}; + export default function JobDetail({ cancel, deleteFn, job, retry, }: JobDetailProps) { + return ( + + } + job={job} + title="Job Details" + /> + ); +} + +// JobDetailView is the job detail page without its actions, so that other +// panes showing a job (like dead letters) share one layout. +export function JobDetailView({ + actions, + job, + notice, + title, +}: JobDetailViewProps) { const featureEnabledWorkflows = useFeature("ENABLE_WORKFLOWS", true); let jobWithMetadata: JobWithKnownMetadata | undefined; @@ -41,7 +73,7 @@ export default function JobDetail({ return ( <> - +
{/* Heading */} @@ -55,14 +87,14 @@ export default function JobDetail({

ID: {job.id.toString()}

+ {notice && ( +

+ {notice} +

+ )}
- + {actions}
diff --git a/src/components/JobList.tsx b/src/components/JobList.tsx index d8a347fd..024ef1da 100644 --- a/src/components/JobList.tsx +++ b/src/components/JobList.tsx @@ -1,11 +1,12 @@ import { Badge } from "@components/Badge"; -import { Button } from "@components/Button"; import ButtonForGroup from "@components/ButtonForGroup"; import ConfirmationDialog from "@components/ConfirmationDialog"; import { CustomCheckbox } from "@components/CustomCheckbox"; import { Dropdown, DropdownItem, DropdownMenu } from "@components/Dropdown"; import { Filter, JobSearch } from "@components/job-search/JobSearch"; import { JobStateFilters } from "@components/JobStateFilters"; +import ListEmptyState from "@components/ListEmptyState"; +import ListPagination from "@components/ListPagination"; import RelativeTimeFormatter from "@components/RelativeTimeFormatter"; import TopNav from "@components/TopNav"; import { useFeatures } from "@contexts/Features.hook"; @@ -152,35 +153,6 @@ const JobListItem = ({ ); }; -type EmptySetIconProps = React.ComponentProps<"svg">; - -const EmptySetIcon = (props: EmptySetIconProps) => ( - - - - -); - -const EmptyState = () => ( -
- -

- No jobs found -

-

- Try adjusting your filters to find what you're looking for. -

-
-); - export type JobRowsProps = { cancelJobs: (jobIDs: bigint[]) => void; canShowFewer: boolean; @@ -408,7 +380,10 @@ const JobRows = ({ {jobs.length === 0 ? (
- +
) : ( <> @@ -426,27 +401,12 @@ const JobRows = ({ /> ))} - + )} diff --git a/src/components/JobNotFound.tsx b/src/components/JobNotFound.tsx index 8716d857..6fbd0a63 100644 --- a/src/components/JobNotFound.tsx +++ b/src/components/JobNotFound.tsx @@ -1,7 +1,7 @@ import { JobState } from "@services/types"; import { Button } from "./Button"; -import TopNavTitleOnly from "./TopNavTitleOnly"; +import NotFoundPage from "./NotFoundPage"; type JobNotFoundProps = { jobId: bigint; @@ -9,27 +9,19 @@ type JobNotFoundProps = { export default function JobNotFound({ jobId }: JobNotFoundProps) { return ( - <> - - -
-
-

- 404 -

-

- Job not found -

-

- Sorry, there is no job with ID {jobId.toString()}. -

-
- -
-
-
- + + Return to job list + + } + description={ + <> + Sorry, there is no job with ID {jobId.toString()}. + + } + heading="Job not found" + title="Job Details" + /> ); } diff --git a/src/components/ListEmptyState.tsx b/src/components/ListEmptyState.tsx new file mode 100644 index 00000000..8e1e8bc3 --- /dev/null +++ b/src/components/ListEmptyState.tsx @@ -0,0 +1,37 @@ +import { ReactNode } from "react"; + +type EmptySetIconProps = React.ComponentProps<"svg">; + +const EmptySetIcon = (props: EmptySetIconProps) => ( + + + + +); + +type ListEmptyStateProps = { + description: ReactNode; + title: string; +}; + +const ListEmptyState = ({ description, title }: ListEmptyStateProps) => ( +
+ +

+ {title} +

+

+ {description} +

+
+); + +export default ListEmptyState; diff --git a/src/components/ListPagination.tsx b/src/components/ListPagination.tsx new file mode 100644 index 00000000..9843ce4c --- /dev/null +++ b/src/components/ListPagination.tsx @@ -0,0 +1,39 @@ +import { Button } from "@components/Button"; + +type ListPaginationProps = { + canShowFewer: boolean; + canShowMore: boolean; + showFewer: () => void; + showMore: () => void; +}; + +const ListPagination = ({ + canShowFewer, + canShowMore, + showFewer, + showMore, +}: ListPaginationProps) => ( + +); + +export default ListPagination; diff --git a/src/components/NotFoundPage.tsx b/src/components/NotFoundPage.tsx new file mode 100644 index 00000000..a1b4cfbb --- /dev/null +++ b/src/components/NotFoundPage.tsx @@ -0,0 +1,40 @@ +import { ReactNode } from "react"; + +import TopNavTitleOnly from "./TopNavTitleOnly"; + +type NotFoundPageProps = { + action: ReactNode; + description: ReactNode; + heading: string; + title: string; +}; + +export default function NotFoundPage({ + action, + description, + heading, + title, +}: NotFoundPageProps) { + return ( + <> + + +
+
+

+ 404 +

+

+ {heading} +

+

+ {description} +

+
+ {action} +
+
+
+ + ); +} From 0087ef5d557cdcaa50e51591217b56302a933c75 Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:29:42 -0500 Subject: [PATCH 02/19] abort superseded job search suggestions Each keystroke in the job search field starts a new suggestion request, but the earlier requests keep running. The server finishes a query for every one of them, which is costly when the query reads a large table. Create an `AbortController` per suggestion request and abort it when a newer request replaces it. Pass the signal to the suggestion fetchers for both Jobs and dead letters so the browser cancels the HTTP request. A test confirms that the first signal aborts when the second request starts. --- src/components/job-search/JobSearch.test.tsx | 23 ++++++++++++++++++++ src/components/job-search/JobSearch.tsx | 2 ++ src/components/job-search/api.ts | 9 +++++++- src/components/job-search/hooks.ts | 10 +++++++++ src/services/deadLetterJobs.ts | 9 ++++---- 5 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/components/job-search/JobSearch.test.tsx b/src/components/job-search/JobSearch.test.tsx index 684081b0..64b346e7 100644 --- a/src/components/job-search/JobSearch.test.tsx +++ b/src/components/job-search/JobSearch.test.tsx @@ -170,6 +170,29 @@ describe("JobSearch", () => { expect(input).toHaveValue("kind:batch"); }); + it("aborts a suggestion request when a newer one replaces it", async () => { + const signals: AbortSignal[] = []; + const fetchSuggestions = vi.fn( + ( + _type: FilterTypeId, + _query: string, + _selected: string[], + signal?: AbortSignal, + ) => { + if (signal) signals.push(signal); + return new Promise(() => {}); + }, + ); + render(); + const input = screen.getByTestId("job-search-input"); + fireEvent.change(input, { target: { value: "kind:b" } }); + await waitFor(() => expect(signals).toHaveLength(1)); + fireEvent.change(input, { target: { value: "kind:ba" } }); + await waitFor(() => expect(signals).toHaveLength(2)); + expect(signals[0].aborted).toBe(true); + expect(signals[1].aborted).toBe(false); + }); + it("notifies parent of filter changes", async () => { const onFiltersChange = vi.fn(); render(); diff --git a/src/components/job-search/JobSearch.tsx b/src/components/job-search/JobSearch.tsx index fc7b2155..887ab2f5 100644 --- a/src/components/job-search/JobSearch.tsx +++ b/src/components/job-search/JobSearch.tsx @@ -22,12 +22,14 @@ export interface JobSearchProps { * @param filterTypeId - The ID of the filter type * @param query - The search query * @param selectedValues - Values already selected for this filter + * @param signal - Aborted when a newer request replaces this one * @returns Promise resolving to an array of suggestion strings */ fetchSuggestions?: ( filterTypeId: JobFilterTypeID, query: string, selectedValues: string[], + signal?: AbortSignal, ) => Promise; /** * Initial filters to display diff --git a/src/components/job-search/api.ts b/src/components/job-search/api.ts index 93b4d107..fe4f53ad 100644 --- a/src/components/job-search/api.ts +++ b/src/components/job-search/api.ts @@ -8,6 +8,7 @@ export async function fetchSuggestions( filterTypeId: JobFilterTypeID, query: string, selectedValues: string[], + signal?: AbortSignal, ): Promise { if (filterTypeId === JobFilterTypeID.PRIORITY) { // Priority is a hardcoded list of values—we just need to filter out @@ -31,7 +32,13 @@ export async function fetchSuggestions( } if (fetchType) { - return await fetchAutocomplete(fetchType, query, undefined, selectedValues); + return await fetchAutocomplete( + fetchType, + query, + undefined, + selectedValues, + signal, + ); } return []; diff --git a/src/components/job-search/hooks.ts b/src/components/job-search/hooks.ts index b19c6625..60b1d752 100644 --- a/src/components/job-search/hooks.ts +++ b/src/components/job-search/hooks.ts @@ -21,6 +21,7 @@ interface UseFilterInputProps { filterTypeId: JobFilterTypeID, query: string, selectedValues: string[], + signal?: AbortSignal, ) => Promise; initialFilters: JobFilter[]; onFiltersChange?: (filters: JobFilter[]) => void; @@ -32,6 +33,7 @@ interface UseSuggestionsProps { filterTypeId: JobFilterTypeID, query: string, selectedValues: string[], + signal?: AbortSignal, ) => Promise; suppressSuggestions: boolean; } @@ -237,10 +239,15 @@ export function useSuggestions({ }); const latestSuggestionRequestRef = useRef(0); + const suggestionAbortRef = useRef(undefined); const updateSuggestions = useCallback( async (text: string, cursor: number) => { const requestId = ++latestSuggestionRequestRef.current; + // A newer request replaces this one, so stop the old fetch instead of + // leaving its query running on the server. + suggestionAbortRef.current?.abort(); + suggestionAbortRef.current = undefined; const context = analyzeAutocompleteContext( text, cursor, @@ -289,10 +296,13 @@ export function useSuggestions({ let suggestions: string[] = []; if (context.type === "filter-value" && context.filterTypeId) { + const abortController = new AbortController(); + suggestionAbortRef.current = abortController; suggestions = await fetchSuggestions( context.filterTypeId, context.currentToken || "", context.existingValues || [], + abortController.signal, ); } diff --git a/src/services/deadLetterJobs.ts b/src/services/deadLetterJobs.ts index ea1bcfa8..7a8d5b2e 100644 --- a/src/services/deadLetterJobs.ts +++ b/src/services/deadLetterJobs.ts @@ -95,12 +95,13 @@ export async function fetchDeadLetterSuggestions( facet: "job_kind" | "queue_name", match: string, exclude: string[], + signal?: AbortSignal, ): Promise { const query = new URLSearchParams({ facet, match }); exclude.forEach((value) => query.append("exclude", value)); - const response = await API.get>({ - path: "/pro/dead-letter-jobs/autocomplete", - query, - }); + const response = await API.get>( + { path: "/pro/dead-letter-jobs/autocomplete", query }, + { signal }, + ); return response.data; } From 600501c8f78892874bd218b003ff1fbaa0b3e631 Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:29:42 -0500 Subject: [PATCH 03/19] rebuild dead letters pane on the Jobs layout In the light theme, most text in the dead letters pane is invisible. The root style sets a near-white default text color, and the list headers, list cells, detail heading, and detail labels set no color of their own. The pane also uses its own table and detail layout: on a phone only the first column is visible, and the detail page omits the state, priority, tags, and timeline that the Jobs pane shows. Render the list with the same rows as the Jobs list (state dot, kind, time, attempts, queue badge), the shared empty state, and the shared pagination controls. Render the detail page with `JobDetailView` and a single Retry action, and the not-found view with the shared 404 layout. Every element now sets a color for both themes, and the layout follows the responsive behavior of the Jobs pane. The empty state now says how to enable the dead letter queue, because the pane appears for every River Pro install whether or not the queue is on. Add stories for both components. --- src/components/DeadLetterJobDetail.stories.ts | 29 +++ src/components/DeadLetterJobDetail.tsx | 90 ++----- src/components/DeadLetterJobList.stories.ts | 57 +++++ src/components/DeadLetterJobList.tsx | 231 ++++++++++-------- src/routes/dead-letter-jobs.test.tsx | 2 + src/routes/dead-letter-jobs/$jobId.tsx | 29 ++- src/services/jobs.ts | 3 + 7 files changed, 249 insertions(+), 192 deletions(-) create mode 100644 src/components/DeadLetterJobDetail.stories.ts create mode 100644 src/components/DeadLetterJobList.stories.ts diff --git a/src/components/DeadLetterJobDetail.stories.ts b/src/components/DeadLetterJobDetail.stories.ts new file mode 100644 index 00000000..65ace602 --- /dev/null +++ b/src/components/DeadLetterJobDetail.stories.ts @@ -0,0 +1,29 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; + +import { jobFactory } from "@test/factories/job"; + +import DeadLetterJobDetail from "./DeadLetterJobDetail"; + +const meta: Meta = { + component: DeadLetterJobDetail, + title: "Pages/DeadLetterJobDetail", +}; + +export default meta; + +type Story = StoryObj; + +export const Default: Story = { + args: { + job: jobFactory.discarded().build(), + retry: () => {}, + retryPending: false, + }, +}; + +export const Retrying: Story = { + args: { + ...Default.args, + retryPending: true, + }, +}; diff --git a/src/components/DeadLetterJobDetail.tsx b/src/components/DeadLetterJobDetail.tsx index 825df79a..a6955e3f 100644 --- a/src/components/DeadLetterJobDetail.tsx +++ b/src/components/DeadLetterJobDetail.tsx @@ -1,12 +1,8 @@ import { ArrowUturnLeftIcon } from "@heroicons/react/24/outline"; import { Job } from "@services/jobs"; -import { Button } from "./Button"; -import JobAttempts from "./JobAttempts"; -import JSONTextView from "./JSONTextView"; -import JSONView from "./JSONView"; -import RelativeTimeFormatter from "./RelativeTimeFormatter"; -import TopNavTitleOnly from "./TopNavTitleOnly"; +import ButtonForGroup from "./ButtonForGroup"; +import { JobDetailView } from "./JobDetail"; export default function DeadLetterJobDetail({ job, @@ -18,72 +14,20 @@ export default function DeadLetterJobDetail({ retryPending: boolean; }) { return ( - <> - -
- -
-
-

{job.kind}

-

- ID: {job.id.toString()} -

-
- -
-

- Retrying removes this job from the dead letter queue and enqueues a - new job with the same args and a fresh attempt history. -

-
-
-
Queue
-
{job.queue}
-
-
-
Attempts
-
- {job.attempt} / {job.maxAttempts} -
-
-
-
Created
-
- -
-
-
-
Discarded
-
- {job.finalizedAt ? ( - - ) : ( - "–" - )} -
-
-
-
-
-
Args
-
- -
-
-
-
Metadata
-
- -
-
- -
-
- + + + + } + job={job} + notice="Retrying removes this job from the dead letter queue and enqueues a new job with the same args and a fresh attempt history." + title="Dead Letter Job" + /> ); } diff --git a/src/components/DeadLetterJobList.stories.ts b/src/components/DeadLetterJobList.stories.ts new file mode 100644 index 00000000..3ad5c1f8 --- /dev/null +++ b/src/components/DeadLetterJobList.stories.ts @@ -0,0 +1,57 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; + +import { FilterTypeId } from "@components/job-search/JobSearch"; +import { jobMinimalFactory } from "@test/factories/job"; + +import DeadLetterJobList from "./DeadLetterJobList"; + +const meta: Meta = { + component: DeadLetterJobList, + title: "Pages/DeadLetterJobList", +}; + +export default meta; + +type Story = StoryObj; + +export const Default: Story = { + args: { + canShowFewer: false, + canShowMore: true, + initialFilters: [], + jobs: jobMinimalFactory.discarded().buildList(10), + loading: false, + onFiltersChange: () => {}, + showFewer: () => {}, + showMore: () => {}, + }, +}; + +export const Empty: Story = { + args: { + ...Default.args, + canShowMore: false, + jobs: [], + }, +}; + +export const NoFilterMatches: Story = { + args: { + ...Empty.args, + initialFilters: [ + { + id: FilterTypeId.KIND, + match: "kind:", + typeId: FilterTypeId.KIND, + values: ["missing"], + }, + ], + }, +}; + +export const RefreshFailed: Story = { + args: { + ...Default.args, + loadError: "Couldn't refresh dead letter jobs: Failed to fetch", + }, +}; diff --git a/src/components/DeadLetterJobList.tsx b/src/components/DeadLetterJobList.tsx index 61312a92..f8e0059b 100644 --- a/src/components/DeadLetterJobList.tsx +++ b/src/components/DeadLetterJobList.tsx @@ -1,18 +1,78 @@ +import { Badge } from "@components/Badge"; import { fetchSuggestions } from "@components/job-search/api"; import { Filter, FilterTypeId, JobSearch, } from "@components/job-search/JobSearch"; +import ListEmptyState from "@components/ListEmptyState"; +import ListPagination from "@components/ListPagination"; +import RelativeTimeFormatter from "@components/RelativeTimeFormatter"; +import TopNavTitleOnly from "@components/TopNavTitleOnly"; import { DeadLetterJobListItem, fetchDeadLetterSuggestions, } from "@services/deadLetterJobs"; import { Link } from "@tanstack/react-router"; -import { Button } from "./Button"; -import RelativeTimeFormatter from "./RelativeTimeFormatter"; -import TopNavTitleOnly from "./TopNavTitleOnly"; +type DeadLetterJobListProps = { + canShowFewer: boolean; + canShowMore: boolean; + filterError?: string; + initialFilters: Filter[]; + jobs: DeadLetterJobListItem[]; + // Set when a refresh fails. The last loaded jobs stay visible. + loadError?: string; + loading: boolean; + onFiltersChange: (filters: Filter[]) => void; + showFewer: () => void; + showMore: () => void; +}; + +const DeadLetterJobRow = ({ job }: { job: DeadLetterJobListItem }) => ( +
  • +
    +
    +
    +
    +
    +

    + + {job.kind}{" "} + + #{job.id.toString()} + + +

    +
    + {job.finalizedAt ? ( + + ) : ( + "–" + )} +
    +
    +
    +
    + {job.attempt.toString()} + / + {job.maxAttempts.toString()} +
    + + {job.queue} + +
    +
    +
    • +); export default function DeadLetterJobList({ canShowFewer, @@ -20,121 +80,76 @@ export default function DeadLetterJobList({ filterError, initialFilters, jobs, + loadError, loading, onFiltersChange, showFewer, showMore, -}: { - canShowFewer: boolean; - canShowMore: boolean; - filterError?: string; - initialFilters: Filter[]; - jobs: DeadLetterJobListItem[]; - loading: boolean; - onFiltersChange: (filters: Filter[]) => void; - showFewer: () => void; - showMore: () => void; -}) { +}: DeadLetterJobListProps) { + const alert = filterError ?? loadError; + return ( <> -
    -

    - Discarded jobs retained in the dead letter queue. Select a job to - inspect its failures and retry it. -

    - - {filterError && ( -

    - {filterError} -

    - )} +
    +
    +
    + +
    + {alert && ( +

    + {alert} +

    + )} +
    {loading ? ( -

    Loading…

    - ) : jobs.length === 0 ? ( -

    - {initialFilters.length - ? "No dead letter jobs match these filters." - : "No dead letter jobs."} +

    + Loading…

    + ) : jobs.length === 0 ? ( +
    + {initialFilters.length ? ( + + ) : ( + + Discarded jobs appear here when the dead letter queue is + enabled with Config.DeadLetter.Enabled in River + Pro. + + } + title="No dead letter jobs." + /> + )} +
    ) : ( <> -
    - - - - - - - - - - - {jobs.map((job) => ( - - - - - - - ))} - -
    - Job - - Queue - - Attempts - - Discarded -
    - - {job.kind}{" "} - #{job.id.toString()} - - {job.queue} - {job.attempt} / {job.maxAttempts} - - {job.finalizedAt ? ( - - ) : ( - "–" - )} -
    -
    - + {jobs.map((job) => ( + + ))} + + )}
    @@ -146,13 +161,15 @@ async function fetchSuggestionsForDeadLetters( type: FilterTypeId, query: string, selectedValues: string[], + signal?: AbortSignal, ): Promise { if (type === FilterTypeId.KIND || type === FilterTypeId.QUEUE) { return fetchDeadLetterSuggestions( type === FilterTypeId.KIND ? "job_kind" : "queue_name", query, selectedValues, + signal, ); } - return fetchSuggestions(type, query, selectedValues); + return fetchSuggestions(type, query, selectedValues, signal); } diff --git a/src/routes/dead-letter-jobs.test.tsx b/src/routes/dead-letter-jobs.test.tsx index 78c25b77..95c86334 100644 --- a/src/routes/dead-letter-jobs.test.tsx +++ b/src/routes/dead-letter-jobs.test.tsx @@ -442,6 +442,7 @@ describe("Dead letter jobs", () => { "job_kind", "archive_", [], + expect.any(AbortSignal), ), ); fireEvent.change(input, { target: { value: "queue:mail,re" } }); @@ -450,6 +451,7 @@ describe("Dead letter jobs", () => { "queue_name", "re", ["mail"], + expect.any(AbortSignal), ), ); }); diff --git a/src/routes/dead-letter-jobs/$jobId.tsx b/src/routes/dead-letter-jobs/$jobId.tsx index fea226cd..b91311aa 100644 --- a/src/routes/dead-letter-jobs/$jobId.tsx +++ b/src/routes/dead-letter-jobs/$jobId.tsx @@ -1,6 +1,6 @@ import { Button } from "@components/Button"; import DeadLetterJobDetail from "@components/DeadLetterJobDetail"; -import TopNavTitleOnly from "@components/TopNavTitleOnly"; +import NotFoundPage from "@components/NotFoundPage"; import { useRefreshSetting } from "@contexts/RefreshSettings.hook"; import { refreshQueryOptions } from "@contexts/RefreshSettings.query"; import { @@ -9,7 +9,7 @@ import { listDeadLetterJobsKey, retryDeadLetterJob, } from "@services/deadLetterJobs"; -import { getJobKey } from "@services/jobs"; +import { getJobKey, listJobsKeyPrefix } from "@services/jobs"; import { countsByStateKey } from "@services/states"; import { toastError, toastSuccess } from "@services/toast"; import { @@ -34,15 +34,16 @@ export const Route = createFileRoute("/dead-letter-jobs/$jobId")({ queryClient.ensureQueryData(jobQueryOptions), errorComponent: ({ error }) => error instanceof NotFoundError ? ( - <> - -
    -

    {error.message}

    - -
    - + } + description={error.message} + heading="Dead letter job not found" + title="Dead Letter Job" + /> ) : ( ), @@ -68,7 +69,7 @@ export function DeadLetterJobComponent() { toastSuccess({ message: "Job enqueued for retry", duration: 2000 }); await Promise.all([ queryClient.invalidateQueries({ queryKey: listDeadLetterJobsKey() }), - queryClient.invalidateQueries({ queryKey: ["listJobs"] }), + queryClient.invalidateQueries({ queryKey: listJobsKeyPrefix }), queryClient.invalidateQueries({ queryKey: countsByStateKey() }), ]); await navigate({ to: "/jobs/$jobId", params: { jobId: job.id } }); @@ -83,7 +84,11 @@ export function DeadLetterJobComponent() { throwOnError: true, }); - if (!query.data) return

    Loading…

    ; + if (!query.data) { + return ( +

    Loading…

    + ); + } return ( { return [ "listJobs", From ad186942c3ff35b1f3f7410605851131e6d37cce Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:29:42 -0500 Subject: [PATCH 04/19] show a not-found page for unknown routes No route defines a `notFoundComponent`, so the router falls back to its bare "Not Found" text. That text has no color and no top navigation, so the page looks blank in the light theme. This happens for any unknown URL, and for `/dead-letter-jobs` on a server without dead letter support, where the route throws `notFound()`. Add a `notFoundComponent` to the root route that uses the shared 404 layout and links back to the job list. --- src/routes/__root.tsx | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/routes/__root.tsx b/src/routes/__root.tsx index 8e12b44a..03f56222 100644 --- a/src/routes/__root.tsx +++ b/src/routes/__root.tsx @@ -1,3 +1,5 @@ +import { Button } from "@components/Button"; +import NotFoundPage from "@components/NotFoundPage"; import { Root } from "@components/Root"; import { type Features, featuresKey, getFeatures } from "@services/features"; import { QueryClient } from "@tanstack/react-query"; @@ -18,6 +20,20 @@ export const Route = createRootRouteWithContext<{ }; }, component: RootComponent, + // Also shown for a pane this server doesn't support, such as a Pro-only + // route opened on an OSS server. + notFoundComponent: () => ( + + Return to job list + + } + description="This page doesn't exist, or this River UI server doesn't support it." + heading="Page not found" + title="Not Found" + /> + ), }); function RootComponent() { From dbe38a30de704538cb0b94d7afec90108abe63db Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:29:42 -0500 Subject: [PATCH 05/19] open dead letters list despite bad URL filters A URL such as `/dead-letter-jobs?priority=5` or `?id=abc` fails search validation, and the route shows the generic error page. The Jobs list ignores the same values and still opens. Validate the route's search params with a schema that drops any filter it can't parse and keeps the rest. Edits in the search field still use the strict schema, so a bad value typed there shows an inline error instead of running an unfiltered query. --- src/routes/dead-letter-jobs/index.schema.ts | 11 +++++++++++ src/routes/dead-letter-jobs/index.tsx | 3 ++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/routes/dead-letter-jobs/index.schema.ts b/src/routes/dead-letter-jobs/index.schema.ts index 57bf8455..16967472 100644 --- a/src/routes/dead-letter-jobs/index.schema.ts +++ b/src/routes/dead-letter-jobs/index.schema.ts @@ -61,6 +61,17 @@ export const deadLetterJobSearchSchema = z.object({ export type DeadLetterJobSearch = z.output; +// The route drops a filter it can't parse, as the Jobs list does, so that a +// hand-edited or stale URL still opens the list. Edits in the search field +// use the strict schema above and show the problem instead. +export const deadLetterJobRouteSearchSchema = deadLetterJobSearchSchema.extend({ + id: ids.catch(undefined), + kind: strings.catch(undefined), + priority: priorities.catch(undefined), + queue: strings.catch(undefined), + tags: strings.catch(undefined), +}); + export const searchToDeadLetterFilters = ( search: DeadLetterJobSearch, ): DeadLetterJobFilters => ({ diff --git a/src/routes/dead-letter-jobs/index.tsx b/src/routes/dead-letter-jobs/index.tsx index e392d753..2be3a678 100644 --- a/src/routes/dead-letter-jobs/index.tsx +++ b/src/routes/dead-letter-jobs/index.tsx @@ -13,6 +13,7 @@ import { createFileRoute, stripSearchParams } from "@tanstack/react-router"; import { useCallback, useMemo, useState } from "react"; import { + deadLetterJobRouteSearchSchema, DeadLetterJobSearch, deadLetterJobSearchSchema, inputFiltersToSearch, @@ -30,7 +31,7 @@ const jobsQueryOptions = (search: DeadLetterJobSearch) => }); export const Route = createFileRoute("/dead-letter-jobs/")({ - validateSearch: deadLetterJobSearchSchema, + validateSearch: deadLetterJobRouteSearchSchema, search: { middlewares: [stripSearchParams({ limit: defaultDeadLetterJobsLimit })], }, From 0308f9dc9755bd275c495de05757491d65435a2a Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:29:43 -0500 Subject: [PATCH 06/19] keep dead letters visible when a refresh fails The list query sets `throwOnError`, so one failed background refresh replaces the whole pane with the error page and stops polling. A short backend restart leaves the tab broken until a reload. The Jobs list keeps its rows and continues to poll. Remove `throwOnError` from the list query and show the failure in an inline alert above the rows. The loaded jobs, the search field, and polling all stay active, so the list recovers by itself. Add route tests for this case, for a URL with filters that can't be parsed, and for the not-found page on a server without dead letter support. --- src/routes/dead-letter-jobs.test.tsx | 19 ++++++++++++++++++- src/routes/dead-letter-jobs/index.tsx | 6 +++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/routes/dead-letter-jobs.test.tsx b/src/routes/dead-letter-jobs.test.tsx index 95c86334..ef497422 100644 --- a/src/routes/dead-letter-jobs.test.tsx +++ b/src/routes/dead-letter-jobs.test.tsx @@ -1,7 +1,10 @@ import Layout from "@components/Layout"; import { FeaturesContext } from "@contexts/Features"; import { SidebarSettingProvider } from "@contexts/SidebarSetting.provider"; -import { deadLetterJobSearchSchema } from "@routes/dead-letter-jobs/index.schema"; +import { + deadLetterJobRouteSearchSchema, + deadLetterJobSearchSchema, +} from "@routes/dead-letter-jobs/index.schema"; import { listDeadLetterJobsKey } from "@services/deadLetterJobs"; import { featuresKey } from "@services/features"; import { Job, listJobs, listJobsKey } from "@services/jobs"; @@ -228,6 +231,12 @@ describe("Dead letter jobs", () => { ).not.toBeInTheDocument(); expect(listDeadLetterJobs).not.toHaveBeenCalled(); expect(getDeadLetterJob).not.toHaveBeenCalled(); + expect( + screen.getByRole("heading", { name: "Page not found" }), + ).toBeInTheDocument(); + expect( + screen.getByRole("link", { name: "Return to job list" }), + ).toHaveAttribute("href", "/jobs"); }, ); @@ -475,6 +484,14 @@ describe("Dead letter jobs", () => { expect(deadLetterJobSearchSchema.safeParse({ priority }).success).toBe( false, ); + // The route schema drops only the filters it can't parse. + expect( + deadLetterJobRouteSearchSchema.parse({ + id: "bad", + kind: "report", + priority: "5", + }), + ).toEqual({ limit: 20, kind: ["report"] }); }); it("retries through the Pro API and opens the newly created job", async () => { diff --git a/src/routes/dead-letter-jobs/index.tsx b/src/routes/dead-letter-jobs/index.tsx index 2be3a678..0212ecf5 100644 --- a/src/routes/dead-letter-jobs/index.tsx +++ b/src/routes/dead-letter-jobs/index.tsx @@ -51,7 +51,6 @@ function DeadLetterJobsIndexComponent() { const query = useQuery({ ...jobsQueryOptions(search), ...refreshQueryOptions(intervalMs), - throwOnError: true, }); const onFiltersChange = useCallback( (filters: Filter[]) => { @@ -79,6 +78,11 @@ function DeadLetterJobsIndexComponent() { filterError={filterError} initialFilters={initialFilters} jobs={query.data ?? []} + loadError={ + query.isError + ? `Couldn't refresh dead letter jobs: ${query.error.message}` + : undefined + } loading={query.isLoading} onFiltersChange={onFiltersChange} showFewer={() => From 119da0e7e54ede6a7ff8fec320fe8ab87b79596e Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:29:43 -0500 Subject: [PATCH 07/19] use fake timers in dead letters Back test The test waits 300 ms of real time for a 200 ms debounce. Under load the debounce can fire late, and then the test passes without checking that the URL stays as restored. Advance fake timers past the debounce instead. --- src/routes/dead-letter-jobs.test.tsx | 41 ++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/src/routes/dead-letter-jobs.test.tsx b/src/routes/dead-letter-jobs.test.tsx index ef497422..18de4556 100644 --- a/src/routes/dead-letter-jobs.test.tsx +++ b/src/routes/dead-letter-jobs.test.tsx @@ -33,7 +33,7 @@ import { } from "@testing-library/react"; import { APIResponseError, NotFoundError } from "@utils/api"; import qs from "query-string"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import "../global-type-overrides"; import { routeTree } from "../routeTree.gen"; @@ -129,6 +129,10 @@ async function renderRoute(path: string, pro = true) { return { queryClient, router }; } +afterEach(() => { + vi.useRealTimers(); +}); + beforeEach(() => { vi.clearAllMocks(); fetchDeadLetterSuggestions.mockResolvedValue([]); @@ -240,6 +244,37 @@ describe("Dead letter jobs", () => { }, ); + it("opens the list when a bookmarked URL has filters it can't parse", async () => { + const { router } = await renderRoute( + "/dead-letter-jobs?id=abc&priority=5&queue=mail", + ); + expect( + await screen.findByRole("link", { name: `send_email #${job.id}` }), + ).toBeInTheDocument(); + expect(screen.getByTestId("job-search-input")).toHaveValue("queue:mail"); + expect(router.state.location.search).toEqual({ queue: "mail" }); + expect(listDeadLetterJobs).toHaveBeenCalledWith( + expect.objectContaining({ + queryKey: listDeadLetterJobsKey(20, { queues: ["mail"] }), + }), + ); + }); + + it("keeps the loaded jobs visible when a refresh fails", async () => { + const { queryClient } = await renderRoute("/dead-letter-jobs"); + listDeadLetterJobs.mockRejectedValue(new Error("Failed to fetch")); + await act(async () => { + await queryClient.refetchQueries({ queryKey: listDeadLetterJobsKey() }); + }); + expect(await screen.findByRole("alert")).toHaveTextContent( + "Couldn't refresh dead letter jobs: Failed to fetch", + ); + expect( + screen.getByRole("link", { name: `send_email #${job.id}` }), + ).toBeInTheDocument(); + expect(screen.getByTestId("job-search-input")).toBeInTheDocument(); + }); + it("shows the Pro navigation and opens the selected dead letter job", async () => { const { router } = await renderRoute("/dead-letter-jobs"); expect( @@ -406,6 +441,7 @@ describe("Dead letter jobs", () => { }); it("restores the search field on browser Back", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); const { router } = await renderRoute("/dead-letter-jobs?queue=mail"); await act(async () => { await router.navigate({ @@ -422,8 +458,9 @@ describe("Dead letter jobs", () => { await waitFor(() => expect(screen.getByTestId("job-search-input")).toHaveValue("queue:mail"), ); + // Run the filter debounce to confirm it doesn't rewrite the restored URL. await act(async () => { - await new Promise((resolve) => setTimeout(resolve, 300)); + await vi.advanceTimersByTimeAsync(300); }); expect(router.state.location.search).toEqual({ queue: "mail" }); }); From 068e700fe56f01847070501742be28bdb0119a16 Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:29:43 -0500 Subject: [PATCH 08/19] share the list of retained Jobs search params The sidebar lists the Jobs search params to clear by hand, and the Jobs route lists the same params to retain. A param added to one list and not the other leaks filters between panes with no type error. Export the list from the Jobs search schema and derive both uses from it. --- src/components/Layout.tsx | 16 ++++++---------- src/routes/jobs/index.schema.ts | 11 +++++++++++ src/routes/jobs/index.tsx | 8 ++++++-- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/components/Layout.tsx b/src/components/Layout.tsx index 57c7e9c0..b54c18cb 100644 --- a/src/components/Layout.tsx +++ b/src/components/Layout.tsx @@ -16,11 +16,16 @@ import { RectangleGroupIcon, XMarkIcon, } from "@heroicons/react/24/outline"; +import { retainedJobSearchKeys } from "@routes/jobs/index.schema"; import { Link, useRouterState } from "@tanstack/react-router"; import { Fragment, PropsWithChildren, useMemo } from "react"; type LayoutProps = PropsWithChildren; +const clearedJobSearch = Object.fromEntries( + retainedJobSearchKeys.map((key) => [key, undefined]), +); + const Layout = ({ children }: LayoutProps) => { const { open: sidebarOpen, setOpen: setSidebarOpen } = useSidebarSetting(); @@ -37,16 +42,7 @@ const Layout = ({ children }: LayoutProps) => { icon: QueueListIcon, name: "Jobs", // Explicitly clear retained Jobs filters when entering from another pane. - search: inJobsPane - ? {} - : { - id: undefined, - kind: undefined, - limit: undefined, - priority: undefined, - queue: undefined, - tags: undefined, - }, + search: inJobsPane ? {} : clearedJobSearch, }, { href: "/queues", icon: InboxStackIcon, name: "Queues" }, { diff --git a/src/routes/jobs/index.schema.ts b/src/routes/jobs/index.schema.ts index ab1b4064..f4978cf7 100644 --- a/src/routes/jobs/index.schema.ts +++ b/src/routes/jobs/index.schema.ts @@ -5,6 +5,17 @@ export const minimumLimit = 20; export const defaultLimit = 20; export const maximumLimit = 200; +// Search params kept when moving between job states. Other panes clear +// them when they link to Jobs. +export const retainedJobSearchKeys = [ + "id", + "kind", + "limit", + "priority", + "queue", + "tags", +] as const; + // Define default values export const defaultValues = { limit: defaultLimit, diff --git a/src/routes/jobs/index.tsx b/src/routes/jobs/index.tsx index 69c4fe0f..268afbce 100644 --- a/src/routes/jobs/index.tsx +++ b/src/routes/jobs/index.tsx @@ -6,7 +6,11 @@ import { refreshQueryOptions, } from "@contexts/RefreshSettings.query"; import { useRetryJobs } from "@hooks/use-retry-jobs"; -import { defaultValues, jobSearchSchema } from "@routes/jobs/index.schema"; +import { + defaultValues, + jobSearchSchema, + retainedJobSearchKeys, +} from "@routes/jobs/index.schema"; import { cancelJobs, deleteJobs, @@ -50,7 +54,7 @@ export const Route = createFileRoute("/jobs/")({ search: { middlewares: [ stripSearchParams(defaultValues), - retainSearchParams(["id", "kind", "limit", "priority", "queue", "tags"]), + retainSearchParams([...retainedJobSearchKeys]), ], }, beforeLoad: async ({ context }) => { From 96191580d128364f20ef0b557b07c5b15f129660 Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:29:43 -0500 Subject: [PATCH 09/19] move Dead Letters below Workflows in sidebar Jobs, Queues, and Workflows are the panes operators open every day. Dead Letters is used less often, so list it after them. --- src/components/Layout.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/Layout.tsx b/src/components/Layout.tsx index b54c18cb..a5634788 100644 --- a/src/components/Layout.tsx +++ b/src/components/Layout.tsx @@ -45,6 +45,12 @@ const Layout = ({ children }: LayoutProps) => { search: inJobsPane ? {} : clearedJobSearch, }, { href: "/queues", icon: InboxStackIcon, name: "Queues" }, + { + hidden: !features.workflowQueries, + href: "/workflows", + icon: RectangleGroupIcon, + name: "Workflows", + }, { hidden: !features.deadLetterJobs, href: "/dead-letter-jobs", @@ -52,12 +58,6 @@ const Layout = ({ children }: LayoutProps) => { name: "Dead Letters", search: {}, }, - { - hidden: !features.workflowQueries, - href: "/workflows", - icon: RectangleGroupIcon, - name: "Workflows", - }, { hidden: !features.durablePeriodicJobs, href: "/periodic-jobs", From 00cf1b6fd2c9b2b1b6a32493a1ba2a625723f7dd Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:33:48 -0500 Subject: [PATCH 10/19] truncate long job kinds in the detail heading A long job kind has no break points, so on the job detail page it runs past the edge of the heading and pushes the action buttons out of place. The workflow detail page already solves this for workflow names: it cuts the name with an ellipsis, shows the full name in a tooltip, and puts a copy button next to it. Move that heading into a shared component and use it for the job kind on the job detail page, which the dead letter detail page also uses. The heading stays on one line at every width, and the copy button gives the full kind. --- src/components/CopyableHeadingText.tsx | 62 ++++++++++++++++++++++++++ src/components/JobDetail.tsx | 9 ++-- src/components/WorkflowDetail.tsx | 57 ++--------------------- 3 files changed, 69 insertions(+), 59 deletions(-) create mode 100644 src/components/CopyableHeadingText.tsx diff --git a/src/components/CopyableHeadingText.tsx b/src/components/CopyableHeadingText.tsx new file mode 100644 index 00000000..abb0dc44 --- /dev/null +++ b/src/components/CopyableHeadingText.tsx @@ -0,0 +1,62 @@ +import { CheckIcon } from "@heroicons/react/16/solid"; +import { ClipboardIcon } from "@heroicons/react/24/outline"; +import { toastSuccess } from "@services/toast"; +import { capitalize } from "@utils/string"; +import { useState } from "react"; + +type CopyableHeadingTextProps = { + // Names what is copied, in lowercase, such as "job kind". + label: string; + text: string; +}; + +// CopyableHeadingText keeps a heading on one line. Text that doesn't fit +// is cut with an ellipsis, and the copy button and tooltip give the full +// text. +export default function CopyableHeadingText({ + label, + text, +}: CopyableHeadingTextProps) { + const [copied, setCopied] = useState(false); + + const copyToClipboard = () => { + if (!navigator.clipboard?.writeText) { + console.error(`Failed to copy ${label}: Clipboard API unavailable`); + return; + } + + navigator.clipboard.writeText(text).then( + () => { + setCopied(true); + toastSuccess({ message: `${capitalize(label)} copied to clipboard` }); + setTimeout(() => setCopied(false), 2000); + }, + (error: unknown) => { + console.error(`Failed to copy ${label} to clipboard:`, error); + }, + ); + }; + + return ( + + + {text} + + + + ); +} diff --git a/src/components/JobDetail.tsx b/src/components/JobDetail.tsx index f366892a..ce26a29e 100644 --- a/src/components/JobDetail.tsx +++ b/src/components/JobDetail.tsx @@ -1,6 +1,7 @@ import { Badge } from "@components/Badge"; import ButtonForGroup from "@components/ButtonForGroup"; import ConfirmationDialog from "@components/ConfirmationDialog"; +import CopyableHeadingText from "@components/CopyableHeadingText"; import JobAttempts from "@components/JobAttempts"; import JobTimeline from "@components/JobTimeline"; import JSONTextView from "@components/JSONTextView"; @@ -78,11 +79,9 @@ export function JobDetailView({
    {/* Heading */}
    -
    -

    - - {job.kind} - +
    +

    +

    ID: {job.id.toString()} diff --git a/src/components/WorkflowDetail.tsx b/src/components/WorkflowDetail.tsx index 3dfeaf72..fd3ad668 100644 --- a/src/components/WorkflowDetail.tsx +++ b/src/components/WorkflowDetail.tsx @@ -1,4 +1,5 @@ import ButtonForGroup from "@components/ButtonForGroup"; +import CopyableHeadingText from "@components/CopyableHeadingText"; import { DurationCompact } from "@components/DurationCompact"; import { Subheading } from "@components/Heading"; import { RunningSpinnerIcon } from "@components/icons/jobStateIcons"; @@ -18,12 +19,7 @@ import WorkflowGateInspector, { } from "@components/WorkflowGateInspector"; import { useFeatures } from "@contexts/Features.hook"; // (Dialog is now encapsulated in RetryWorkflowDialog) -import { CheckIcon } from "@heroicons/react/16/solid"; -import { - ArrowPathIcon, - ClipboardIcon, - LinkIcon, -} from "@heroicons/react/24/outline"; +import { ArrowPathIcon, LinkIcon } from "@heroicons/react/24/outline"; import { CheckCircleIcon, PlayCircleIcon, @@ -31,7 +27,6 @@ import { TrashIcon, XCircleIcon, } from "@heroicons/react/24/solid"; -import { toastSuccess } from "@services/toast"; import { type Heroicon, JobState } from "@services/types"; import { Workflow, @@ -122,7 +117,6 @@ export default function WorkflowDetail({ // Modal state for retry const [retryOpen, setRetryOpen] = useState(false); const [retryMode, setRetryMode] = useState(); - const [workflowNameCopied, setWorkflowNameCopied] = useState(false); if (!features.workflowQueries) { return ( @@ -156,52 +150,7 @@ export default function WorkflowDetail({

    - - - {workflowName} - - - +

    ID: {workflow.id} From 9bbcfca72ba2a8c8cbf9536d8352614d4dedf2a5 Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:36:52 -0500 Subject: [PATCH 11/19] update changelog for dead letter fixes The dead letters entry promises bounded database queries, but only the number of returned rows is bounded: a filter can still read the whole table. Remove that claim, say that the list is newest first, and add entries for the user-visible fixes outside the new pane. --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08615905..ccf6a658 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add a Pro-only dead letter queue panel with ID, kind, queue, priority, and tag filters, dead letter autocomplete, bounded database queries, and More/Fewer controls to inspect discarded jobs and retry them as new jobs. [PR #711](https://github.com/riverqueue/riverui/pull/711). +- Add a Pro-only dead letter queue panel that lists discarded jobs newest first, with ID, kind, queue, priority, and tag filters, dead letter autocomplete, and More/Fewer controls to inspect jobs and retry them as new jobs. [PR #711](https://github.com/riverqueue/riverui/pull/711). ### Changed @@ -17,6 +17,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Show a "Page not found" page with a link back to Jobs for unknown URLs, instead of a blank page. [PR #711](https://github.com/riverqueue/riverui/pull/711). +- Keep a long job kind on one line in the job detail heading, with a tooltip and a copy button for the full kind. [PR #711](https://github.com/riverqueue/riverui/pull/711). +- Cancel job search suggestion requests that a newer keystroke has replaced. [PR #711](https://github.com/riverqueue/riverui/pull/711). - Clear filters and pagination when switching between Jobs and Dead Letters, while preserving them when switching job states within Jobs. [PR #711](https://github.com/riverqueue/riverui/pull/711). - Show queues whose concurrency metadata is missing, `null`, or `{}` as having no concurrency override, matching how River Pro interprets them, and stop showing a zero concurrency config for producers that report none. [PR #704](https://github.com/riverqueue/riverui/pull/704). - Reject negative queue concurrency `global_limit` and `local_limit` values with a 400 error instead of storing them. [PR #704](https://github.com/riverqueue/riverui/pull/704). From 1416ca394ce1c68c7a4589433030d59de0065885 Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:26:54 -0500 Subject: [PATCH 12/19] list dead letter jobs newest first `JobDeadLetterList` orders by ID ascending and caps the result at 200 rows, so once the dead letter table holds more rows than that the most recently dead-lettered jobs can't be reached from the UI at all, and those are the ones an operator is most likely to be looking for. Order both the limited subquery and the JSON aggregate by `id DESC` in the PostgreSQL and SQLite queries so the limit keeps the newest rows. SQLite makes no promise that `json_group_array` preserves the order of an ordered subquery, so both drivers also sort the decoded rows by ID descending through one shared helper, which makes the final order deterministic and identical across databases. The shared driver suite now inserts its rows in ascending order and expects them back descending at every limit, and the list endpoint test asserts that the most recently inserted job comes first. --- .../river_ui_driver_interface.go | 15 +++++++++++-- .../riveruidrivertest/job_dead_letter.go | 22 +++++++++---------- .../internal/dbsqlc/river_job_dead_letter.sql | 10 ++++----- .../dbsqlc/river_job_dead_letter.sql.go | 10 ++++----- .../river_ui_postgres_driver.go | 1 + .../internal/dbsqlc/river_job_dead_letter.sql | 8 ++++--- .../dbsqlc/river_job_dead_letter.sql.go | 8 ++++--- .../riveruisqlite/river_ui_sqlite_driver.go | 1 + .../prohandler/dead_letter_endpoints_test.go | 7 +++--- 9 files changed, 50 insertions(+), 32 deletions(-) diff --git a/internal/riveruidriver/river_ui_driver_interface.go b/internal/riveruidriver/river_ui_driver_interface.go index 97826751..7dc808bc 100644 --- a/internal/riveruidriver/river_ui_driver_interface.go +++ b/internal/riveruidriver/river_ui_driver_interface.go @@ -5,7 +5,9 @@ package riveruidriver import ( + "cmp" "context" + "slices" "time" "github.com/riverqueue/river/riverdriver" @@ -28,8 +30,10 @@ type Executor interface { // JobCountEstimate returns inexpensive database estimates where supported. // Drivers without an estimate strategy return an empty map. JobCountEstimate(ctx context.Context, params *JobCountEstimateParams) (map[rivertype.JobState]JobCountEstimateResult, error) - // JobDeadLetterList returns at most Max jobs, ordered by ID ascending. - // Only Pro endpoints call this query; it requires the dead letter table. + // JobDeadLetterList returns at most Max jobs, ordered by ID descending so + // the newest dead letter jobs come first and stay reachable when more jobs + // match than Max. Only Pro endpoints call this query; it requires the dead + // letter table. JobDeadLetterList(ctx context.Context, params *JobDeadLetterListParams) ([]*JobDeadLetterListResult, error) JobDeadLetterAutocomplete(ctx context.Context, params *JobDeadLetterAutocompleteParams) ([]string, error) } @@ -69,6 +73,13 @@ type JobDeadLetterListResult struct { Queue string `json:"queue"` } +// SortJobDeadLetterListResults orders jobs by ID descending. Drivers apply it +// after decoding because not every database guarantees the order of elements +// within an aggregated JSON array. +func SortJobDeadLetterListResults(jobs []*JobDeadLetterListResult) { + slices.SortFunc(jobs, func(a, b *JobDeadLetterListResult) int { return cmp.Compare(b.ID, a.ID) }) +} + type JobCountByAllStatesCappedParams struct { // Max must be positive and less than math.MaxInt32 so callers get the same // behavior from every database implementation. diff --git a/internal/riveruidriver/riveruidrivertest/job_dead_letter.go b/internal/riveruidriver/riveruidrivertest/job_dead_letter.go index 302cc9e3..7aa47f0d 100644 --- a/internal/riveruidriver/riveruidrivertest/job_dead_letter.go +++ b/internal/riveruidriver/riveruidrivertest/job_dead_letter.go @@ -49,13 +49,13 @@ func exerciseJobDeadLetterList( require.Empty(t, jobs, "live jobs must not appear in the dead letter list") require.NotNil(t, jobs) - // Deliberately insert in reverse order, with IDs beyond JavaScript's + // Deliberately insert out of result order, with IDs beyond JavaScript's // exact integer range, and enough rows to exceed the maximum UI limit. const firstID int64 = 1970670598291982290 rows := make([]string, 0, 205) - for i := 204; i >= 0; i-- { + for i := range 205 { finalizedAt := "'2026-10-21 10:02:03.456+00:00'" - if i == 0 { + if i == 204 { finalizedAt = "NULL" } rows = append(rows, fmt.Sprintf("(%d, 3, %s, 'send_email', 5, 'mail')", firstID+int64(i), finalizedAt)) @@ -67,7 +67,7 @@ func exerciseJobDeadLetterList( require.NoError(t, err) require.Len(t, jobs, limit) for i, job := range jobs { - require.Equal(t, firstID+int64(i), job.ID) + require.Equal(t, firstID+204-int64(i), job.ID, "newest jobs must come first") require.Equal(t, 3, job.Attempt) require.Equal(t, 5, job.MaxAttempts) require.Equal(t, originalKind, job.Kind) @@ -87,11 +87,11 @@ func exerciseJobDeadLetterList( // Filtering must happen before limiting, without rounding large IDs. jobs, err = executor.JobDeadLetterList(ctx, &riveruidriver.JobDeadLetterListParams{ - Filters: riveruidriver.JobDeadLetterFilters{IDs: []int64{firstID + 204}}, Max: 1, Schema: schema, + Filters: riveruidriver.JobDeadLetterFilters{IDs: []int64{firstID}}, Max: 1, Schema: schema, }) require.NoError(t, err) require.Len(t, jobs, 1) - require.Equal(t, firstID+204, jobs[0].ID) + require.Equal(t, firstID, jobs[0].ID) archive := func(kind, queue string, priority int, tags []string) int64 { job := testfactory.Job(ctx, t, executor, &testfactory.JobOpts{ @@ -108,16 +108,16 @@ func exerciseJobDeadLetterList( filters riveruidriver.JobDeadLetterFilters want []int64 }{ - {name: "IDs", filters: riveruidriver.JobDeadLetterFilters{IDs: []int64{second, first}}, want: []int64{first, second}}, - {name: "Kinds", filters: riveruidriver.JobDeadLetterFilters{Kinds: []string{emailKind, reportKind}}, want: []int64{first, second, third}}, + {name: "IDs", filters: riveruidriver.JobDeadLetterFilters{IDs: []int64{first, second}}, want: []int64{second, first}}, + {name: "Kinds", filters: riveruidriver.JobDeadLetterFilters{Kinds: []string{emailKind, reportKind}}, want: []int64{third, second, first}}, {name: "Queues", filters: riveruidriver.JobDeadLetterFilters{Queues: []string{reportsQueue}}, want: []int64{second}}, - {name: "Priorities", filters: riveruidriver.JobDeadLetterFilters{Priorities: []int16{1, 3}}, want: []int64{first, second}}, - {name: "TagsMatchAny", filters: riveruidriver.JobDeadLetterFilters{Tags: []string{urgentTag, "batch"}}, want: []int64{first, second}}, + {name: "Priorities", filters: riveruidriver.JobDeadLetterFilters{Priorities: []int16{1, 3}}, want: []int64{second, first}}, + {name: "TagsMatchAny", filters: riveruidriver.JobDeadLetterFilters{Tags: []string{urgentTag, "batch"}}, want: []int64{second, first}}, {name: "TagsAreExact", filters: riveruidriver.JobDeadLetterFilters{Tags: []string{urgentTag}}, want: []int64{first}}, {name: "Combined", filters: riveruidriver.JobDeadLetterFilters{Kinds: []string{emailKind, reportKind}, Queues: []string{mailQueue}, Priorities: []int16{1, 2}, Tags: []string{customerTag}}, want: []int64{first}}, {name: "NoMatches", filters: riveruidriver.JobDeadLetterFilters{Kinds: []string{"missing"}}, want: []int64{}}, {name: "LiteralValues", filters: riveruidriver.JobDeadLetterFilters{Tags: []string{"urgent' OR 1=1 --"}}, want: []int64{}}, - {name: "EmptyFilters", filters: riveruidriver.JobDeadLetterFilters{IDs: []int64{}, Kinds: []string{}, Queues: []string{}, Tags: []string{}, Priorities: []int16{}}, want: []int64{first, second, third}}, + {name: "EmptyFilters", filters: riveruidriver.JobDeadLetterFilters{IDs: []int64{}, Kinds: []string{}, Queues: []string{}, Tags: []string{}, Priorities: []int16{}}, want: []int64{firstID + 204, firstID + 203, firstID + 202}}, } { t.Run(tt.name, func(t *testing.T) { jobs, err := executor.JobDeadLetterList(ctx, &riveruidriver.JobDeadLetterListParams{Filters: tt.filters, Max: 3, Schema: schema}) diff --git a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql index f9f2bf5b..59708739 100644 --- a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql +++ b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql @@ -12,9 +12,9 @@ CREATE TABLE river_job_dead_letter ( ); -- name: JobDeadLetterList :one --- Apply the limit before aggregation so neither the database nor the browser --- materializes the entire queue. River's common executor exposes QueryRow, --- so return the bounded set as one JSON value. +-- Return the newest jobs first. Apply the limit before aggregation so neither +-- the database nor the browser materializes the entire queue. River's common +-- executor exposes QueryRow, so return the bounded set as one JSON value. SELECT coalesce(json_agg(json_build_object( 'id', limited.id, 'attempt', limited.attempt, @@ -22,7 +22,7 @@ SELECT coalesce(json_agg(json_build_object( 'kind', limited.kind, 'max_attempts', limited.max_attempts, 'queue', limited.queue -) ORDER BY limited.id), '[]'::json)::text AS jobs +) ORDER BY limited.id DESC), '[]'::json)::text AS jobs FROM ( SELECT id, attempt, finalized_at, kind, max_attempts, queue FROM /* TEMPLATE: schema */river_job_dead_letter @@ -31,7 +31,7 @@ FROM ( AND (NOT jsonb_exists(@filters::jsonb, 'priorities') OR priority IN (SELECT value::smallint FROM jsonb_array_elements_text(@filters::jsonb -> 'priorities'))) AND (NOT jsonb_exists(@filters::jsonb, 'queues') OR queue IN (SELECT value::text FROM jsonb_array_elements_text(@filters::jsonb -> 'queues'))) AND (NOT jsonb_exists(@filters::jsonb, 'tags') OR tags && ARRAY(SELECT jsonb_array_elements_text(@filters::jsonb -> 'tags'))::varchar[]) - ORDER BY id + ORDER BY id DESC LIMIT @max::int ) AS limited; diff --git a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql.go b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql.go index add724a0..5c8679b7 100644 --- a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql.go +++ b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql.go @@ -49,7 +49,7 @@ SELECT coalesce(json_agg(json_build_object( 'kind', limited.kind, 'max_attempts', limited.max_attempts, 'queue', limited.queue -) ORDER BY limited.id), '[]'::json)::text AS jobs +) ORDER BY limited.id DESC), '[]'::json)::text AS jobs FROM ( SELECT id, attempt, finalized_at, kind, max_attempts, queue FROM /* TEMPLATE: schema */river_job_dead_letter @@ -58,7 +58,7 @@ FROM ( AND (NOT jsonb_exists($1::jsonb, 'priorities') OR priority IN (SELECT value::smallint FROM jsonb_array_elements_text($1::jsonb -> 'priorities'))) AND (NOT jsonb_exists($1::jsonb, 'queues') OR queue IN (SELECT value::text FROM jsonb_array_elements_text($1::jsonb -> 'queues'))) AND (NOT jsonb_exists($1::jsonb, 'tags') OR tags && ARRAY(SELECT jsonb_array_elements_text($1::jsonb -> 'tags'))::varchar[]) - ORDER BY id + ORDER BY id DESC LIMIT $2::int ) AS limited ` @@ -68,9 +68,9 @@ type JobDeadLetterListParams struct { Max int32 } -// Apply the limit before aggregation so neither the database nor the browser -// materializes the entire queue. River's common executor exposes QueryRow, -// so return the bounded set as one JSON value. +// Return the newest jobs first. Apply the limit before aggregation so neither +// the database nor the browser materializes the entire queue. River's common +// executor exposes QueryRow, so return the bounded set as one JSON value. func (q *Queries) JobDeadLetterList(ctx context.Context, db DBTX, arg *JobDeadLetterListParams) (string, error) { row := db.QueryRow(ctx, jobDeadLetterList, arg.Filters, arg.Max) var jobs string diff --git a/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go b/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go index d62d5ada..e0933430 100644 --- a/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go +++ b/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go @@ -156,6 +156,7 @@ func (e *Executor) JobDeadLetterList(ctx context.Context, params *riveruidriver. if err := json.Unmarshal([]byte(rawJobs), &jobs); err != nil { return nil, fmt.Errorf("error decoding dead letter jobs: %w", err) } + riveruidriver.SortJobDeadLetterListResults(jobs) return jobs, nil } diff --git a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql index 6295e003..19d47f35 100644 --- a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql +++ b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql @@ -12,8 +12,10 @@ CREATE TABLE river_job_dead_letter ( ); -- name: JobDeadLetterList :one --- Apply the limit before aggregation so neither the database nor the browser --- materializes the entire queue. Normalize SQLite timestamps to RFC 3339 for +-- Return the newest jobs first. Apply the limit before aggregation so neither +-- the database nor the browser materializes the entire queue. SQLite doesn't +-- guarantee that json_group_array preserves the subquery's order, so the +-- driver sorts the decoded rows. Normalize SQLite timestamps to RFC 3339 for -- the shared result decoder; strftime preserves NULL timestamps. SELECT CAST(json_group_array(json_object( 'id', limited.id, @@ -31,7 +33,7 @@ FROM ( AND (json_type(CAST(@filters AS text), '$.priorities') IS NULL OR priority IN (SELECT CAST(value AS integer) FROM json_each(CAST(@filters AS text), '$.priorities'))) AND (json_type(CAST(@filters AS text), '$.queues') IS NULL OR queue IN (SELECT CAST(value AS text) FROM json_each(CAST(@filters AS text), '$.queues'))) AND (json_type(CAST(@filters AS text), '$.tags') IS NULL OR EXISTS (SELECT 1 FROM json_each(river_job_dead_letter.tags) AS job_tag JOIN json_each(CAST(@filters AS text), '$.tags') AS filter_tag ON job_tag.value = filter_tag.value)) - ORDER BY id + ORDER BY id DESC LIMIT @max ) AS limited; diff --git a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql.go b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql.go index 84d6d0a3..be4e5ed6 100644 --- a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql.go +++ b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql.go @@ -61,7 +61,7 @@ FROM ( AND (json_type(CAST(?1 AS text), '$.priorities') IS NULL OR priority IN (SELECT CAST(value AS integer) FROM json_each(CAST(?1 AS text), '$.priorities'))) AND (json_type(CAST(?1 AS text), '$.queues') IS NULL OR queue IN (SELECT CAST(value AS text) FROM json_each(CAST(?1 AS text), '$.queues'))) AND (json_type(CAST(?1 AS text), '$.tags') IS NULL OR EXISTS (SELECT 1 FROM json_each(river_job_dead_letter.tags) AS job_tag JOIN json_each(CAST(?1 AS text), '$.tags') AS filter_tag ON job_tag.value = filter_tag.value)) - ORDER BY id + ORDER BY id DESC LIMIT ?2 ) AS limited ` @@ -71,8 +71,10 @@ type JobDeadLetterListParams struct { Max int64 } -// Apply the limit before aggregation so neither the database nor the browser -// materializes the entire queue. Normalize SQLite timestamps to RFC 3339 for +// Return the newest jobs first. Apply the limit before aggregation so neither +// the database nor the browser materializes the entire queue. SQLite doesn't +// guarantee that json_group_array preserves the subquery's order, so the +// driver sorts the decoded rows. Normalize SQLite timestamps to RFC 3339 for // the shared result decoder; strftime preserves NULL timestamps. func (q *Queries) JobDeadLetterList(ctx context.Context, db DBTX, arg *JobDeadLetterListParams) (string, error) { row := db.QueryRowContext(ctx, jobDeadLetterList, arg.Filters, arg.Max) diff --git a/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go b/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go index bb4ec784..7861569c 100644 --- a/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go +++ b/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go @@ -258,6 +258,7 @@ func (e *Executor) JobDeadLetterList(ctx context.Context, params *riveruidriver. if err := json.Unmarshal([]byte(rawJobs), &jobs); err != nil { return nil, fmt.Errorf("error decoding dead letter jobs: %w", err) } + riveruidriver.SortJobDeadLetterListResults(jobs) return jobs, nil } diff --git a/riverproui/internal/prohandler/dead_letter_endpoints_test.go b/riverproui/internal/prohandler/dead_letter_endpoints_test.go index f149d285..eb125e21 100644 --- a/riverproui/internal/prohandler/dead_letter_endpoints_test.go +++ b/riverproui/internal/prohandler/dead_letter_endpoints_test.go @@ -68,17 +68,18 @@ func TestDeadLetterJobList(t *testing.T) { encoded, err = json.Marshal(resp) require.NoError(t, err) require.Contains(t, string(encoded), fmt.Sprintf(`"id":"%d"`, job.ID)) + var newest *rivertype.JobRow for range 24 { - insertDeadLetterJob(ctx, t, bundle) + newest = insertDeadLetterJob(ctx, t, bundle) } resp, err = apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobListRequest{}) require.NoError(t, err) require.Len(t, resp.Data, 20) - require.Equal(t, job.ID, resp.Data[0].ID) + require.Equal(t, newest.ID, resp.Data[0].ID, "newest jobs must come first") resp, err = apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobListRequest{Limit: new(2)}) require.NoError(t, err) require.Len(t, resp.Data, 2) - require.Less(t, resp.Data[0].ID, resp.Data[1].ID) + require.Greater(t, resp.Data[0].ID, resp.Data[1].ID) resp, err = apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobListRequest{Limit: new(40)}) require.NoError(t, err) require.Len(t, resp.Data, 25) From 6d2525abd41006483b2f7f1494ab387456c6df15 Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:29:10 -0500 Subject: [PATCH 13/19] use primary key for dead letter ID lookups The dead letter list filters IDs with a predicate of the form "no IDs requested OR `id IN (subquery)`". Neither database can use that as an index condition, so looking up a single job by ID walks the entire table: on a 200k row table PostgreSQL scans the whole primary key and discards every row but one with a filter, and SQLite reports a `SCAN`. The cost grows with the size of the dead letter table even though the request names the exact rows it wants. Add a `JobDeadLetterListByIDs` query to both drivers in which the ID predicate stands alone as a top-level condition, and have the drivers choose it whenever the filters include IDs. `JobDeadLetterList` drops its ID predicate and keeps serving every other request. On PostgreSQL the requested IDs collapse into an array (`id = ANY (ARRAY(...))`), which the planner evaluates once and uses as an index condition on the primary key under both custom and generic plans, so the result doesn't depend on how the statement's parameters are bound. On SQLite the plain `id IN (...)` becomes a `SEARCH` on the integer primary key. The remaining filters keep their existing form in both queries. The shared driver suite gains cases that combine IDs with another filter and that look up an ID with no matching row. --- .../riveruidrivertest/job_dead_letter.go | 2 + .../internal/dbsqlc/river_job_dead_letter.sql | 30 ++++++++++++- .../dbsqlc/river_job_dead_letter.sql.go | 45 ++++++++++++++++++- .../internal/dbsqlc/river_queries.go | 8 ++++ .../river_ui_postgres_driver.go | 8 +++- .../internal/dbsqlc/river_job_dead_letter.sql | 28 +++++++++++- .../dbsqlc/river_job_dead_letter.sql.go | 43 +++++++++++++++++- .../internal/dbsqlc/river_queries.go | 8 ++++ .../riveruisqlite/river_ui_sqlite_driver.go | 8 +++- 9 files changed, 172 insertions(+), 8 deletions(-) diff --git a/internal/riveruidriver/riveruidrivertest/job_dead_letter.go b/internal/riveruidriver/riveruidrivertest/job_dead_letter.go index 7aa47f0d..53ee5a23 100644 --- a/internal/riveruidriver/riveruidrivertest/job_dead_letter.go +++ b/internal/riveruidriver/riveruidrivertest/job_dead_letter.go @@ -109,6 +109,8 @@ func exerciseJobDeadLetterList( want []int64 }{ {name: "IDs", filters: riveruidriver.JobDeadLetterFilters{IDs: []int64{first, second}}, want: []int64{second, first}}, + {name: "IDsWithOtherFilters", filters: riveruidriver.JobDeadLetterFilters{IDs: []int64{first, second, third}, Queues: []string{mailQueue}}, want: []int64{third, first}}, + {name: "IDsNoMatches", filters: riveruidriver.JobDeadLetterFilters{IDs: []int64{firstID - 1}}, want: []int64{}}, {name: "Kinds", filters: riveruidriver.JobDeadLetterFilters{Kinds: []string{emailKind, reportKind}}, want: []int64{third, second, first}}, {name: "Queues", filters: riveruidriver.JobDeadLetterFilters{Queues: []string{reportsQueue}}, want: []int64{second}}, {name: "Priorities", filters: riveruidriver.JobDeadLetterFilters{Priorities: []int16{1, 3}}, want: []int64{second, first}}, diff --git a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql index 59708739..de1136dd 100644 --- a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql +++ b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql @@ -15,6 +15,9 @@ CREATE TABLE river_job_dead_letter ( -- Return the newest jobs first. Apply the limit before aggregation so neither -- the database nor the browser materializes the entire queue. River's common -- executor exposes QueryRow, so return the bounded set as one JSON value. +-- +-- This query ignores the filter's IDs; the driver sends ID lookups to +-- JobDeadLetterListByIDs instead. SELECT coalesce(json_agg(json_build_object( 'id', limited.id, 'attempt', limited.attempt, @@ -26,7 +29,32 @@ SELECT coalesce(json_agg(json_build_object( FROM ( SELECT id, attempt, finalized_at, kind, max_attempts, queue FROM /* TEMPLATE: schema */river_job_dead_letter - WHERE (NOT jsonb_exists(@filters::jsonb, 'ids') OR id IN (SELECT value::bigint FROM jsonb_array_elements_text(@filters::jsonb -> 'ids'))) + WHERE (NOT jsonb_exists(@filters::jsonb, 'kinds') OR kind IN (SELECT value::text FROM jsonb_array_elements_text(@filters::jsonb -> 'kinds'))) + AND (NOT jsonb_exists(@filters::jsonb, 'priorities') OR priority IN (SELECT value::smallint FROM jsonb_array_elements_text(@filters::jsonb -> 'priorities'))) + AND (NOT jsonb_exists(@filters::jsonb, 'queues') OR queue IN (SELECT value::text FROM jsonb_array_elements_text(@filters::jsonb -> 'queues'))) + AND (NOT jsonb_exists(@filters::jsonb, 'tags') OR tags && ARRAY(SELECT jsonb_array_elements_text(@filters::jsonb -> 'tags'))::varchar[]) + ORDER BY id DESC + LIMIT @max::int +) AS limited; + +-- name: JobDeadLetterListByIDs :one +-- JobDeadLetterList for filters that include IDs. The ID predicate must stand +-- alone as a top-level condition: written as "no IDs requested OR id IN (...)" +-- the planner can't turn it into an index condition and reads the whole +-- table. Collapsing the requested IDs to an array lets it probe the primary +-- key once per ID, whether the statement runs with a custom or generic plan. +SELECT coalesce(json_agg(json_build_object( + 'id', limited.id, + 'attempt', limited.attempt, + 'finalized_at', limited.finalized_at, + 'kind', limited.kind, + 'max_attempts', limited.max_attempts, + 'queue', limited.queue +) ORDER BY limited.id DESC), '[]'::json)::text AS jobs +FROM ( + SELECT id, attempt, finalized_at, kind, max_attempts, queue + FROM /* TEMPLATE: schema */river_job_dead_letter + WHERE id = ANY (ARRAY(SELECT value::bigint FROM jsonb_array_elements_text(@filters::jsonb -> 'ids'))) AND (NOT jsonb_exists(@filters::jsonb, 'kinds') OR kind IN (SELECT value::text FROM jsonb_array_elements_text(@filters::jsonb -> 'kinds'))) AND (NOT jsonb_exists(@filters::jsonb, 'priorities') OR priority IN (SELECT value::smallint FROM jsonb_array_elements_text(@filters::jsonb -> 'priorities'))) AND (NOT jsonb_exists(@filters::jsonb, 'queues') OR queue IN (SELECT value::text FROM jsonb_array_elements_text(@filters::jsonb -> 'queues'))) diff --git a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql.go b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql.go index 5c8679b7..891d2fa2 100644 --- a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql.go +++ b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql.go @@ -53,8 +53,7 @@ SELECT coalesce(json_agg(json_build_object( FROM ( SELECT id, attempt, finalized_at, kind, max_attempts, queue FROM /* TEMPLATE: schema */river_job_dead_letter - WHERE (NOT jsonb_exists($1::jsonb, 'ids') OR id IN (SELECT value::bigint FROM jsonb_array_elements_text($1::jsonb -> 'ids'))) - AND (NOT jsonb_exists($1::jsonb, 'kinds') OR kind IN (SELECT value::text FROM jsonb_array_elements_text($1::jsonb -> 'kinds'))) + WHERE (NOT jsonb_exists($1::jsonb, 'kinds') OR kind IN (SELECT value::text FROM jsonb_array_elements_text($1::jsonb -> 'kinds'))) AND (NOT jsonb_exists($1::jsonb, 'priorities') OR priority IN (SELECT value::smallint FROM jsonb_array_elements_text($1::jsonb -> 'priorities'))) AND (NOT jsonb_exists($1::jsonb, 'queues') OR queue IN (SELECT value::text FROM jsonb_array_elements_text($1::jsonb -> 'queues'))) AND (NOT jsonb_exists($1::jsonb, 'tags') OR tags && ARRAY(SELECT jsonb_array_elements_text($1::jsonb -> 'tags'))::varchar[]) @@ -71,9 +70,51 @@ type JobDeadLetterListParams struct { // Return the newest jobs first. Apply the limit before aggregation so neither // the database nor the browser materializes the entire queue. River's common // executor exposes QueryRow, so return the bounded set as one JSON value. +// +// This query ignores the filter's IDs; the driver sends ID lookups to +// JobDeadLetterListByIDs instead. func (q *Queries) JobDeadLetterList(ctx context.Context, db DBTX, arg *JobDeadLetterListParams) (string, error) { row := db.QueryRow(ctx, jobDeadLetterList, arg.Filters, arg.Max) var jobs string err := row.Scan(&jobs) return jobs, err } + +const jobDeadLetterListByIDs = `-- name: JobDeadLetterListByIDs :one +SELECT coalesce(json_agg(json_build_object( + 'id', limited.id, + 'attempt', limited.attempt, + 'finalized_at', limited.finalized_at, + 'kind', limited.kind, + 'max_attempts', limited.max_attempts, + 'queue', limited.queue +) ORDER BY limited.id DESC), '[]'::json)::text AS jobs +FROM ( + SELECT id, attempt, finalized_at, kind, max_attempts, queue + FROM /* TEMPLATE: schema */river_job_dead_letter + WHERE id = ANY (ARRAY(SELECT value::bigint FROM jsonb_array_elements_text($1::jsonb -> 'ids'))) + AND (NOT jsonb_exists($1::jsonb, 'kinds') OR kind IN (SELECT value::text FROM jsonb_array_elements_text($1::jsonb -> 'kinds'))) + AND (NOT jsonb_exists($1::jsonb, 'priorities') OR priority IN (SELECT value::smallint FROM jsonb_array_elements_text($1::jsonb -> 'priorities'))) + AND (NOT jsonb_exists($1::jsonb, 'queues') OR queue IN (SELECT value::text FROM jsonb_array_elements_text($1::jsonb -> 'queues'))) + AND (NOT jsonb_exists($1::jsonb, 'tags') OR tags && ARRAY(SELECT jsonb_array_elements_text($1::jsonb -> 'tags'))::varchar[]) + ORDER BY id DESC + LIMIT $2::int +) AS limited +` + +type JobDeadLetterListByIDsParams struct { + Filters []byte + Max int32 +} + +// JobDeadLetterList for filters that include IDs. The ID predicate must stand +// alone as a top-level condition: written as "no IDs requested OR id IN (...)" +// the planner can't turn it into an index condition and reads the whole +// table. Collapsing the requested IDs to an array lets it probe the primary +// key once per ID, whether the statement runs with a custom or generic plan. +func (q *Queries) JobDeadLetterListByIDs(ctx context.Context, db DBTX, arg *JobDeadLetterListByIDsParams) (string, error) { + row := db.QueryRow(ctx, jobDeadLetterListByIDs, arg.Filters, arg.Max) + var jobs string + err := row.Scan(&jobs) + return jobs, err +} diff --git a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_queries.go b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_queries.go index 6021c0d2..f28f55f1 100644 --- a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_queries.go +++ b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_queries.go @@ -95,6 +95,14 @@ func JobDeadLetterListRiver(ctx context.Context, db RowQuerier, maxRows int, fil return jobs, nil } +func JobDeadLetterListByIDsRiver(ctx context.Context, db RowQuerier, maxRows int, filters string) (string, error) { + var jobs string + if err := db.QueryRow(ctx, jobDeadLetterListByIDs, filters, maxRows).Scan(&jobs); err != nil { + return "", err + } + return jobs, nil +} + func JobDeadLetterAutocompleteRiver(ctx context.Context, db RowQuerier, facet, match, exclude string, maxRows int) (string, error) { var values string if err := db.QueryRow(ctx, jobDeadLetterAutocomplete, facet, match, exclude, maxRows).Scan(&values); err != nil { diff --git a/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go b/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go index e0933430..8f5a8bb6 100644 --- a/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go +++ b/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go @@ -148,7 +148,13 @@ func (e *Executor) JobDeadLetterList(ctx context.Context, params *riveruidriver. if err != nil { return nil, fmt.Errorf("error encoding dead letter filters: %w", err) } - rawJobs, err := dbsqlc.JobDeadLetterListRiver(schemaTemplateParam(ctx, params.Schema), e.dbtx, params.Max, string(filters)) + // ID lookups use their own query so the database can search the primary + // key instead of reading the whole table. + listQuery := dbsqlc.JobDeadLetterListRiver + if len(params.Filters.IDs) > 0 { + listQuery = dbsqlc.JobDeadLetterListByIDsRiver + } + rawJobs, err := listQuery(schemaTemplateParam(ctx, params.Schema), e.dbtx, params.Max, string(filters)) if err != nil { return nil, fmt.Errorf("error listing dead letter jobs: %w", err) } diff --git a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql index 19d47f35..a403bba4 100644 --- a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql +++ b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql @@ -17,6 +17,9 @@ CREATE TABLE river_job_dead_letter ( -- guarantee that json_group_array preserves the subquery's order, so the -- driver sorts the decoded rows. Normalize SQLite timestamps to RFC 3339 for -- the shared result decoder; strftime preserves NULL timestamps. +-- +-- This query ignores the filter's IDs; the driver sends ID lookups to +-- JobDeadLetterListByIDs instead. SELECT CAST(json_group_array(json_object( 'id', limited.id, 'attempt', limited.attempt, @@ -28,7 +31,30 @@ SELECT CAST(json_group_array(json_object( FROM ( SELECT id, attempt, finalized_at, kind, max_attempts, queue FROM /* TEMPLATE: schema */river_job_dead_letter - WHERE (json_type(CAST(@filters AS text), '$.ids') IS NULL OR id IN (SELECT CAST(value AS integer) FROM json_each(CAST(@filters AS text), '$.ids'))) + WHERE (json_type(CAST(@filters AS text), '$.kinds') IS NULL OR kind IN (SELECT CAST(value AS text) FROM json_each(CAST(@filters AS text), '$.kinds'))) + AND (json_type(CAST(@filters AS text), '$.priorities') IS NULL OR priority IN (SELECT CAST(value AS integer) FROM json_each(CAST(@filters AS text), '$.priorities'))) + AND (json_type(CAST(@filters AS text), '$.queues') IS NULL OR queue IN (SELECT CAST(value AS text) FROM json_each(CAST(@filters AS text), '$.queues'))) + AND (json_type(CAST(@filters AS text), '$.tags') IS NULL OR EXISTS (SELECT 1 FROM json_each(river_job_dead_letter.tags) AS job_tag JOIN json_each(CAST(@filters AS text), '$.tags') AS filter_tag ON job_tag.value = filter_tag.value)) + ORDER BY id DESC + LIMIT @max +) AS limited; + +-- name: JobDeadLetterListByIDs :one +-- JobDeadLetterList for filters that include IDs. The ID predicate must stand +-- alone as a top-level condition: written as "no IDs requested OR id IN (...)" +-- SQLite scans the whole table instead of searching the primary key. +SELECT CAST(json_group_array(json_object( + 'id', limited.id, + 'attempt', limited.attempt, + 'finalized_at', strftime('%Y-%m-%dT%H:%M:%fZ', limited.finalized_at), + 'kind', limited.kind, + 'max_attempts', limited.max_attempts, + 'queue', limited.queue +)) AS text) AS jobs +FROM ( + SELECT id, attempt, finalized_at, kind, max_attempts, queue + FROM /* TEMPLATE: schema */river_job_dead_letter + WHERE id IN (SELECT CAST(value AS integer) FROM json_each(CAST(@filters AS text), '$.ids')) AND (json_type(CAST(@filters AS text), '$.kinds') IS NULL OR kind IN (SELECT CAST(value AS text) FROM json_each(CAST(@filters AS text), '$.kinds'))) AND (json_type(CAST(@filters AS text), '$.priorities') IS NULL OR priority IN (SELECT CAST(value AS integer) FROM json_each(CAST(@filters AS text), '$.priorities'))) AND (json_type(CAST(@filters AS text), '$.queues') IS NULL OR queue IN (SELECT CAST(value AS text) FROM json_each(CAST(@filters AS text), '$.queues'))) diff --git a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql.go b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql.go index be4e5ed6..ebf52cf0 100644 --- a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql.go +++ b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql.go @@ -56,8 +56,7 @@ SELECT CAST(json_group_array(json_object( FROM ( SELECT id, attempt, finalized_at, kind, max_attempts, queue FROM /* TEMPLATE: schema */river_job_dead_letter - WHERE (json_type(CAST(?1 AS text), '$.ids') IS NULL OR id IN (SELECT CAST(value AS integer) FROM json_each(CAST(?1 AS text), '$.ids'))) - AND (json_type(CAST(?1 AS text), '$.kinds') IS NULL OR kind IN (SELECT CAST(value AS text) FROM json_each(CAST(?1 AS text), '$.kinds'))) + WHERE (json_type(CAST(?1 AS text), '$.kinds') IS NULL OR kind IN (SELECT CAST(value AS text) FROM json_each(CAST(?1 AS text), '$.kinds'))) AND (json_type(CAST(?1 AS text), '$.priorities') IS NULL OR priority IN (SELECT CAST(value AS integer) FROM json_each(CAST(?1 AS text), '$.priorities'))) AND (json_type(CAST(?1 AS text), '$.queues') IS NULL OR queue IN (SELECT CAST(value AS text) FROM json_each(CAST(?1 AS text), '$.queues'))) AND (json_type(CAST(?1 AS text), '$.tags') IS NULL OR EXISTS (SELECT 1 FROM json_each(river_job_dead_letter.tags) AS job_tag JOIN json_each(CAST(?1 AS text), '$.tags') AS filter_tag ON job_tag.value = filter_tag.value)) @@ -76,9 +75,49 @@ type JobDeadLetterListParams struct { // guarantee that json_group_array preserves the subquery's order, so the // driver sorts the decoded rows. Normalize SQLite timestamps to RFC 3339 for // the shared result decoder; strftime preserves NULL timestamps. +// +// This query ignores the filter's IDs; the driver sends ID lookups to +// JobDeadLetterListByIDs instead. func (q *Queries) JobDeadLetterList(ctx context.Context, db DBTX, arg *JobDeadLetterListParams) (string, error) { row := db.QueryRowContext(ctx, jobDeadLetterList, arg.Filters, arg.Max) var jobs string err := row.Scan(&jobs) return jobs, err } + +const jobDeadLetterListByIDs = `-- name: JobDeadLetterListByIDs :one +SELECT CAST(json_group_array(json_object( + 'id', limited.id, + 'attempt', limited.attempt, + 'finalized_at', strftime('%Y-%m-%dT%H:%M:%fZ', limited.finalized_at), + 'kind', limited.kind, + 'max_attempts', limited.max_attempts, + 'queue', limited.queue +)) AS text) AS jobs +FROM ( + SELECT id, attempt, finalized_at, kind, max_attempts, queue + FROM /* TEMPLATE: schema */river_job_dead_letter + WHERE id IN (SELECT CAST(value AS integer) FROM json_each(CAST(?1 AS text), '$.ids')) + AND (json_type(CAST(?1 AS text), '$.kinds') IS NULL OR kind IN (SELECT CAST(value AS text) FROM json_each(CAST(?1 AS text), '$.kinds'))) + AND (json_type(CAST(?1 AS text), '$.priorities') IS NULL OR priority IN (SELECT CAST(value AS integer) FROM json_each(CAST(?1 AS text), '$.priorities'))) + AND (json_type(CAST(?1 AS text), '$.queues') IS NULL OR queue IN (SELECT CAST(value AS text) FROM json_each(CAST(?1 AS text), '$.queues'))) + AND (json_type(CAST(?1 AS text), '$.tags') IS NULL OR EXISTS (SELECT 1 FROM json_each(river_job_dead_letter.tags) AS job_tag JOIN json_each(CAST(?1 AS text), '$.tags') AS filter_tag ON job_tag.value = filter_tag.value)) + ORDER BY id DESC + LIMIT ?2 +) AS limited +` + +type JobDeadLetterListByIDsParams struct { + Filters string + Max int64 +} + +// JobDeadLetterList for filters that include IDs. The ID predicate must stand +// alone as a top-level condition: written as "no IDs requested OR id IN (...)" +// SQLite scans the whole table instead of searching the primary key. +func (q *Queries) JobDeadLetterListByIDs(ctx context.Context, db DBTX, arg *JobDeadLetterListByIDsParams) (string, error) { + row := db.QueryRowContext(ctx, jobDeadLetterListByIDs, arg.Filters, arg.Max) + var jobs string + err := row.Scan(&jobs) + return jobs, err +} diff --git a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_queries.go b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_queries.go index 7376e0b9..71023799 100644 --- a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_queries.go +++ b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_queries.go @@ -45,6 +45,14 @@ func JobDeadLetterListRiver(ctx context.Context, db RowQuerier, maxRows int, fil return jobs, nil } +func JobDeadLetterListByIDsRiver(ctx context.Context, db RowQuerier, maxRows int, filters string) (string, error) { + var jobs string + if err := db.QueryRow(ctx, jobDeadLetterListByIDs, filters, maxRows).Scan(&jobs); err != nil { + return "", err + } + return jobs, nil +} + func JobDeadLetterAutocompleteRiver(ctx context.Context, db RowQuerier, facet, match, exclude string, maxRows int) (string, error) { var values string if err := db.QueryRow(ctx, jobDeadLetterAutocomplete, facet, match, exclude, maxRows).Scan(&values); err != nil { diff --git a/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go b/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go index 7861569c..7d5bf2e1 100644 --- a/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go +++ b/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go @@ -250,7 +250,13 @@ func (e *Executor) JobDeadLetterList(ctx context.Context, params *riveruidriver. if err != nil { return nil, fmt.Errorf("error encoding dead letter filters: %w", err) } - rawJobs, err := dbsqlc.JobDeadLetterListRiver(schemaTemplateParam(ctx, params.Schema), e.dbtx, params.Max, string(filters)) + // ID lookups use their own query so the database can search the primary + // key instead of reading the whole table. + listQuery := dbsqlc.JobDeadLetterListRiver + if len(params.Filters.IDs) > 0 { + listQuery = dbsqlc.JobDeadLetterListByIDsRiver + } + rawJobs, err := listQuery(schemaTemplateParam(ctx, params.Schema), e.dbtx, params.Max, string(filters)) if err != nil { return nil, fmt.Errorf("error listing dead letter jobs: %w", err) } From f760bf00bbcfb73b2ba62f30322a89f69384ddf1 Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:30:23 -0500 Subject: [PATCH 14/19] reject NUL and invalid UTF-8 in dead letter text A dead letter request such as `?kinds=%00` passes validation and reaches the database. The filters travel as JSON, where the NUL byte becomes `\u0000`, and PostgreSQL refuses that in a `jsonb` cast, so the request fails with a 500. The same applies to invalid UTF-8, which PostgreSQL rejects in text parameters like the autocomplete `match`. SQLite accepts all of these, so the two databases also disagree about whether the request is valid. Check `kinds`, `queues`, and `tags` on the list request and `match` and `exclude` on the autocomplete request while extracting them from the query string, and return a bad request error naming the parameter when a value contains a NUL byte or isn't valid UTF-8. Rejecting the input before it reaches a query makes the response a 400 on every database. Other control characters and non-ASCII text remain valid. Tests cover each parameter with lone and embedded NUL bytes and with invalid UTF-8 sequences, and send requests through mounted endpoints against PostgreSQL to assert the 400 status. --- .../prohandler/dead_letter_endpoints.go | 29 +++++++- .../prohandler/dead_letter_endpoints_test.go | 71 +++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/riverproui/internal/prohandler/dead_letter_endpoints.go b/riverproui/internal/prohandler/dead_letter_endpoints.go index 9676e0f3..1757e13a 100644 --- a/riverproui/internal/prohandler/dead_letter_endpoints.go +++ b/riverproui/internal/prohandler/dead_letter_endpoints.go @@ -6,6 +6,8 @@ import ( "fmt" "net/http" "strconv" + "strings" + "unicode/utf8" "github.com/riverqueue/apiframe/apiendpoint" "github.com/riverqueue/apiframe/apierror" @@ -94,9 +96,31 @@ type deadLetterJobListRequest struct { Limit *int `json:"-" validate:"omitempty,min=1,max=200"` } +// validateDeadLetterText rejects text that the databases can't handle +// consistently. PostgreSQL refuses NUL bytes and invalid UTF-8 in both text and +// jsonb parameters, which would otherwise surface as an internal server error, +// while SQLite accepts them. +func validateDeadLetterText(param string, values ...string) error { + for _, value := range values { + if strings.ContainsRune(value, 0) || !utf8.ValidString(value) { + return apierror.NewBadRequestf("Invalid `%s` value: must be valid UTF-8 without NUL bytes.", param) + } + } + return nil +} + func (req *deadLetterJobListRequest) ExtractRaw(r *http.Request) error { query := r.URL.Query() req.Kinds, req.Queues, req.Tags = query["kinds"], query["queues"], query["tags"] + if err := validateDeadLetterText("kinds", req.Kinds...); err != nil { + return err + } + if err := validateDeadLetterText("queues", req.Queues...); err != nil { + return err + } + if err := validateDeadLetterText("tags", req.Tags...); err != nil { + return err + } for _, value := range query["ids"] { id, err := strconv.ParseInt(value, 10, 64) if err != nil { @@ -238,7 +262,10 @@ type deadLetterAutocompleteRequest struct { func (req *deadLetterAutocompleteRequest) ExtractRaw(r *http.Request) error { query := r.URL.Query() req.Facet, req.Match, req.Exclude = query.Get("facet"), query.Get("match"), query["exclude"] - return nil + if err := validateDeadLetterText("match", req.Match); err != nil { + return err + } + return validateDeadLetterText("exclude", req.Exclude...) } func (a *deadLetterAutocompleteEndpoint[TTx]) Execute(ctx context.Context, req *deadLetterAutocompleteRequest) (*listResponse[string], error) { diff --git a/riverproui/internal/prohandler/dead_letter_endpoints_test.go b/riverproui/internal/prohandler/dead_letter_endpoints_test.go index eb125e21..8cd0dfa6 100644 --- a/riverproui/internal/prohandler/dead_letter_endpoints_test.go +++ b/riverproui/internal/prohandler/dead_letter_endpoints_test.go @@ -12,6 +12,8 @@ import ( "github.com/stretchr/testify/require" + "github.com/riverqueue/apiframe/apiendpoint" + "github.com/riverqueue/apiframe/apierror" "github.com/riverqueue/apiframe/apitest" "github.com/riverqueue/river/riverdriver" "github.com/riverqueue/river/rivertype" @@ -213,6 +215,75 @@ func TestDeadLetterJobListRequest(t *testing.T) { require.Equal(t, []string{"urgent", "batch"}, parsed.Tags) } +func TestDeadLetterRequestRejectsInvalidText(t *testing.T) { + t.Parallel() + + extractList := func(r *http.Request) error { return (&deadLetterJobListRequest{}).ExtractRaw(r) } + extractAutocomplete := func(r *http.Request) error { return (&deadLetterAutocompleteRequest{}).ExtractRaw(r) } + for _, tt := range []struct { + param string + extract func(r *http.Request) error + }{ + {param: "kinds", extract: extractList}, + {param: "queues", extract: extractList}, + {param: "tags", extract: extractList}, + {param: "match", extract: extractAutocomplete}, + {param: "exclude", extract: extractAutocomplete}, + } { + // A NUL byte alone and within text, then invalid UTF-8 sequences. + for _, value := range []string{"%00", "send%00email", "%ff", "caf%c3"} { + t.Run(tt.param+"="+value, func(t *testing.T) { + t.Parallel() + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/?facet=job_kind&kinds=valid&"+tt.param+"="+value, nil) + err := tt.extract(req) + var badRequest *apierror.BadRequest + require.ErrorAs(t, err, &badRequest) + require.Equal(t, http.StatusBadRequest, badRequest.StatusCode) + require.Equal(t, "Invalid `"+tt.param+"` value: must be valid UTF-8 without NUL bytes.", badRequest.Message) + }) + } + } + + // Other control characters and non-ASCII text are valid in both databases. + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/?kinds=caf%C3%A9&queues=tab%09bed&tags=%C3%BCber", nil) + var parsed deadLetterJobListRequest + require.NoError(t, parsed.ExtractRaw(req)) + require.Equal(t, []string{"café"}, parsed.Kinds) + require.Equal(t, []string{"tab\tbed"}, parsed.Queues) + require.Equal(t, []string{"über"}, parsed.Tags) +} + +// Requests go through the mounted endpoint so that a value PostgreSQL rejects +// is shown to produce a bad request rather than an internal server error. +func TestDeadLetterInvalidTextStatus(t *testing.T) { + t.Parallel() + ctx := t.Context() + + listEndpoint, _ := setupEndpoint(ctx, t, NewDeadLetterJobListEndpoint) + autocompleteEndpoint, _ := setupEndpoint(ctx, t, NewDeadLetterAutocompleteEndpoint) + mux := http.NewServeMux() + apiendpoint.Mount(mux, listEndpoint, testMountOpts(t)) + apiendpoint.Mount(mux, autocompleteEndpoint, testMountOpts(t)) + + for _, tt := range []struct { + path string + want int + }{ + {path: "/api/pro/dead-letter-jobs?kinds=%00", want: http.StatusBadRequest}, + {path: "/api/pro/dead-letter-jobs?queues=%00", want: http.StatusBadRequest}, + {path: "/api/pro/dead-letter-jobs?tags=%ff", want: http.StatusBadRequest}, + {path: "/api/pro/dead-letter-jobs?kinds=caf%C3%A9&tags=tab%09bed", want: http.StatusOK}, + {path: "/api/pro/dead-letter-jobs/autocomplete?facet=job_kind&match=%00", want: http.StatusBadRequest}, + {path: "/api/pro/dead-letter-jobs/autocomplete?facet=job_kind&match=%ff", want: http.StatusBadRequest}, + {path: "/api/pro/dead-letter-jobs/autocomplete?facet=job_kind&exclude=%00", want: http.StatusBadRequest}, + {path: "/api/pro/dead-letter-jobs/autocomplete?facet=job_kind&match=caf%C3%A9", want: http.StatusOK}, + } { + recorder := httptest.NewRecorder() + mux.ServeHTTP(recorder, httptest.NewRequestWithContext(ctx, http.MethodGet, tt.path, nil)) + require.Equal(t, tt.want, recorder.Code, "%s: %s", tt.path, recorder.Body.String()) + } +} + func TestDeadLetterAutocomplete(t *testing.T) { t.Parallel() ctx := t.Context() From e3836b96c906e663b5898f55080f9aebcbd4cbd2 Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:32:49 -0500 Subject: [PATCH 15/19] align dead letter queries across drivers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dead letter autocomplete behaves differently depending on the database. SQLite's `lower()` folds only ASCII letters and orders values by their bytes, while PostgreSQL follows the collation of the database and its columns. With a linguistic collation, a `match` of `é` also returns `École`, and suggestions come back in dictionary order, which changes which values survive the limit as well as how they are sorted. Pin the PostgreSQL autocomplete query to `COLLATE "C"` for the case folding of both the candidate and `match`, for the exclusion check, and for the ordering before the limit and inside the aggregate. That gives ASCII-only folding and byte ordering, the same as SQLite. Both drivers also sort the decoded suggestions, since SQLite doesn't guarantee the order of `json_group_array`. The list query has a related difference: PostgreSQL renders `finalized_at` in the session's time zone and SQLite renders it in UTC. The shared normalization that sorts decoded jobs now also converts `finalized_at` to UTC, so both drivers return the same representation of the same instant. One difference remains and is documented rather than changed: SQLite reports milliseconds where PostgreSQL reports microseconds. The shared driver suite gains an autocomplete group with mixed-case and non-ASCII values. On PostgreSQL it gives the columns an ICU collation when one is available, so the cases fail without the explicit collation even when the test database itself uses "C". --- .../river_ui_driver_interface.go | 24 ++++- .../riveruidrivertest/job_dead_letter.go | 102 ++++++++++++++++-- .../riveruidrivertest/riveruidrivertest.go | 1 + .../internal/dbsqlc/river_job_dead_letter.sql | 20 ++-- .../dbsqlc/river_job_dead_letter.sql.go | 20 ++-- .../river_ui_postgres_driver.go | 6 +- .../internal/dbsqlc/river_job_dead_letter.sql | 9 +- .../dbsqlc/river_job_dead_letter.sql.go | 9 +- .../riveruisqlite/river_ui_sqlite_driver.go | 6 +- 9 files changed, 170 insertions(+), 27 deletions(-) diff --git a/internal/riveruidriver/river_ui_driver_interface.go b/internal/riveruidriver/river_ui_driver_interface.go index 7dc808bc..762d35d1 100644 --- a/internal/riveruidriver/river_ui_driver_interface.go +++ b/internal/riveruidriver/river_ui_driver_interface.go @@ -35,6 +35,12 @@ type Executor interface { // match than Max. Only Pro endpoints call this query; it requires the dead // letter table. JobDeadLetterList(ctx context.Context, params *JobDeadLetterListParams) ([]*JobDeadLetterListResult, error) + // JobDeadLetterAutocomplete returns at most Max distinct values of the + // facet that start with Match and aren't in Exclude. Every driver compares + // and orders values the same way regardless of database collation: Match + // ignores the case of ASCII letters only, and values sort by their UTF-8 + // bytes. Only Pro endpoints call this query; it requires the dead letter + // table. JobDeadLetterAutocomplete(ctx context.Context, params *JobDeadLetterAutocompleteParams) ([]string, error) } @@ -64,6 +70,9 @@ type JobDeadLetterListParams struct { // JobDeadLetterListResult contains only fields displayed in the list. Args, // metadata, and attempt history are loaded separately when opening a job. +// +// FinalizedAt is in UTC. Its precision is the one difference between drivers: +// PostgreSQL returns microseconds while SQLite returns milliseconds. type JobDeadLetterListResult struct { ID int64 `json:"id"` Attempt int `json:"attempt"` @@ -73,10 +82,17 @@ type JobDeadLetterListResult struct { Queue string `json:"queue"` } -// SortJobDeadLetterListResults orders jobs by ID descending. Drivers apply it -// after decoding because not every database guarantees the order of elements -// within an aggregated JSON array. -func SortJobDeadLetterListResults(jobs []*JobDeadLetterListResult) { +// NormalizeJobDeadLetterListResults puts decoded jobs into the form that every +// driver returns. Jobs are ordered by ID descending, because not every database +// guarantees the order of elements within an aggregated JSON array. Timestamps +// are in UTC, because PostgreSQL renders them in the session's time zone while +// SQLite renders them in UTC. +func NormalizeJobDeadLetterListResults(jobs []*JobDeadLetterListResult) { + for _, job := range jobs { + if job.FinalizedAt != nil { + *job.FinalizedAt = job.FinalizedAt.UTC() + } + } slices.SortFunc(jobs, func(a, b *JobDeadLetterListResult) int { return cmp.Compare(b.ID, a.ID) }) } diff --git a/internal/riveruidriver/riveruidrivertest/job_dead_letter.go b/internal/riveruidriver/riveruidrivertest/job_dead_letter.go index 53ee5a23..9fcdd5eb 100644 --- a/internal/riveruidriver/riveruidrivertest/job_dead_letter.go +++ b/internal/riveruidriver/riveruidrivertest/job_dead_letter.go @@ -3,6 +3,7 @@ package riveruidrivertest import ( "context" "fmt" + "slices" "strings" "testing" "time" @@ -16,6 +17,21 @@ import ( "riverqueue.com/riverui/internal/riveruidriver" ) +// createJobDeadLetterTable creates the dead letter table in the test schema and +// returns the prefix that qualifies table names in it. OSS test schemas don't +// include Pro tables, so reproduce only the columns these queries need, +// preserving each database's timestamp column type. +func createJobDeadLetterTable(ctx context.Context, t *testing.T, executor riveruidriver.Executor, schema string) string { + t.Helper() + + prefix := "" + if schema != "" { + prefix = dbutil.SafeIdentifier(schema) + "." + } + require.NoError(t, executor.Exec(ctx, "CREATE TABLE "+prefix+"river_job_dead_letter AS SELECT id, attempt, finalized_at, kind, max_attempts, queue, priority, tags FROM "+prefix+"river_job WHERE 1 = 0")) + return prefix +} + func exerciseJobDeadLetterList( ctx context.Context, t *testing.T, @@ -36,13 +52,7 @@ func exerciseJobDeadLetterList( t.Run("JobDeadLetterList", func(t *testing.T) { t.Parallel() executor, schema := executorWithSchema(ctx, t, driver, executorWithTx) - prefix := "" - if schema != "" { - prefix = dbutil.SafeIdentifier(schema) + "." - } - // OSS test schemas don't include Pro tables. Reproduce only the columns - // this query needs, preserving each database's timestamp column type. - require.NoError(t, executor.Exec(ctx, "CREATE TABLE "+prefix+"river_job_dead_letter AS SELECT id, attempt, finalized_at, kind, max_attempts, queue, priority, tags FROM "+prefix+"river_job WHERE 1 = 0")) + prefix := createJobDeadLetterTable(ctx, t, executor, schema) testfactory.Job(ctx, t, executor, &testfactory.JobOpts{Schema: schema}) jobs, err := executor.JobDeadLetterList(ctx, &riveruidriver.JobDeadLetterListParams{Max: 20, Schema: schema}) require.NoError(t, err) @@ -77,6 +87,7 @@ func exerciseJobDeadLetterList( } else { require.NotNil(t, job.FinalizedAt) require.True(t, job.FinalizedAt.Equal(time.Date(2026, time.October, 21, 10, 2, 3, 456_000_000, time.UTC))) + require.Equal(t, time.UTC, job.FinalizedAt.Location(), "timestamps must be in UTC on every driver") } } } @@ -156,3 +167,80 @@ func exerciseJobDeadLetterList( } }) } + +// exerciseJobDeadLetterAutocomplete proves that every driver matches and orders +// suggestions the same way for text where database collations disagree: +// mixed-case ASCII and non-ASCII letters. +func exerciseJobDeadLetterAutocomplete( + ctx context.Context, + t *testing.T, + databaseName string, + driver riveruidriver.Driver, + executorWithTx func(ctx context.Context, t *testing.T) (riverdriver.Executor, string), +) { + t.Helper() + const ( + kindFacet = "job_kind" + queueFacet = "queue_name" + ) + t.Run("JobDeadLetterAutocomplete", func(t *testing.T) { + t.Parallel() + executor, schema := executorWithSchema(ctx, t, driver, executorWithTx) + prefix := createJobDeadLetterTable(ctx, t, executor, schema) + + if databaseName == riverdriver.DatabaseNamePostgres { + // Give the columns a linguistic collation where one is available so + // the query is shown not to depend on the collation of the database + // or its columns. Unlike "C", this collation folds non-ASCII letters + // in lower() and sorts letters of both cases together. + var hasICU bool + require.NoError(t, executor.QueryRow(ctx, "SELECT EXISTS (SELECT 1 FROM pg_collation WHERE collname = 'und-x-icu')").Scan(&hasICU)) + if hasICU { + require.NoError(t, executor.Exec(ctx, "ALTER TABLE "+prefix+`river_job_dead_letter ALTER COLUMN kind TYPE text COLLATE "und-x-icu", ALTER COLUMN queue TYPE text COLLATE "und-x-icu"`)) + } + } + + // Listed in the byte order that every driver must return. A linguistic + // collation would instead return alpha, école, École, zebra, Zeta. + const ( + zeta = "Zeta" + alpha = "alpha" + zebra = "zebra" + ecoleUpper = "École" + ecoleLower = "école" + ) + kinds := []string{zeta, alpha, zebra, ecoleUpper, ecoleLower} + rows := make([]string, 0, len(kinds)) + for i, kind := range slices.Backward(kinds) { + rows = append(rows, fmt.Sprintf("(%d, 3, NULL, '%s', 5, '%s')", i+1, kind, strings.ToUpper(kind))) + } + require.NoError(t, executor.Exec(ctx, "INSERT INTO "+prefix+"river_job_dead_letter (id, attempt, finalized_at, kind, max_attempts, queue) VALUES "+strings.Join(rows, ","))) + + for _, tt := range []struct { + name string + facet string + match string + exclude []string + max int + want []string + }{ + {name: "ByteOrder", facet: kindFacet, max: 100, want: kinds}, + {name: "ByteOrderQueues", facet: queueFacet, max: 100, want: []string{"ALPHA", "ZEBRA", "ZETA", "ÉCOLE"}}, + {name: "LimitFollowsByteOrder", facet: kindFacet, max: 2, want: []string{zeta, alpha}}, + {name: "MixedCaseMatch", facet: kindFacet, match: "zE", max: 100, want: []string{zeta, zebra}}, + {name: "MixedCaseMatchUpper", facet: kindFacet, match: "ALPHA", max: 100, want: []string{alpha}}, + {name: "MixedCaseMatchQueues", facet: queueFacet, match: "zeb", max: 100, want: []string{"ZEBRA"}}, + {name: "NonASCIILower", facet: kindFacet, match: "é", max: 100, want: []string{ecoleLower}}, + {name: "NonASCIIUpper", facet: kindFacet, match: "É", max: 100, want: []string{ecoleUpper}}, + {name: "NonASCIIFoldsOnlyASCII", facet: kindFacet, match: "éCOLE", max: 100, want: []string{ecoleLower}}, + {name: "NonASCIIFoldsOnlyASCIIQueues", facet: queueFacet, match: "éc", max: 100, want: []string{}}, + {name: "NonASCIIExclude", facet: kindFacet, exclude: []string{ecoleUpper, "ZETA"}, max: 100, want: []string{zeta, alpha, zebra, ecoleLower}}, + } { + t.Run(tt.name, func(t *testing.T) { + values, err := executor.JobDeadLetterAutocomplete(ctx, &riveruidriver.JobDeadLetterAutocompleteParams{Facet: tt.facet, Match: tt.match, Exclude: tt.exclude, Max: tt.max, Schema: schema}) + require.NoError(t, err) + require.Equal(t, tt.want, values) + }) + } + }) +} diff --git a/internal/riveruidriver/riveruidrivertest/riveruidrivertest.go b/internal/riveruidriver/riveruidrivertest/riveruidrivertest.go index 1fbb6632..e58b7386 100644 --- a/internal/riveruidriver/riveruidrivertest/riveruidrivertest.go +++ b/internal/riveruidriver/riveruidrivertest/riveruidrivertest.go @@ -29,6 +29,7 @@ func Exercise( exerciseJobCountByAllStatesCapped(ctx, t, driver, executorWithTx) exerciseJobCountEstimate(ctx, t, databaseName, driver, executorWithTx) exerciseJobDeadLetterList(ctx, t, driver, executorWithTx) + exerciseJobDeadLetterAutocomplete(ctx, t, databaseName, driver, executorWithTx) } func exerciseGetExecutor( diff --git a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql index de1136dd..5c43ab08 100644 --- a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql +++ b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql @@ -65,12 +65,20 @@ FROM ( -- name: JobDeadLetterAutocomplete :one -- Suggest values from the archive, including kinds and queues with no live jobs. -SELECT coalesce(json_agg(suggestions.value ORDER BY suggestions.value), '[]'::json)::text AS values +-- +-- The "C" collation makes matching and ordering independent of the database's +-- collation and identical to SQLite: lower() folds only ASCII letters, and +-- values sort by their bytes. The ordering matters before the limit as well, +-- because it decides which suggestions are returned. +SELECT coalesce(json_agg(suggestions.value ORDER BY suggestions.value COLLATE "C"), '[]'::json)::text AS values FROM ( - SELECT DISTINCT CASE WHEN @facet::text = 'job_kind' THEN kind ELSE queue END AS value - FROM /* TEMPLATE: schema */river_job_dead_letter - WHERE left(lower(CASE WHEN @facet::text = 'job_kind' THEN kind ELSE queue END), length(@match::text)) = lower(@match::text) - AND (CASE WHEN @facet::text = 'job_kind' THEN kind ELSE queue END) NOT IN (SELECT jsonb_array_elements_text(@exclude::jsonb)) - ORDER BY value + SELECT DISTINCT candidates.value + FROM ( + SELECT (CASE WHEN @facet::text = 'job_kind' THEN kind ELSE queue END) COLLATE "C" AS value + FROM /* TEMPLATE: schema */river_job_dead_letter + ) AS candidates + WHERE left(lower(candidates.value COLLATE "C"), length(@match::text)) = lower((@match::text) COLLATE "C") + AND candidates.value NOT IN (SELECT excluded.value COLLATE "C" FROM jsonb_array_elements_text(@exclude::jsonb) AS excluded(value)) + ORDER BY candidates.value LIMIT @max::int ) AS suggestions; diff --git a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql.go b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql.go index 891d2fa2..7451eb97 100644 --- a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql.go +++ b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_job_dead_letter.sql.go @@ -10,13 +10,16 @@ import ( ) const jobDeadLetterAutocomplete = `-- name: JobDeadLetterAutocomplete :one -SELECT coalesce(json_agg(suggestions.value ORDER BY suggestions.value), '[]'::json)::text AS values +SELECT coalesce(json_agg(suggestions.value ORDER BY suggestions.value COLLATE "C"), '[]'::json)::text AS values FROM ( - SELECT DISTINCT CASE WHEN $1::text = 'job_kind' THEN kind ELSE queue END AS value - FROM /* TEMPLATE: schema */river_job_dead_letter - WHERE left(lower(CASE WHEN $1::text = 'job_kind' THEN kind ELSE queue END), length($2::text)) = lower($2::text) - AND (CASE WHEN $1::text = 'job_kind' THEN kind ELSE queue END) NOT IN (SELECT jsonb_array_elements_text($3::jsonb)) - ORDER BY value + SELECT DISTINCT candidates.value + FROM ( + SELECT (CASE WHEN $1::text = 'job_kind' THEN kind ELSE queue END) COLLATE "C" AS value + FROM /* TEMPLATE: schema */river_job_dead_letter + ) AS candidates + WHERE left(lower(candidates.value COLLATE "C"), length($2::text)) = lower(($2::text) COLLATE "C") + AND candidates.value NOT IN (SELECT excluded.value COLLATE "C" FROM jsonb_array_elements_text($3::jsonb) AS excluded(value)) + ORDER BY candidates.value LIMIT $4::int ) AS suggestions ` @@ -29,6 +32,11 @@ type JobDeadLetterAutocompleteParams struct { } // Suggest values from the archive, including kinds and queues with no live jobs. +// +// The "C" collation makes matching and ordering independent of the database's +// collation and identical to SQLite: lower() folds only ASCII letters, and +// values sort by their bytes. The ordering matters before the limit as well, +// because it decides which suggestions are returned. func (q *Queries) JobDeadLetterAutocomplete(ctx context.Context, db DBTX, arg *JobDeadLetterAutocompleteParams) (string, error) { row := db.QueryRow(ctx, jobDeadLetterAutocomplete, arg.Facet, diff --git a/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go b/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go index 8f5a8bb6..3fadebf7 100644 --- a/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go +++ b/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "math" + "slices" "time" "github.com/riverqueue/river/riverdriver" @@ -162,7 +163,7 @@ func (e *Executor) JobDeadLetterList(ctx context.Context, params *riveruidriver. if err := json.Unmarshal([]byte(rawJobs), &jobs); err != nil { return nil, fmt.Errorf("error decoding dead letter jobs: %w", err) } - riveruidriver.SortJobDeadLetterListResults(jobs) + riveruidriver.NormalizeJobDeadLetterListResults(jobs) return jobs, nil } @@ -185,5 +186,8 @@ func (e *Executor) JobDeadLetterAutocomplete(ctx context.Context, params *riveru if err := json.Unmarshal([]byte(rawValues), &values); err != nil { return nil, fmt.Errorf("error decoding dead letter suggestions: %w", err) } + // Byte order matches the queries' ordering; sorting here keeps the result + // deterministic where the database doesn't guarantee aggregate order. + slices.Sort(values) return values, nil } diff --git a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql index a403bba4..e837a095 100644 --- a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql +++ b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql @@ -16,7 +16,8 @@ CREATE TABLE river_job_dead_letter ( -- the database nor the browser materializes the entire queue. SQLite doesn't -- guarantee that json_group_array preserves the subquery's order, so the -- driver sorts the decoded rows. Normalize SQLite timestamps to RFC 3339 for --- the shared result decoder; strftime preserves NULL timestamps. +-- the shared result decoder; strftime preserves NULL timestamps. Its %f keeps +-- milliseconds, where PostgreSQL returns microseconds. -- -- This query ignores the filter's IDs; the driver sends ID lookups to -- JobDeadLetterListByIDs instead. @@ -65,6 +66,12 @@ FROM ( -- name: JobDeadLetterAutocomplete :one -- Suggest values from the archive, including kinds and queues with no live jobs. +-- +-- SQLite's built-in lower() folds only ASCII letters and its default BINARY +-- collation sorts values by their bytes. The PostgreSQL query pins the "C" +-- collation to behave the same way. A SQLite build that loads the ICU +-- extension replaces lower() with a Unicode-aware version and would also fold +-- non-ASCII letters. SELECT CAST(json_group_array(suggestions.value) AS text) AS values_json FROM ( SELECT DISTINCT candidates.value diff --git a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql.go b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql.go index ebf52cf0..3dadb5a5 100644 --- a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql.go +++ b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_job_dead_letter.sql.go @@ -32,6 +32,12 @@ type JobDeadLetterAutocompleteParams struct { } // Suggest values from the archive, including kinds and queues with no live jobs. +// +// SQLite's built-in lower() folds only ASCII letters and its default BINARY +// collation sorts values by their bytes. The PostgreSQL query pins the "C" +// collation to behave the same way. A SQLite build that loads the ICU +// extension replaces lower() with a Unicode-aware version and would also fold +// non-ASCII letters. func (q *Queries) JobDeadLetterAutocomplete(ctx context.Context, db DBTX, arg *JobDeadLetterAutocompleteParams) (string, error) { row := db.QueryRowContext(ctx, jobDeadLetterAutocomplete, arg.Facet, @@ -74,7 +80,8 @@ type JobDeadLetterListParams struct { // the database nor the browser materializes the entire queue. SQLite doesn't // guarantee that json_group_array preserves the subquery's order, so the // driver sorts the decoded rows. Normalize SQLite timestamps to RFC 3339 for -// the shared result decoder; strftime preserves NULL timestamps. +// the shared result decoder; strftime preserves NULL timestamps. Its %f keeps +// milliseconds, where PostgreSQL returns microseconds. // // This query ignores the filter's IDs; the driver sends ID lookups to // JobDeadLetterListByIDs instead. diff --git a/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go b/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go index 7d5bf2e1..7187a1d8 100644 --- a/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go +++ b/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "math" + "slices" "strconv" "strings" @@ -264,7 +265,7 @@ func (e *Executor) JobDeadLetterList(ctx context.Context, params *riveruidriver. if err := json.Unmarshal([]byte(rawJobs), &jobs); err != nil { return nil, fmt.Errorf("error decoding dead letter jobs: %w", err) } - riveruidriver.SortJobDeadLetterListResults(jobs) + riveruidriver.NormalizeJobDeadLetterListResults(jobs) return jobs, nil } @@ -287,5 +288,8 @@ func (e *Executor) JobDeadLetterAutocomplete(ctx context.Context, params *riveru if err := json.Unmarshal([]byte(rawValues), &values); err != nil { return nil, fmt.Errorf("error decoding dead letter suggestions: %w", err) } + // Byte order matches the queries' ordering; sorting here keeps the result + // deterministic where the database doesn't guarantee aggregate order. + slices.Sort(values) return values, nil } From 3e02f8047bafcbb1de29c0d699b111143582df8b Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:34:15 -0500 Subject: [PATCH 16/19] close gaps in dead letter test coverage Several dead letter behaviors have no test that would notice them breaking. Nothing exercises the endpoints when the dead letter table is missing. The size-limit cases use zero-valued slices such as `make([]int64, 1001)`, which fail on element validation (an ID of zero, an empty string) before the size limit is ever consulted, so the limits themselves are untested. The shared driver suite never calls autocomplete with an invalid facet or an out-of-range max, and the not-found and unique-conflict cases assert only an error message, not the status a client receives. Add a test that drops the table from the test schema and asserts that list, autocomplete, get, and retry each return the bad request error naming the migration. Replace the size-limit cases with table-driven validation tests whose slices hold valid elements, covering the passing boundary and the first failing size for IDs, kinds, queues, tags, and priorities, the per-value length limit, and the autocomplete `match` and `exclude` limits. Each failing case asserts the specific validation message, so a case can't pass for the wrong reason again. Extend the shared driver suite with the autocomplete guards for invalid facets and for max values outside 1 through 100, and assert that the not-found cases are a 404 `NotFound` error and that the unique conflict is an `APIError` with status 409. --- .../riveruidrivertest/job_dead_letter.go | 16 ++ .../prohandler/dead_letter_endpoints_test.go | 191 ++++++++++++++++-- 2 files changed, 189 insertions(+), 18 deletions(-) diff --git a/internal/riveruidriver/riveruidrivertest/job_dead_letter.go b/internal/riveruidriver/riveruidrivertest/job_dead_letter.go index 9fcdd5eb..c680e5db 100644 --- a/internal/riveruidriver/riveruidrivertest/job_dead_letter.go +++ b/internal/riveruidriver/riveruidrivertest/job_dead_letter.go @@ -242,5 +242,21 @@ func exerciseJobDeadLetterAutocomplete( require.Equal(t, tt.want, values) }) } + + // Drivers guard their own inputs rather than relying on the caller's + // validation, and reject them before running a query. + for _, facet := range []string{"", "tags", "kind", "JOB_KIND"} { + _, err := executor.JobDeadLetterAutocomplete(ctx, &riveruidriver.JobDeadLetterAutocompleteParams{Facet: facet, Max: 100, Schema: schema}) + require.EqualError(t, err, "invalid dead letter autocomplete facet", "facet %q", facet) + } + for _, limit := range []int{-1, 0, 101} { + _, err := executor.JobDeadLetterAutocomplete(ctx, &riveruidriver.JobDeadLetterAutocompleteParams{Facet: kindFacet, Max: limit, Schema: schema}) + require.EqualError(t, err, "dead letter autocomplete max must be between 1 and 100", "max %d", limit) + } + for _, limit := range []int{1, 100} { + values, err := executor.JobDeadLetterAutocomplete(ctx, &riveruidriver.JobDeadLetterAutocompleteParams{Facet: kindFacet, Max: limit, Schema: schema}) + require.NoError(t, err) + require.Len(t, values, min(limit, len(kinds))) + } }) } diff --git a/riverproui/internal/prohandler/dead_letter_endpoints_test.go b/riverproui/internal/prohandler/dead_letter_endpoints_test.go index 8cd0dfa6..fc4f954d 100644 --- a/riverproui/internal/prohandler/dead_letter_endpoints_test.go +++ b/riverproui/internal/prohandler/dead_letter_endpoints_test.go @@ -16,6 +16,7 @@ import ( "github.com/riverqueue/apiframe/apierror" "github.com/riverqueue/apiframe/apitest" "github.com/riverqueue/river/riverdriver" + "github.com/riverqueue/river/rivershared/util/dbutil" "github.com/riverqueue/river/rivertype" "riverqueue.com/riverpro/driver" @@ -94,25 +95,175 @@ func TestDeadLetterJobList(t *testing.T) { resp, err = apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobListRequest{Queues: []string{"missing"}}) require.NoError(t, err) require.Empty(t, resp.Data) - for _, invalid := range []*deadLetterJobListRequest{ - {IDs: []int64{0}}, - {IDs: []int64{-1}}, - {IDs: make([]int64, 1001)}, - {Priorities: []int16{0}}, - {Priorities: []int16{5}}, - {Kinds: []string{""}}, - {Queues: []string{strings.Repeat("x", 1025)}}, - {Tags: make([]string, 101)}, - } { - _, err := apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), invalid) - require.Error(t, err) - } for _, limit := range []int{-1, 0, 201} { _, err := apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobListRequest{Limit: &limit}) require.Error(t, err) } } +// validDeadLetterIDs and validDeadLetterValues return slices whose elements +// pass validation, so that only the size of the slice is under test. +func validDeadLetterIDs(count int) []int64 { + ids := make([]int64, count) + for i := range ids { + ids[i] = int64(i) + 1 + } + return ids +} + +func validDeadLetterValues(count int) []string { + values := make([]string, count) + for i := range values { + values[i] = fmt.Sprintf("value_%d", i) + } + return values +} + +func TestDeadLetterJobListValidation(t *testing.T) { + t.Parallel() + ctx := t.Context() + endpoint, _ := setupEndpoint(ctx, t, NewDeadLetterJobListEndpoint) + + const ( + tooManyElements = "must contain at most %d element(s)" + tooLong = "must be at most 1024 character(s) long" + required = "is required" + ) + for _, tt := range []struct { + name string + request *deadLetterJobListRequest + wantErr string // empty when the request is valid + }{ + {name: "IDsAtLimit", request: &deadLetterJobListRequest{IDs: validDeadLetterIDs(1000)}}, + {name: "IDsOverLimit", request: &deadLetterJobListRequest{IDs: validDeadLetterIDs(1001)}, wantErr: fmt.Sprintf(tooManyElements, 1000)}, + {name: "IDZero", request: &deadLetterJobListRequest{IDs: []int64{0}}, wantErr: "must be greater or equal to 1"}, + {name: "IDNegative", request: &deadLetterJobListRequest{IDs: []int64{-1}}, wantErr: "must be greater or equal to 1"}, + {name: "KindsAtLimit", request: &deadLetterJobListRequest{Kinds: validDeadLetterValues(100)}}, + {name: "KindsOverLimit", request: &deadLetterJobListRequest{Kinds: validDeadLetterValues(101)}, wantErr: fmt.Sprintf(tooManyElements, 100)}, + {name: "KindAtLength", request: &deadLetterJobListRequest{Kinds: []string{strings.Repeat("x", 1024)}}}, + {name: "KindOverLength", request: &deadLetterJobListRequest{Kinds: []string{strings.Repeat("x", 1025)}}, wantErr: tooLong}, + {name: "KindEmpty", request: &deadLetterJobListRequest{Kinds: []string{""}}, wantErr: required}, + {name: "QueuesAtLimit", request: &deadLetterJobListRequest{Queues: validDeadLetterValues(100)}}, + {name: "QueuesOverLimit", request: &deadLetterJobListRequest{Queues: validDeadLetterValues(101)}, wantErr: fmt.Sprintf(tooManyElements, 100)}, + {name: "QueueAtLength", request: &deadLetterJobListRequest{Queues: []string{strings.Repeat("x", 1024)}}}, + {name: "QueueOverLength", request: &deadLetterJobListRequest{Queues: []string{strings.Repeat("x", 1025)}}, wantErr: tooLong}, + {name: "QueueEmpty", request: &deadLetterJobListRequest{Queues: []string{""}}, wantErr: required}, + {name: "TagsAtLimit", request: &deadLetterJobListRequest{Tags: validDeadLetterValues(100)}}, + {name: "TagsOverLimit", request: &deadLetterJobListRequest{Tags: validDeadLetterValues(101)}, wantErr: fmt.Sprintf(tooManyElements, 100)}, + {name: "TagAtLength", request: &deadLetterJobListRequest{Tags: []string{strings.Repeat("x", 1024)}}}, + {name: "TagOverLength", request: &deadLetterJobListRequest{Tags: []string{strings.Repeat("x", 1025)}}, wantErr: tooLong}, + {name: "TagEmpty", request: &deadLetterJobListRequest{Tags: []string{""}}, wantErr: required}, + {name: "PrioritiesAll", request: &deadLetterJobListRequest{Priorities: []int16{1, 2, 3, 4}}}, + {name: "PrioritiesOverLimit", request: &deadLetterJobListRequest{Priorities: []int16{1, 2, 3, 4, 1}}, wantErr: fmt.Sprintf(tooManyElements, 4)}, + {name: "PriorityZero", request: &deadLetterJobListRequest{Priorities: []int16{0}}, wantErr: "min"}, + {name: "PriorityTooHigh", request: &deadLetterJobListRequest{Priorities: []int16{5}}, wantErr: "max"}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + _, err := apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), tt.request) + if tt.wantErr == "" { + require.NoError(t, err) + return + } + var badRequest *apierror.BadRequest + require.ErrorAs(t, err, &badRequest) + require.Equal(t, http.StatusBadRequest, badRequest.StatusCode) + require.Contains(t, badRequest.Message, tt.wantErr) + }) + } +} + +func TestDeadLetterAutocompleteValidation(t *testing.T) { + t.Parallel() + ctx := t.Context() + endpoint, _ := setupEndpoint(ctx, t, NewDeadLetterAutocompleteEndpoint) + + const tooLong = "must be at most 1024 character(s) long" + for _, tt := range []struct { + name string + request *deadLetterAutocompleteRequest + wantErr string // empty when the request is valid + }{ + {name: "MatchAtLength", request: &deadLetterAutocompleteRequest{Facet: "job_kind", Match: strings.Repeat("x", 1024)}}, + {name: "MatchOverLength", request: &deadLetterAutocompleteRequest{Facet: "job_kind", Match: strings.Repeat("x", 1025)}, wantErr: tooLong}, + {name: "ExcludeAtLimit", request: &deadLetterAutocompleteRequest{Facet: "job_kind", Exclude: validDeadLetterValues(100)}}, + {name: "ExcludeOverLimit", request: &deadLetterAutocompleteRequest{Facet: "job_kind", Exclude: validDeadLetterValues(101)}, wantErr: "must contain at most 100 element(s)"}, + {name: "ExcludeValueAtLength", request: &deadLetterAutocompleteRequest{Facet: "queue_name", Exclude: []string{strings.Repeat("x", 1024)}}}, + {name: "ExcludeValueOverLength", request: &deadLetterAutocompleteRequest{Facet: "queue_name", Exclude: []string{strings.Repeat("x", 1025)}}, wantErr: tooLong}, + {name: "FacetMissing", request: &deadLetterAutocompleteRequest{}, wantErr: "is required"}, + {name: "FacetUnknown", request: &deadLetterAutocompleteRequest{Facet: "tags"}, wantErr: "should be one of the following values: job_kind queue_name"}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + _, err := apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), tt.request) + if tt.wantErr == "" { + require.NoError(t, err) + return + } + var badRequest *apierror.BadRequest + require.ErrorAs(t, err, &badRequest) + require.Equal(t, http.StatusBadRequest, badRequest.StatusCode) + require.Contains(t, badRequest.Message, tt.wantErr) + }) + } +} + +// Every dead letter endpoint must report a missing table as a bad request that +// names the migration, rather than failing in a query. +func TestDeadLetterJobsRequireTable(t *testing.T) { + t.Parallel() + ctx := t.Context() + + dropTable := func(t *testing.T, bundle *setupEndpointTestBundle) { + t.Helper() + require.NoError(t, bundle.exec.Exec(ctx, "DROP TABLE "+dbutil.SafeIdentifier(bundle.schema)+".river_job_dead_letter")) + } + requireMissingTableError := func(t *testing.T, err error) { + t.Helper() + var badRequest *apierror.BadRequest + require.ErrorAs(t, err, &badRequest) + require.Equal(t, http.StatusBadRequest, badRequest.StatusCode) + require.Equal(t, "Dead letter jobs require River Pro's dead letter queue migration.", badRequest.Message) + } + + t.Run("List", func(t *testing.T) { + t.Parallel() + endpoint, bundle := setupEndpoint(ctx, t, NewDeadLetterJobListEndpoint) + dropTable(t, bundle) + _, err := apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobListRequest{}) + requireMissingTableError(t, err) + }) + t.Run("Autocomplete", func(t *testing.T) { + t.Parallel() + endpoint, bundle := setupEndpoint(ctx, t, NewDeadLetterAutocompleteEndpoint) + dropTable(t, bundle) + _, err := apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterAutocompleteRequest{Facet: "job_kind"}) + requireMissingTableError(t, err) + }) + t.Run("Get", func(t *testing.T) { + t.Parallel() + endpoint, bundle := setupEndpoint(ctx, t, NewDeadLetterJobGetEndpoint) + dropTable(t, bundle) + _, err := apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobRequest{JobID: 1}) + requireMissingTableError(t, err) + }) + t.Run("Retry", func(t *testing.T) { + t.Parallel() + endpoint, bundle := setupEndpoint(ctx, t, NewDeadLetterJobRetryEndpoint) + dropTable(t, bundle) + _, err := apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobRequest{JobID: 1}) + requireMissingTableError(t, err) + }) +} + +func requireDeadLetterNotFound(t *testing.T, err error, jobID int64) { + t.Helper() + var notFound *apierror.NotFound + require.ErrorAs(t, err, ¬Found) + require.Equal(t, http.StatusNotFound, notFound.StatusCode) + require.Equal(t, fmt.Sprintf("Dead letter job with ID %d not found. It may already have been retried.", jobID), notFound.Message) +} + func TestDeadLetterJobGet(t *testing.T) { t.Parallel() ctx := t.Context() @@ -126,7 +277,7 @@ func TestDeadLetterJobGet(t *testing.T) { require.Equal(t, "delivery failed", resp.Errors[0].Error) _, err = apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobRequest{JobID: job.ID + 1}) - require.EqualError(t, err, fmt.Sprintf("Dead letter job with ID %d not found. It may already have been retried.", job.ID+1)) + requireDeadLetterNotFound(t, err, job.ID+1) } func TestDeadLetterJobRetry(t *testing.T) { @@ -151,9 +302,9 @@ func TestDeadLetterJobRetry(t *testing.T) { require.Equal(t, rivertype.JobStateAvailable, live.State) _, err = apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobRequest{JobID: job.ID}) - require.EqualError(t, err, fmt.Sprintf("Dead letter job with ID %d not found. It may already have been retried.", job.ID)) + requireDeadLetterNotFound(t, err, job.ID) _, err = apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobRequest{JobID: live.ID}) - require.EqualError(t, err, fmt.Sprintf("Dead letter job with ID %d not found. It may already have been retried.", live.ID)) + requireDeadLetterNotFound(t, err, live.ID) } func TestDeadLetterJobRetryUniqueConflict(t *testing.T) { @@ -173,7 +324,10 @@ func TestDeadLetterJobRetryUniqueConflict(t *testing.T) { require.NoError(t, err) _, err = apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobRequest{JobID: job.ID}) - require.ErrorContains(t, err, "another active job has the same unique properties") + var apiErr *apierror.APIError + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusConflict, apiErr.StatusCode) + require.Equal(t, "Job can't be retried because another active job has the same unique properties. Wait for it to finish before retrying.", apiErr.Message) _, err = bundle.exec.JobDeadLetterGetByID(ctx, &driver.JobDeadLetterGetByIDParams{ID: job.ID, Schema: bundle.schema}) require.NoError(t, err, "a conflict must retain the dead letter job") } @@ -307,6 +461,7 @@ func TestDeadLetterAutocomplete(t *testing.T) { } for _, facet := range []string{"", "tags", "id"} { _, err := apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterAutocompleteRequest{Facet: facet}) - require.Error(t, err) + var badRequest *apierror.BadRequest + require.ErrorAs(t, err, &badRequest) } } From 6334f8751aaaee69a01e6b598a17a4d3e20acd8a Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:35:30 -0500 Subject: [PATCH 17/19] check for dead letter table in one place The name of the dead letter table is spelled out twice: once where the extensions report whether dead letter jobs are available, and once in the guard that each dead letter endpoint runs. The feature flag and the guard have to agree, or the UI offers a page whose requests all fail. Add `HasDeadLetterTable` alongside the endpoints, mirroring `HasWorkflowV2Tables`, and use it for both the extension flag and the guard so there is a single definition of what "available" means. --- riverproui/endpoints.go | 5 +---- .../prohandler/dead_letter_endpoints.go | 18 ++++++++++++++---- .../prohandler/dead_letter_endpoints_test.go | 11 +++++++++-- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/riverproui/endpoints.go b/riverproui/endpoints.go index 220714f9..00cf7f4a 100644 --- a/riverproui/endpoints.go +++ b/riverproui/endpoints.go @@ -90,10 +90,7 @@ func (e *endpoints[TTx]) Extensions(ctx context.Context) (map[string]bool, error } defer execTx.Rollback(ctx) - hasDeadLetterTable, err := execTx.TableExists(ctx, &riverdriver.TableExistsParams{ - Schema: schema, - Table: "river_job_dead_letter", - }) + hasDeadLetterTable, err := prohandler.HasDeadLetterTable(ctx, execTx, schema) if err != nil { return nil, err } diff --git a/riverproui/internal/prohandler/dead_letter_endpoints.go b/riverproui/internal/prohandler/dead_letter_endpoints.go index 1757e13a..a33c92ab 100644 --- a/riverproui/internal/prohandler/dead_letter_endpoints.go +++ b/riverproui/internal/prohandler/dead_letter_endpoints.go @@ -24,11 +24,21 @@ import ( "riverqueue.com/riverui/internal/riveruidriver/riveruisqlite" ) -func (a *ProAPIBundle[TTx]) requireDeadLetterJobs(ctx context.Context) error { - exists, err := a.DB.TableExists(ctx, &riverdriver.TableExistsParams{ - Schema: a.Client.Schema(), - Table: "river_job_dead_letter", +// deadLetterTableName is the table created by River Pro's dead letter queue +// migration. +const deadLetterTableName = "river_job_dead_letter" + +// HasDeadLetterTable reports whether the schema has the dead letter table, and +// therefore whether the dead letter endpoints can serve requests. +func HasDeadLetterTable(ctx context.Context, exec riverdriver.Executor, schema string) (bool, error) { + return exec.TableExists(ctx, &riverdriver.TableExistsParams{ + Schema: schema, + Table: deadLetterTableName, }) +} + +func (a *ProAPIBundle[TTx]) requireDeadLetterJobs(ctx context.Context) error { + exists, err := HasDeadLetterTable(ctx, a.DB, a.Client.Schema()) if err != nil { return fmt.Errorf("error checking dead letter schema support: %w", err) } diff --git a/riverproui/internal/prohandler/dead_letter_endpoints_test.go b/riverproui/internal/prohandler/dead_letter_endpoints_test.go index fc4f954d..f448aa75 100644 --- a/riverproui/internal/prohandler/dead_letter_endpoints_test.go +++ b/riverproui/internal/prohandler/dead_letter_endpoints_test.go @@ -216,7 +216,11 @@ func TestDeadLetterJobsRequireTable(t *testing.T) { dropTable := func(t *testing.T, bundle *setupEndpointTestBundle) { t.Helper() - require.NoError(t, bundle.exec.Exec(ctx, "DROP TABLE "+dbutil.SafeIdentifier(bundle.schema)+".river_job_dead_letter")) + require.NoError(t, bundle.exec.Exec(ctx, "DROP TABLE "+dbutil.SafeIdentifier(bundle.schema)+"."+deadLetterTableName)) + + exists, err := HasDeadLetterTable(ctx, bundle.exec, bundle.schema) + require.NoError(t, err) + require.False(t, exists) } requireMissingTableError := func(t *testing.T, err error) { t.Helper() @@ -229,8 +233,11 @@ func TestDeadLetterJobsRequireTable(t *testing.T) { t.Run("List", func(t *testing.T) { t.Parallel() endpoint, bundle := setupEndpoint(ctx, t, NewDeadLetterJobListEndpoint) + exists, err := HasDeadLetterTable(ctx, bundle.exec, bundle.schema) + require.NoError(t, err) + require.True(t, exists) dropTable(t, bundle) - _, err := apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobListRequest{}) + _, err = apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobListRequest{}) requireMissingTableError(t, err) }) t.Run("Autocomplete", func(t *testing.T) { From f393f951b5430b8f1b1479a3f5db3302f2a82baa Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:36:51 -0500 Subject: [PATCH 18/19] name dead letter limits and facets The dead letter list limit of 200, its default of 20, the autocomplete limit of 100, and the facet names `job_kind` and `queue_name` appear as bare literals in the handler and again in each driver's guards. The copies have to change together, and nothing ties them to one another. Define the limits as constants and the facet as a `JobDeadLetterFacet` type in the driver interface package, and move the two drivers' identical guards into `Validate` methods on the params so both reject the same inputs with the same errors. The handler takes its default and maximum from the constants and carries the typed facet through to the driver. Two places can't reference the constants and are checked instead. Validation struct tags must be literals, so a comment points at the constants and a test compares the tags against them. The queries embed the job kind facet as a literal, so a test in each driver asserts that the generated query still uses the constant's value. --- .../river_ui_driver_interface.go | 48 ++++++++++++++++++- .../riveruidrivertest/job_dead_letter.go | 22 ++++----- .../internal/dbsqlc/river_queries_test.go | 11 +++++ .../river_ui_postgres_driver.go | 13 ++--- .../internal/dbsqlc/river_queries_test.go | 11 +++++ .../riveruisqlite/river_ui_sqlite_driver.go | 13 ++--- .../prohandler/dead_letter_endpoints.go | 16 ++++--- .../prohandler/dead_letter_endpoints_test.go | 26 ++++++++-- 8 files changed, 123 insertions(+), 37 deletions(-) diff --git a/internal/riveruidriver/river_ui_driver_interface.go b/internal/riveruidriver/river_ui_driver_interface.go index 762d35d1..0f694c62 100644 --- a/internal/riveruidriver/river_ui_driver_interface.go +++ b/internal/riveruidriver/river_ui_driver_interface.go @@ -7,6 +7,8 @@ package riveruidriver import ( "cmp" "context" + "errors" + "fmt" "slices" "time" @@ -44,6 +46,27 @@ type Executor interface { JobDeadLetterAutocomplete(ctx context.Context, params *JobDeadLetterAutocompleteParams) ([]string, error) } +const ( + // JobDeadLetterListMaxDefault is the number of jobs to list when the caller + // has no preference. + JobDeadLetterListMaxDefault = 20 + // JobDeadLetterListMaxLimit is the largest Max that JobDeadLetterList + // accepts. + JobDeadLetterListMaxLimit = 200 + // JobDeadLetterAutocompleteMaxLimit is the largest Max that + // JobDeadLetterAutocomplete accepts. + JobDeadLetterAutocompleteMaxLimit = 100 +) + +// JobDeadLetterFacet is a job attribute that JobDeadLetterAutocomplete can +// suggest values for. The queries compare against these values literally. +type JobDeadLetterFacet string + +const ( + JobDeadLetterFacetJobKind JobDeadLetterFacet = "job_kind" + JobDeadLetterFacetQueueName JobDeadLetterFacet = "queue_name" +) + // JobDeadLetterFilters matches any value within each populated filter and // requires all populated filters to match. Tags match any exact tag. type JobDeadLetterFilters struct { @@ -56,18 +79,41 @@ type JobDeadLetterFilters struct { type JobDeadLetterAutocompleteParams struct { Exclude []string - Facet string + Facet JobDeadLetterFacet Match string Max int Schema string } +// Validate reports whether the params are ones that every driver accepts. +// Drivers call it before querying so they all reject the same inputs. +func (p *JobDeadLetterAutocompleteParams) Validate() error { + switch p.Facet { + case JobDeadLetterFacetJobKind, JobDeadLetterFacetQueueName: + default: + return errors.New("invalid dead letter autocomplete facet") + } + if p.Max < 1 || p.Max > JobDeadLetterAutocompleteMaxLimit { + return fmt.Errorf("dead letter autocomplete max must be between 1 and %d", JobDeadLetterAutocompleteMaxLimit) + } + return nil +} + type JobDeadLetterListParams struct { Filters JobDeadLetterFilters Max int Schema string } +// Validate reports whether the params are ones that every driver accepts. +// Drivers call it before querying so they all reject the same inputs. +func (p *JobDeadLetterListParams) Validate() error { + if p.Max < 1 || p.Max > JobDeadLetterListMaxLimit { + return fmt.Errorf("dead letter list max must be between 1 and %d", JobDeadLetterListMaxLimit) + } + return nil +} + // JobDeadLetterListResult contains only fields displayed in the list. Args, // metadata, and attempt history are loaded separately when opening a job. // diff --git a/internal/riveruidriver/riveruidrivertest/job_dead_letter.go b/internal/riveruidriver/riveruidrivertest/job_dead_letter.go index c680e5db..2051906b 100644 --- a/internal/riveruidriver/riveruidrivertest/job_dead_letter.go +++ b/internal/riveruidriver/riveruidrivertest/job_dead_letter.go @@ -47,7 +47,7 @@ func exerciseJobDeadLetterList( originalKind = "send_email" customerTag = "customer" emailKind = "archive_email" - kindFacet = "job_kind" + kindFacet = riveruidriver.JobDeadLetterFacetJobKind ) t.Run("JobDeadLetterList", func(t *testing.T) { t.Parallel() @@ -72,7 +72,7 @@ func exerciseJobDeadLetterList( } require.NoError(t, executor.Exec(ctx, "INSERT INTO "+prefix+"river_job_dead_letter (id, attempt, finalized_at, kind, max_attempts, queue) VALUES "+strings.Join(rows, ","))) - for _, limit := range []int{1, 20, 40, 200} { + for _, limit := range []int{1, riveruidriver.JobDeadLetterListMaxDefault, 40, riveruidriver.JobDeadLetterListMaxLimit} { jobs, err := executor.JobDeadLetterList(ctx, &riveruidriver.JobDeadLetterListParams{Max: limit, Schema: schema}) require.NoError(t, err) require.Len(t, jobs, limit) @@ -91,7 +91,7 @@ func exerciseJobDeadLetterList( } } } - for _, limit := range []int{-1, 0, 201} { + for _, limit := range []int{-1, 0, riveruidriver.JobDeadLetterListMaxLimit + 1} { _, err := executor.JobDeadLetterList(ctx, &riveruidriver.JobDeadLetterListParams{Max: limit, Schema: schema}) require.EqualError(t, err, "dead letter list max must be between 1 and 200") } @@ -145,7 +145,7 @@ func exerciseJobDeadLetterList( for _, tt := range []struct { name string - facet string + facet riveruidriver.JobDeadLetterFacet match string exclude []string max int @@ -155,7 +155,7 @@ func exerciseJobDeadLetterList( {name: "PrefixCaseInsensitive", facet: kindFacet, match: "ARCHIVE_", max: 100, want: []string{emailKind, reportKind}}, {name: "Exclude", facet: kindFacet, match: "archive_", exclude: []string{reportKind}, max: 100, want: []string{emailKind}}, {name: "Limit", facet: kindFacet, max: 1, want: []string{emailKind}}, - {name: "Queues", facet: "queue_name", max: 100, want: []string{mailQueue, reportsQueue}}, + {name: "Queues", facet: riveruidriver.JobDeadLetterFacetQueueName, max: 100, want: []string{mailQueue, reportsQueue}}, {name: "NoLiveJobs", facet: kindFacet, match: "fake_job", max: 100, want: []string{}}, {name: "LiteralPrefix", facet: kindFacet, match: "%", max: 100, want: []string{}}, } { @@ -180,8 +180,8 @@ func exerciseJobDeadLetterAutocomplete( ) { t.Helper() const ( - kindFacet = "job_kind" - queueFacet = "queue_name" + kindFacet = riveruidriver.JobDeadLetterFacetJobKind + queueFacet = riveruidriver.JobDeadLetterFacetQueueName ) t.Run("JobDeadLetterAutocomplete", func(t *testing.T) { t.Parallel() @@ -218,7 +218,7 @@ func exerciseJobDeadLetterAutocomplete( for _, tt := range []struct { name string - facet string + facet riveruidriver.JobDeadLetterFacet match string exclude []string max int @@ -245,15 +245,15 @@ func exerciseJobDeadLetterAutocomplete( // Drivers guard their own inputs rather than relying on the caller's // validation, and reject them before running a query. - for _, facet := range []string{"", "tags", "kind", "JOB_KIND"} { + for _, facet := range []riveruidriver.JobDeadLetterFacet{"", "tags", "kind", "JOB_KIND"} { _, err := executor.JobDeadLetterAutocomplete(ctx, &riveruidriver.JobDeadLetterAutocompleteParams{Facet: facet, Max: 100, Schema: schema}) require.EqualError(t, err, "invalid dead letter autocomplete facet", "facet %q", facet) } - for _, limit := range []int{-1, 0, 101} { + for _, limit := range []int{-1, 0, riveruidriver.JobDeadLetterAutocompleteMaxLimit + 1} { _, err := executor.JobDeadLetterAutocomplete(ctx, &riveruidriver.JobDeadLetterAutocompleteParams{Facet: kindFacet, Max: limit, Schema: schema}) require.EqualError(t, err, "dead letter autocomplete max must be between 1 and 100", "max %d", limit) } - for _, limit := range []int{1, 100} { + for _, limit := range []int{1, riveruidriver.JobDeadLetterAutocompleteMaxLimit} { values, err := executor.JobDeadLetterAutocomplete(ctx, &riveruidriver.JobDeadLetterAutocompleteParams{Facet: kindFacet, Max: limit, Schema: schema}) require.NoError(t, err) require.Len(t, values, min(limit, len(kinds))) diff --git a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_queries_test.go b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_queries_test.go index b7e94119..fdb8c45b 100644 --- a/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_queries_test.go +++ b/internal/riveruidriver/riveruipostgres/internal/dbsqlc/river_queries_test.go @@ -6,6 +6,8 @@ import ( "github.com/stretchr/testify/require" "github.com/riverqueue/river/rivertype" + + "riverqueue.com/riverui/internal/riveruidriver" ) func TestJobCountEstimateQuery(t *testing.T) { @@ -29,3 +31,12 @@ func TestJobCountByAllStatesCappedQueryStates(t *testing.T) { require.Contains(t, jobCountByAllStatesCapped, "state = '"+string(state)+"'") } } + +func TestJobDeadLetterAutocompleteQueryFacets(t *testing.T) { + t.Parallel() + + // The query selects kinds for the job kind facet and queues for anything + // else, so its literal must stay in step with the driver's facet values. + require.Contains(t, jobDeadLetterAutocomplete, "= '"+string(riveruidriver.JobDeadLetterFacetJobKind)+"' THEN kind ELSE queue END") + require.NotContains(t, jobDeadLetterAutocomplete, string(riveruidriver.JobDeadLetterFacetQueueName)) +} diff --git a/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go b/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go index 3fadebf7..04157ecf 100644 --- a/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go +++ b/internal/riveruidriver/riveruipostgres/river_ui_postgres_driver.go @@ -142,8 +142,8 @@ func (w riverExecutorWrapper) QueryRow(ctx context.Context, query string, args . } func (e *Executor) JobDeadLetterList(ctx context.Context, params *riveruidriver.JobDeadLetterListParams) ([]*riveruidriver.JobDeadLetterListResult, error) { - if params.Max < 1 || params.Max > 200 { - return nil, errors.New("dead letter list max must be between 1 and 200") + if err := params.Validate(); err != nil { + return nil, err } filters, err := json.Marshal(params.Filters) if err != nil { @@ -168,17 +168,14 @@ func (e *Executor) JobDeadLetterList(ctx context.Context, params *riveruidriver. } func (e *Executor) JobDeadLetterAutocomplete(ctx context.Context, params *riveruidriver.JobDeadLetterAutocompleteParams) ([]string, error) { - if params.Facet != "job_kind" && params.Facet != "queue_name" { - return nil, errors.New("invalid dead letter autocomplete facet") - } - if params.Max < 1 || params.Max > 100 { - return nil, errors.New("dead letter autocomplete max must be between 1 and 100") + if err := params.Validate(); err != nil { + return nil, err } exclude, err := json.Marshal(append([]string{}, params.Exclude...)) if err != nil { return nil, fmt.Errorf("error encoding dead letter exclusions: %w", err) } - rawValues, err := dbsqlc.JobDeadLetterAutocompleteRiver(schemaTemplateParam(ctx, params.Schema), e.dbtx, params.Facet, params.Match, string(exclude), params.Max) + rawValues, err := dbsqlc.JobDeadLetterAutocompleteRiver(schemaTemplateParam(ctx, params.Schema), e.dbtx, string(params.Facet), params.Match, string(exclude), params.Max) if err != nil { return nil, fmt.Errorf("error autocompleting dead letter jobs: %w", err) } diff --git a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_queries_test.go b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_queries_test.go index 9baae87d..e7a92900 100644 --- a/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_queries_test.go +++ b/internal/riveruidriver/riveruisqlite/internal/dbsqlc/river_queries_test.go @@ -6,6 +6,8 @@ import ( "github.com/stretchr/testify/require" "github.com/riverqueue/river/rivertype" + + "riverqueue.com/riverui/internal/riveruidriver" ) func TestJobCountByAllStatesCappedQueryStates(t *testing.T) { @@ -23,3 +25,12 @@ func TestJobCountEstimateStat4Query(t *testing.T) { require.Contains(t, jobCountEstimateStat4, "river_job_prioritized_fetching_index") require.Contains(t, jobCountEstimateStat4, "group_concat") } + +func TestJobDeadLetterAutocompleteQueryFacets(t *testing.T) { + t.Parallel() + + // The query selects kinds for the job kind facet and queues for anything + // else, so its literal must stay in step with the driver's facet values. + require.Contains(t, jobDeadLetterAutocomplete, "= '"+string(riveruidriver.JobDeadLetterFacetJobKind)+"' THEN kind ELSE queue END") + require.NotContains(t, jobDeadLetterAutocomplete, string(riveruidriver.JobDeadLetterFacetQueueName)) +} diff --git a/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go b/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go index 7187a1d8..8e4a6096 100644 --- a/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go +++ b/internal/riveruidriver/riveruisqlite/river_ui_sqlite_driver.go @@ -244,8 +244,8 @@ func (w riverExecutorWrapper) QueryRow(ctx context.Context, query string, args . } func (e *Executor) JobDeadLetterList(ctx context.Context, params *riveruidriver.JobDeadLetterListParams) ([]*riveruidriver.JobDeadLetterListResult, error) { - if params.Max < 1 || params.Max > 200 { - return nil, errors.New("dead letter list max must be between 1 and 200") + if err := params.Validate(); err != nil { + return nil, err } filters, err := json.Marshal(params.Filters) if err != nil { @@ -270,17 +270,14 @@ func (e *Executor) JobDeadLetterList(ctx context.Context, params *riveruidriver. } func (e *Executor) JobDeadLetterAutocomplete(ctx context.Context, params *riveruidriver.JobDeadLetterAutocompleteParams) ([]string, error) { - if params.Facet != "job_kind" && params.Facet != "queue_name" { - return nil, errors.New("invalid dead letter autocomplete facet") - } - if params.Max < 1 || params.Max > 100 { - return nil, errors.New("dead letter autocomplete max must be between 1 and 100") + if err := params.Validate(); err != nil { + return nil, err } exclude, err := json.Marshal(append([]string{}, params.Exclude...)) if err != nil { return nil, fmt.Errorf("error encoding dead letter exclusions: %w", err) } - rawValues, err := dbsqlc.JobDeadLetterAutocompleteRiver(schemaTemplateParam(ctx, params.Schema), e.dbtx, params.Facet, params.Match, string(exclude), params.Max) + rawValues, err := dbsqlc.JobDeadLetterAutocompleteRiver(schemaTemplateParam(ctx, params.Schema), e.dbtx, string(params.Facet), params.Match, string(exclude), params.Max) if err != nil { return nil, fmt.Errorf("error autocompleting dead letter jobs: %w", err) } diff --git a/riverproui/internal/prohandler/dead_letter_endpoints.go b/riverproui/internal/prohandler/dead_letter_endpoints.go index a33c92ab..f0f4a21b 100644 --- a/riverproui/internal/prohandler/dead_letter_endpoints.go +++ b/riverproui/internal/prohandler/dead_letter_endpoints.go @@ -97,6 +97,8 @@ func (*deadLetterJobListEndpoint[TTx]) Meta() *apiendpoint.EndpointMeta { return &apiendpoint.EndpointMeta{Pattern: "GET /api/pro/dead-letter-jobs", StatusCode: http.StatusOK} } +// The validate tags must be literals. Limit's maximum mirrors +// riveruidriver.JobDeadLetterListMaxLimit. type deadLetterJobListRequest struct { IDs []int64 `json:"-" validate:"omitempty,max=1000,dive,min=1"` Kinds []string `json:"-" validate:"omitempty,max=100,dive,required,max=1024"` @@ -161,7 +163,7 @@ func (a *deadLetterJobListEndpoint[TTx]) Execute(ctx context.Context, req *deadL } jobs, err := a.uiDriver.GetExecutor(a.DB).JobDeadLetterList(ctx, &riveruidriver.JobDeadLetterListParams{ Filters: riveruidriver.JobDeadLetterFilters{IDs: req.IDs, Kinds: req.Kinds, Priorities: req.Priorities, Queues: req.Queues, Tags: req.Tags}, - Max: ptrutil.ValOrDefault(req.Limit, 20), + Max: ptrutil.ValOrDefault(req.Limit, riveruidriver.JobDeadLetterListMaxDefault), Schema: a.Client.Schema(), }) if err != nil { @@ -263,15 +265,17 @@ func (*deadLetterAutocompleteEndpoint[TTx]) Meta() *apiendpoint.EndpointMeta { return &apiendpoint.EndpointMeta{Pattern: "GET /api/pro/dead-letter-jobs/autocomplete", StatusCode: http.StatusOK} } +// The validate tags must be literals. Facet's values mirror the +// riveruidriver.JobDeadLetterFacet constants. type deadLetterAutocompleteRequest struct { - Facet string `json:"-" validate:"required,oneof=job_kind queue_name"` - Match string `json:"-" validate:"max=1024"` - Exclude []string `json:"-" validate:"omitempty,max=100,dive,max=1024"` + Facet riveruidriver.JobDeadLetterFacet `json:"-" validate:"required,oneof=job_kind queue_name"` + Match string `json:"-" validate:"max=1024"` + Exclude []string `json:"-" validate:"omitempty,max=100,dive,max=1024"` } func (req *deadLetterAutocompleteRequest) ExtractRaw(r *http.Request) error { query := r.URL.Query() - req.Facet, req.Match, req.Exclude = query.Get("facet"), query.Get("match"), query["exclude"] + req.Facet, req.Match, req.Exclude = riveruidriver.JobDeadLetterFacet(query.Get("facet")), query.Get("match"), query["exclude"] if err := validateDeadLetterText("match", req.Match); err != nil { return err } @@ -283,7 +287,7 @@ func (a *deadLetterAutocompleteEndpoint[TTx]) Execute(ctx context.Context, req * return nil, err } values, err := a.uiDriver.GetExecutor(a.DB).JobDeadLetterAutocomplete(ctx, &riveruidriver.JobDeadLetterAutocompleteParams{ - Facet: req.Facet, Match: req.Match, Exclude: req.Exclude, Max: 100, Schema: a.Client.Schema(), + Facet: req.Facet, Match: req.Match, Exclude: req.Exclude, Max: riveruidriver.JobDeadLetterAutocompleteMaxLimit, Schema: a.Client.Schema(), }) if err != nil { return nil, err diff --git a/riverproui/internal/prohandler/dead_letter_endpoints_test.go b/riverproui/internal/prohandler/dead_letter_endpoints_test.go index f448aa75..89fc8d3a 100644 --- a/riverproui/internal/prohandler/dead_letter_endpoints_test.go +++ b/riverproui/internal/prohandler/dead_letter_endpoints_test.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "reflect" "strings" "testing" "time" @@ -22,6 +23,7 @@ import ( "riverqueue.com/riverpro/driver" "riverqueue.com/riverui/internal/riverinternaltest/testfactory" + "riverqueue.com/riverui/internal/riveruidriver" ) func insertDeadLetterJob(ctx context.Context, t *testing.T, bundle *setupEndpointTestBundle) *rivertype.JobRow { @@ -77,7 +79,7 @@ func TestDeadLetterJobList(t *testing.T) { } resp, err = apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobListRequest{}) require.NoError(t, err) - require.Len(t, resp.Data, 20) + require.Len(t, resp.Data, riveruidriver.JobDeadLetterListMaxDefault) require.Equal(t, newest.ID, resp.Data[0].ID, "newest jobs must come first") resp, err = apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobListRequest{Limit: new(2)}) require.NoError(t, err) @@ -95,12 +97,30 @@ func TestDeadLetterJobList(t *testing.T) { resp, err = apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobListRequest{Queues: []string{"missing"}}) require.NoError(t, err) require.Empty(t, resp.Data) - for _, limit := range []int{-1, 0, 201} { + for _, limit := range []int{-1, 0, riveruidriver.JobDeadLetterListMaxLimit + 1} { _, err := apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterJobListRequest{Limit: &limit}) require.Error(t, err) } } +// The validate tags can't refer to constants, so check that they agree with the +// limits and facets that the drivers enforce. +func TestDeadLetterRequestTagsMatchDriver(t *testing.T) { + t.Parallel() + + validateTag := func(request any, fieldName string) string { + field, ok := reflect.TypeOf(request).FieldByName(fieldName) + require.True(t, ok) + return field.Tag.Get("validate") + } + require.Equal(t, + fmt.Sprintf("omitempty,min=1,max=%d", riveruidriver.JobDeadLetterListMaxLimit), + validateTag(deadLetterJobListRequest{}, "Limit")) + require.Equal(t, + fmt.Sprintf("required,oneof=%s %s", riveruidriver.JobDeadLetterFacetJobKind, riveruidriver.JobDeadLetterFacetQueueName), + validateTag(deadLetterAutocompleteRequest{}, "Facet")) +} + // validDeadLetterIDs and validDeadLetterValues return slices whose elements // pass validation, so that only the size of the slice is under test. func validDeadLetterIDs(count int) []int64 { @@ -466,7 +486,7 @@ func TestDeadLetterAutocomplete(t *testing.T) { } require.Equal(t, tt.want, got) } - for _, facet := range []string{"", "tags", "id"} { + for _, facet := range []riveruidriver.JobDeadLetterFacet{"", "tags", "id"} { _, err := apitest.InvokeHandler(ctx, endpoint.Execute, testMountOpts(t), &deadLetterAutocompleteRequest{Facet: facet}) var badRequest *apierror.BadRequest require.ErrorAs(t, err, &badRequest) From aa2a8f3a74653f0c66def277d516edc2f51bf457 Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Sat, 24 Oct 2026 13:37:23 -0500 Subject: [PATCH 19/19] declare dead letter list response shape The dead letter list response embeds the driver's result struct and shadows its `ID` field to serialize it as a string. The driver's JSON tags exist to decode the payload its query returns, so embedding them makes an internal detail the definition of the public API: renaming a key in the query would silently rename it in the response. Declare the response item as its own struct with its own JSON tags and map the driver result onto it. The wire format is unchanged: `id` as a string, then `attempt`, `finalized_at`, `kind`, `max_attempts`, and `queue`. A serialization test pins that exact output, including a null `finalized_at`. --- .../prohandler/dead_letter_endpoints.go | 28 +++++++++++++++---- .../prohandler/dead_letter_endpoints_test.go | 15 ++++++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/riverproui/internal/prohandler/dead_letter_endpoints.go b/riverproui/internal/prohandler/dead_letter_endpoints.go index f0f4a21b..7de38e7d 100644 --- a/riverproui/internal/prohandler/dead_letter_endpoints.go +++ b/riverproui/internal/prohandler/dead_letter_endpoints.go @@ -7,6 +7,7 @@ import ( "net/http" "strconv" "strings" + "time" "unicode/utf8" "github.com/riverqueue/apiframe/apiendpoint" @@ -48,11 +49,28 @@ func (a *ProAPIBundle[TTx]) requireDeadLetterJobs(ctx context.Context) error { return nil } -// IDs are strings so JavaScript can preserve the full int64 range. +// deadLetterJobListItem is the public shape of a job in the list response. It +// is declared here rather than built on the driver's result, whose JSON tags +// exist to decode a query and shouldn't define the API. IDs are strings so +// JavaScript can preserve the full int64 range. type deadLetterJobListItem struct { - *riveruidriver.JobDeadLetterListResult + ID int64 `json:"id,string"` + Attempt int `json:"attempt"` + FinalizedAt *time.Time `json:"finalized_at"` + Kind string `json:"kind"` + MaxAttempts int `json:"max_attempts"` + Queue string `json:"queue"` +} - ID int64 `json:"id,string"` +func deadLetterJobListItemFromDriver(job *riveruidriver.JobDeadLetterListResult) *deadLetterJobListItem { + return &deadLetterJobListItem{ + ID: job.ID, + Attempt: job.Attempt, + FinalizedAt: job.FinalizedAt, + Kind: job.Kind, + MaxAttempts: job.MaxAttempts, + Queue: job.Queue, + } } type deadLetterJob struct { @@ -169,9 +187,7 @@ func (a *deadLetterJobListEndpoint[TTx]) Execute(ctx context.Context, req *deadL if err != nil { return nil, fmt.Errorf("error listing dead letter jobs: %w", err) } - return listResponseFrom(sliceutil.Map(jobs, func(job *riveruidriver.JobDeadLetterListResult) *deadLetterJobListItem { - return &deadLetterJobListItem{JobDeadLetterListResult: job, ID: job.ID} - })), nil + return listResponseFrom(sliceutil.Map(jobs, deadLetterJobListItemFromDriver)), nil } type deadLetterJobRequest struct { diff --git a/riverproui/internal/prohandler/dead_letter_endpoints_test.go b/riverproui/internal/prohandler/dead_letter_endpoints_test.go index 89fc8d3a..001cfb80 100644 --- a/riverproui/internal/prohandler/dead_letter_endpoints_test.go +++ b/riverproui/internal/prohandler/dead_letter_endpoints_test.go @@ -379,6 +379,21 @@ func TestDeadLetterJobSerialization(t *testing.T) { require.Contains(t, string(encoded), `"errors":[]`) } +func TestDeadLetterJobListItemSerialization(t *testing.T) { + t.Parallel() + + finalizedAt := time.Date(2026, time.October, 21, 10, 2, 3, 456_000_000, time.UTC) + encoded, err := json.Marshal(deadLetterJobListItemFromDriver(&riveruidriver.JobDeadLetterListResult{ + ID: 1970670598291982290, Attempt: 3, FinalizedAt: &finalizedAt, Kind: "send_email", MaxAttempts: 5, Queue: "mail", + })) + require.NoError(t, err) + require.JSONEq(t, `{"id":"1970670598291982290","attempt":3,"finalized_at":"2026-10-21T10:02:03.456Z","kind":"send_email","max_attempts":5,"queue":"mail"}`, string(encoded)) + + encoded, err = json.Marshal(deadLetterJobListItemFromDriver(&riveruidriver.JobDeadLetterListResult{ID: 1})) + require.NoError(t, err) + require.JSONEq(t, `{"id":"1","attempt":0,"finalized_at":null,"kind":"","max_attempts":0,"queue":""}`, string(encoded)) +} + func TestDeadLetterJobListRequest(t *testing.T) { t.Parallel() for _, query := range []string{"limit=invalid", "limit=1.5", "limit=9223372036854775808", "ids=bad", "ids=1.5", "ids=9223372036854775808", "priorities=bad", "priorities=32768"} {