diff --git a/web/service/__tests__/request-error-toast.spec.ts b/web/service/__tests__/request-error-toast.spec.ts new file mode 100644 index 00000000000..c76780b4c29 --- /dev/null +++ b/web/service/__tests__/request-error-toast.spec.ts @@ -0,0 +1,204 @@ +import { toast } from '@langgenius/dify-ui/toast' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vite-plus/test' +// oxlint-disable-next-line no-restricted-imports -- Exercise the shared request transport and its notification behavior. +import { base } from '../fetch' +import { clearRequestErrorToasts, notifyRequestError } from '../request-error-toast' + +vi.mock('@langgenius/dify-ui/toast', () => ({ + toast: { error: vi.fn() }, +})) + +describe('request error toast timer lifecycle', () => { + const request = { method: 'GET', url: 'https://example.com/apps' } + + beforeEach(() => { + vi.useFakeTimers() + vi.clearAllMocks() + }) + + afterEach(() => { + vi.runOnlyPendingTimers() + vi.useRealTimers() + }) + + it('expires by wall clock even when the timer callback has not run', () => { + notifyRequestError(request, 'A') + + vi.setSystemTime(Date.now() + 10000) + notifyRequestError(request, 'A') + + expect(vi.mocked(toast.error).mock.calls).toEqual([['A'], ['A']]) + }) + + it('keeps a reopened window active past the cleared window’s old deadline', async () => { + notifyRequestError(request, 'A') + await vi.advanceTimersByTimeAsync(2000) + clearRequestErrorToasts(request) + notifyRequestError(request, 'A') + + await vi.advanceTimersByTimeAsync(8000) + notifyRequestError(request, 'A') + expect(toast.error).toHaveBeenCalledTimes(2) + + await vi.advanceTimersByTimeAsync(2000) + notifyRequestError(request, 'A') + expect(toast.error).toHaveBeenCalledTimes(3) + }) + + it('expires staggered messages independently', async () => { + notifyRequestError(request, 'A') + await vi.advanceTimersByTimeAsync(2000) + notifyRequestError(request, 'B') + + await vi.advanceTimersByTimeAsync(8000) + notifyRequestError(request, 'A') + notifyRequestError(request, 'B') + expect(vi.mocked(toast.error).mock.calls).toEqual([['A'], ['B'], ['A']]) + + await vi.advanceTimersByTimeAsync(2000) + notifyRequestError(request, 'A') + notifyRequestError(request, 'B') + expect(vi.mocked(toast.error).mock.calls).toEqual([['A'], ['B'], ['A'], ['B']]) + }) +}) + +const errorResponse = (message = 'Unavailable', status = 500) => + new Response(JSON.stringify({ message, status }), { + status, + headers: { 'Content-Type': 'application/json' }, + }) + +const expectError = async (...args: Parameters) => { + await expect(base(...args)).rejects.toMatchObject({ status: 500 }) +} + +describe('request error toast deduplication', () => { + const network = vi.fn() + + beforeEach(() => { + vi.useFakeTimers() + vi.clearAllMocks() + network.mockReset() + network.mockImplementation(async () => errorResponse()) + vi.stubGlobal('fetch', network) + }) + + afterEach(() => { + vi.runOnlyPendingTimers() + vi.useRealTimers() + vi.unstubAllGlobals() + }) + + it('shows immediately and again at 10000ms without extending the window for suppressed errors', async () => { + await expectError('/apps') + expect(toast.error).toHaveBeenCalledExactlyOnceWith('Unavailable') + + await vi.advanceTimersByTimeAsync(5000) + await expectError('/apps') + expect(toast.error).toHaveBeenCalledTimes(1) + + // Reach 9999ms from the last displayed toast. + await vi.advanceTimersByTimeAsync(4999) + await expectError('/apps') + expect(toast.error).toHaveBeenCalledTimes(1) + + await vi.advanceTimersByTimeAsync(1) + await expectError('/apps') + expect(toast.error).toHaveBeenCalledTimes(2) + + await expectError('/apps') + expect(toast.error).toHaveBeenCalledTimes(2) + expect(network).toHaveBeenCalledTimes(5) + }) + + it('shows each distinct message while suppressing a previously shown message', async () => { + network + .mockResolvedValueOnce(errorResponse('A')) + .mockResolvedValueOnce(errorResponse('B')) + .mockResolvedValueOnce(errorResponse('A')) + + await expectError('/apps') + await expectError('/apps') + await expectError('/apps') + + expect(toast.error).toHaveBeenCalledTimes(2) + expect(toast.error).toHaveBeenNthCalledWith(1, 'A') + expect(toast.error).toHaveBeenNthCalledWith(2, 'B') + }) + + it.each([ + ['URL', '/datasets', {}], + ['query params', '/apps', { params: { page: 2 } }], + ] as const)( + 'shows the same message independently for a different %s', + async (_, url, options) => { + await expectError('/apps') + await expectError(url, options) + await expectError('/apps') + await expectError(url, options) + + expect(toast.error).toHaveBeenCalledTimes(2) + expect(toast.error).toHaveBeenNthCalledWith(1, 'Unavailable') + expect(toast.error).toHaveBeenNthCalledWith(2, 'Unavailable') + }, + ) + + it('shows every POST, PUT, PATCH, and DELETE error even while the same GET error is suppressed', async () => { + await expectError('/apps') + await expectError('/apps') + expect(toast.error).toHaveBeenCalledExactlyOnceWith('Unavailable') + + for (const method of ['POST', 'PUT', 'PATCH', 'DELETE']) { + const previousCount = vi.mocked(toast.error).mock.calls.length + await expectError('/apps', { method }) + await expectError('/apps', { method }) + expect(toast.error).toHaveBeenCalledTimes(previousCount + 2) + expect(toast.error).toHaveBeenNthCalledWith(previousCount + 1, 'Unavailable') + expect(toast.error).toHaveBeenNthCalledWith(previousCount + 2, 'Unavailable') + } + + await expectError('/apps') + expect(toast.error).toHaveBeenCalledTimes(9) + }) + + it('clears all messages after success only for the successful request', async () => { + await expectError('/datasets') + network + .mockResolvedValueOnce(errorResponse('A')) + .mockResolvedValueOnce(errorResponse('B')) + .mockResolvedValueOnce( + new Response(JSON.stringify({ result: 'success' }), { + headers: { 'Content-Type': 'application/json' }, + }), + ) + .mockResolvedValueOnce(errorResponse('A')) + .mockResolvedValueOnce(errorResponse('B')) + + await expectError('/apps') + await expectError('/apps') + await expect(base('/apps')).resolves.toEqual({ result: 'success' }) + await expectError('/apps') + await expectError('/apps') + await expectError('/datasets') + + expect(vi.mocked(toast.error).mock.calls).toEqual([['Unavailable'], ['A'], ['B'], ['A'], ['B']]) + }) + + it('keeps silent errors silent without suppressing the next visible error', async () => { + await expectError('/apps', {}, { silent: true }) + expect(toast.error).not.toHaveBeenCalled() + + await expectError('/apps') + expect(toast.error).toHaveBeenCalledExactlyOnceWith('Unavailable') + }) + + it('suppresses 401 toasts without suppressing a subsequent non-401 error with the same message', async () => { + network.mockResolvedValueOnce(errorResponse('Unavailable', 401)) + + await expect(base('/apps')).rejects.toMatchObject({ status: 401 }) + expect(toast.error).not.toHaveBeenCalled() + + await expectError('/apps') + expect(toast.error).toHaveBeenCalledExactlyOnceWith('Unavailable') + }) +}) diff --git a/web/service/fetch.ts b/web/service/fetch.ts index 7dfd5453da7..c6dc9b0f519 100644 --- a/web/service/fetch.ts +++ b/web/service/fetch.ts @@ -1,6 +1,5 @@ import type { AfterResponseHook, BeforeRequestHook, Hooks } from 'ky' import type { IOtherOptions } from './base' -import { toast } from '@langgenius/dify-ui/toast' import Cookies from 'js-cookie' import ky, { HTTPError } from 'ky' import { @@ -15,6 +14,7 @@ import { WEB_APP_SHARE_CODE_HEADER_NAME, } from '@/config' import { shouldSuppressAppDeletionErrorToast } from './app-deletion' +import { clearRequestErrorToasts, notifyRequestError } from './request-error-toast' import { getWebAppPublicApiPath, resolveWebAppAddress } from './webapp-address' import { getWebAppAccessToken, getWebAppPassport } from './webapp-auth' @@ -84,10 +84,12 @@ const afterResponseErrorCode = (otherOptions: IOtherOptions): AfterResponseHook !shouldSuppressAppDeletionErrorToast(request.url, response.status) const errorMessage = errorData?.message || errorData?.error - if (shouldNotifyError && errorMessage) toast.error(errorMessage) + if (shouldNotifyError && errorMessage) notifyRequestError(request, errorMessage) if (response.status === 403 && errorData?.code === 'already_setup') globalThis.location.href = `${globalThis.location.origin}/signin` + } else { + clearRequestErrorToasts(request) } } } diff --git a/web/service/request-error-toast.ts b/web/service/request-error-toast.ts new file mode 100644 index 00000000000..5588e0753c1 --- /dev/null +++ b/web/service/request-error-toast.ts @@ -0,0 +1,44 @@ +import { toast } from '@langgenius/dify-ui/toast' + +const DEDUPE_WINDOW_MS = 10000 + +type RequestIdentity = Pick +type ErrorNotification = { + expiresAt: number + timer: ReturnType +} + +const notifications = new Map>() +const requestKey = (request: RequestIdentity) => JSON.stringify([request.method, request.url]) + +export function clearRequestErrorToasts(request: RequestIdentity) { + const key = requestKey(request) + const messages = notifications.get(key) + if (!messages) return + for (const notification of messages.values()) clearTimeout(notification.timer) + notifications.delete(key) +} + +export function notifyRequestError(request: RequestIdentity, message: string) { + // Writes can be separate user actions even when their errors are identical. + if (request.method !== 'GET') { + toast.error(message) + return + } + + const key = requestKey(request) + const messages = notifications.get(key) ?? new Map() + notifications.set(key, messages) + const previous = messages.get(message) + const now = Date.now() + if (previous && now < previous.expiresAt) return + if (previous) clearTimeout(previous.timer) + + // Only a displayed toast starts the window; suppressed retries do not extend it. + const timer = setTimeout(() => { + messages.delete(message) + if (messages.size === 0) notifications.delete(key) + }, DEDUPE_WINDOW_MS) + messages.set(message, { expiresAt: now + DEDUPE_WINDOW_MS, timer }) + toast.error(message) +}