From df6a46391ca98e7dd70cde143f26bbaaeaa43b9a Mon Sep 17 00:00:00 2001 From: Giuseppe Scuglia Date: Thu, 28 May 2026 13:45:47 +0200 Subject: [PATCH] fix(use-toast-mutation): show error toast when successMsg is null MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../__tests__/use-toast-mutation.test.tsx | 150 ++++++++++++++++++ .../src/common/hooks/use-toast-mutation.ts | 4 +- 2 files changed, 152 insertions(+), 2 deletions(-) create mode 100644 renderer/src/common/hooks/__tests__/use-toast-mutation.test.tsx diff --git a/renderer/src/common/hooks/__tests__/use-toast-mutation.test.tsx b/renderer/src/common/hooks/__tests__/use-toast-mutation.test.tsx new file mode 100644 index 000000000..8af3be98c --- /dev/null +++ b/renderer/src/common/hooks/__tests__/use-toast-mutation.test.tsx @@ -0,0 +1,150 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { renderHook, waitFor } from '@testing-library/react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import React from 'react' +import { useToastMutation } from '../use-toast-mutation' + +const mockToastPromise = vi.fn() +vi.mock('sonner', () => ({ + toast: { + promise: (...args: unknown[]) => mockToastPromise(...args), + }, +})) + +const createWrapper = () => { + const queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false }, + mutations: { retry: false }, + }, + }) + return ({ children }: { children: React.ReactNode }) => + React.createElement(QueryClientProvider, { client: queryClient }, children) +} + +describe('useToastMutation', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('registers toast.promise when successMsg is provided', async () => { + const { result } = renderHook( + () => + useToastMutation({ + mutationFn: async () => 'ok', + successMsg: 'Done', + }), + { wrapper: createWrapper() } + ) + + await result.current.mutateAsync(undefined) + + expect(mockToastPromise).toHaveBeenCalledTimes(1) + const [, config] = mockToastPromise.mock.calls[0]! + expect(config.success).toBe('Done') + expect(typeof config.error).toBe('function') + }) + + // Regression: see issue #2293 — when successMsg is null, the error toast was + // never registered, silently swallowing failures for callers that navigate + // away on success (e.g. signin) but still want user-visible error feedback. + it('still shows the error toast when successMsg is null but errorMsg is set', async () => { + const error = new Error('boom') + const { result } = renderHook( + () => + useToastMutation({ + mutationFn: async () => { + throw error + }, + successMsg: null, + errorMsg: 'Sign in failed. Please try again.', + }), + { wrapper: createWrapper() } + ) + + await expect(result.current.mutateAsync(undefined)).rejects.toBe(error) + + expect(mockToastPromise).toHaveBeenCalledTimes(1) + const [, config] = mockToastPromise.mock.calls[0]! + expect(config.success).toBeUndefined() + expect(config.error(error)).toBe('Sign in failed. Please try again.') + }) + + it('invokes errorMsg as a function with the rejected error', async () => { + const error = Object.assign(new Error('boom'), { detail: 'nope' }) + const errorMsg = vi.fn((e: unknown) => `got: ${(e as Error).message}`) + + const { result } = renderHook( + () => + useToastMutation({ + mutationFn: async () => { + throw error + }, + successMsg: null, + errorMsg, + }), + { wrapper: createWrapper() } + ) + + await expect(result.current.mutateAsync(undefined)).rejects.toBe(error) + + const [, config] = mockToastPromise.mock.calls[0]! + expect(config.error(error)).toBe('got: boom') + expect(errorMsg).toHaveBeenCalledWith(error) + }) + + it('stays silent when both successMsg is null and errorMsg is not set', async () => { + const { result } = renderHook( + () => + useToastMutation({ + mutationFn: async () => { + throw new Error('boom') + }, + successMsg: null, + }), + { wrapper: createWrapper() } + ) + + await expect(result.current.mutateAsync(undefined)).rejects.toThrow('boom') + + expect(mockToastPromise).not.toHaveBeenCalled() + }) + + it('falls back to error.detail when no errorMsg is configured', async () => { + const error = { detail: 'server says no' } + const { result } = renderHook( + () => + useToastMutation({ + mutationFn: async () => { + throw error + }, + successMsg: 'Done', + }), + { wrapper: createWrapper() } + ) + + await expect(result.current.mutateAsync(undefined)).rejects.toBe(error) + + const [, config] = mockToastPromise.mock.calls[0]! + expect(config.error(error)).toBe('server says no') + }) + + it('passes through toastId when provided', async () => { + const { result } = renderHook( + () => + useToastMutation({ + mutationFn: async () => 'ok', + successMsg: null, + errorMsg: 'X', + toastId: 'signin', + }), + { wrapper: createWrapper() } + ) + + await result.current.mutateAsync(undefined).catch(() => {}) + + await waitFor(() => expect(mockToastPromise).toHaveBeenCalled()) + const [, config] = mockToastPromise.mock.calls[0]! + expect(config.id).toBe('signin') + }) +}) diff --git a/renderer/src/common/hooks/use-toast-mutation.ts b/renderer/src/common/hooks/use-toast-mutation.ts index f8eaf6f3f..f67df4c9e 100644 --- a/renderer/src/common/hooks/use-toast-mutation.ts +++ b/renderer/src/common/hooks/use-toast-mutation.ts @@ -47,9 +47,9 @@ export function useToastMutation< ? loadingMsg(variables) : (loadingMsg ?? 'Loading...') - if (resolvedSuccessMsg !== null) { + if (resolvedSuccessMsg !== null || errorMsg !== undefined) { toast.promise(promise, { - success: resolvedSuccessMsg, + ...(resolvedSuccessMsg !== null && { success: resolvedSuccessMsg }), loading: resolvedLoadingMsg, error: (e: TError) => { if (typeof errorMsg === 'function') return errorMsg(e)