Skip to content

Commit ad184c4

Browse files
peppescgclaude
andauthored
fix(use-toast-mutation): show error toast when successMsg is null (#2294)
The entire `toast.promise(...)` call — including the `error:` branch — was gated on `resolvedSuccessMsg !== null`, so any configured `errorMsg` became dead code when callers passed `successMsg: null`. This silently swallowed mutation errors in flows that legitimately suppress the success toast (e.g. routes that navigate away on success) but still want user-visible error feedback. Register `toast.promise` whenever a success message or an error message is configured, and only emit the `success:` key when applicable. The silent-mode escape hatch (`successMsg: null` with no `errorMsg`) is preserved. Fixes #2293 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 30f2b11 commit ad184c4

2 files changed

Lines changed: 152 additions & 2 deletions

File tree

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
import { describe, it, expect, vi, beforeEach } from 'vitest'
2+
import { renderHook, waitFor } from '@testing-library/react'
3+
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
4+
import React from 'react'
5+
import { useToastMutation } from '../use-toast-mutation'
6+
7+
const mockToastPromise = vi.fn()
8+
vi.mock('sonner', () => ({
9+
toast: {
10+
promise: (...args: unknown[]) => mockToastPromise(...args),
11+
},
12+
}))
13+
14+
const createWrapper = () => {
15+
const queryClient = new QueryClient({
16+
defaultOptions: {
17+
queries: { retry: false },
18+
mutations: { retry: false },
19+
},
20+
})
21+
return ({ children }: { children: React.ReactNode }) =>
22+
React.createElement(QueryClientProvider, { client: queryClient }, children)
23+
}
24+
25+
describe('useToastMutation', () => {
26+
beforeEach(() => {
27+
vi.clearAllMocks()
28+
})
29+
30+
it('registers toast.promise when successMsg is provided', async () => {
31+
const { result } = renderHook(
32+
() =>
33+
useToastMutation({
34+
mutationFn: async () => 'ok',
35+
successMsg: 'Done',
36+
}),
37+
{ wrapper: createWrapper() }
38+
)
39+
40+
await result.current.mutateAsync(undefined)
41+
42+
expect(mockToastPromise).toHaveBeenCalledTimes(1)
43+
const [, config] = mockToastPromise.mock.calls[0]!
44+
expect(config.success).toBe('Done')
45+
expect(typeof config.error).toBe('function')
46+
})
47+
48+
// Regression: see issue #2293 — when successMsg is null, the error toast was
49+
// never registered, silently swallowing failures for callers that navigate
50+
// away on success (e.g. signin) but still want user-visible error feedback.
51+
it('still shows the error toast when successMsg is null but errorMsg is set', async () => {
52+
const error = new Error('boom')
53+
const { result } = renderHook(
54+
() =>
55+
useToastMutation({
56+
mutationFn: async () => {
57+
throw error
58+
},
59+
successMsg: null,
60+
errorMsg: 'Sign in failed. Please try again.',
61+
}),
62+
{ wrapper: createWrapper() }
63+
)
64+
65+
await expect(result.current.mutateAsync(undefined)).rejects.toBe(error)
66+
67+
expect(mockToastPromise).toHaveBeenCalledTimes(1)
68+
const [, config] = mockToastPromise.mock.calls[0]!
69+
expect(config.success).toBeUndefined()
70+
expect(config.error(error)).toBe('Sign in failed. Please try again.')
71+
})
72+
73+
it('invokes errorMsg as a function with the rejected error', async () => {
74+
const error = Object.assign(new Error('boom'), { detail: 'nope' })
75+
const errorMsg = vi.fn((e: unknown) => `got: ${(e as Error).message}`)
76+
77+
const { result } = renderHook(
78+
() =>
79+
useToastMutation({
80+
mutationFn: async () => {
81+
throw error
82+
},
83+
successMsg: null,
84+
errorMsg,
85+
}),
86+
{ wrapper: createWrapper() }
87+
)
88+
89+
await expect(result.current.mutateAsync(undefined)).rejects.toBe(error)
90+
91+
const [, config] = mockToastPromise.mock.calls[0]!
92+
expect(config.error(error)).toBe('got: boom')
93+
expect(errorMsg).toHaveBeenCalledWith(error)
94+
})
95+
96+
it('stays silent when both successMsg is null and errorMsg is not set', async () => {
97+
const { result } = renderHook(
98+
() =>
99+
useToastMutation({
100+
mutationFn: async () => {
101+
throw new Error('boom')
102+
},
103+
successMsg: null,
104+
}),
105+
{ wrapper: createWrapper() }
106+
)
107+
108+
await expect(result.current.mutateAsync(undefined)).rejects.toThrow('boom')
109+
110+
expect(mockToastPromise).not.toHaveBeenCalled()
111+
})
112+
113+
it('falls back to error.detail when no errorMsg is configured', async () => {
114+
const error = { detail: 'server says no' }
115+
const { result } = renderHook(
116+
() =>
117+
useToastMutation({
118+
mutationFn: async () => {
119+
throw error
120+
},
121+
successMsg: 'Done',
122+
}),
123+
{ wrapper: createWrapper() }
124+
)
125+
126+
await expect(result.current.mutateAsync(undefined)).rejects.toBe(error)
127+
128+
const [, config] = mockToastPromise.mock.calls[0]!
129+
expect(config.error(error)).toBe('server says no')
130+
})
131+
132+
it('passes through toastId when provided', async () => {
133+
const { result } = renderHook(
134+
() =>
135+
useToastMutation({
136+
mutationFn: async () => 'ok',
137+
successMsg: null,
138+
errorMsg: 'X',
139+
toastId: 'signin',
140+
}),
141+
{ wrapper: createWrapper() }
142+
)
143+
144+
await result.current.mutateAsync(undefined).catch(() => {})
145+
146+
await waitFor(() => expect(mockToastPromise).toHaveBeenCalled())
147+
const [, config] = mockToastPromise.mock.calls[0]!
148+
expect(config.id).toBe('signin')
149+
})
150+
})

renderer/src/common/hooks/use-toast-mutation.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ export function useToastMutation<
4747
? loadingMsg(variables)
4848
: (loadingMsg ?? 'Loading...')
4949

50-
if (resolvedSuccessMsg !== null) {
50+
if (resolvedSuccessMsg !== null || errorMsg !== undefined) {
5151
toast.promise(promise, {
52-
success: resolvedSuccessMsg,
52+
...(resolvedSuccessMsg !== null && { success: resolvedSuccessMsg }),
5353
loading: resolvedLoadingMsg,
5454
error: (e: TError) => {
5555
if (typeof errorMsg === 'function') return errorMsg(e)

0 commit comments

Comments
 (0)