diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f0028184..9b940ccbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to ## [Unreleased] +### Fixed + +🐛(frontend) abort check media status unmount #2194 + ## [v4.8.6] - 2026-04-08 ### Added diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/api/__tests__/checkDocMediaStatus.test.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/api/__tests__/checkDocMediaStatus.test.tsx new file mode 100644 index 000000000..88660831e --- /dev/null +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/api/__tests__/checkDocMediaStatus.test.tsx @@ -0,0 +1,177 @@ +import fetchMock from 'fetch-mock'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { + checkDocMediaStatus, + loopCheckDocMediaStatus, +} from '../checkDocMediaStatus'; + +const VALID_URL = 'http://test.jest/media-check/some-file-id'; + +describe('checkDocMediaStatus', () => { + beforeEach(() => { + fetchMock.restore(); + }); + + afterEach(() => { + fetchMock.restore(); + }); + + it('returns the response when the status is ready', async () => { + fetchMock.get(VALID_URL, { + body: { status: 'ready', file: '/media/some-file.pdf' }, + }); + + const result = await checkDocMediaStatus({ urlMedia: VALID_URL }); + + expect(result).toEqual({ status: 'ready', file: '/media/some-file.pdf' }); + expect(fetchMock.lastOptions(VALID_URL)).toMatchObject({ + credentials: 'include', + }); + }); + + it('returns the response when the status is processing', async () => { + fetchMock.get(VALID_URL, { + body: { status: 'processing' }, + }); + + const result = await checkDocMediaStatus({ urlMedia: VALID_URL }); + + expect(result).toEqual({ status: 'processing' }); + }); + + it('throws an APIError when the URL is not safe', async () => { + await expect( + checkDocMediaStatus({ urlMedia: 'javascript:alert(1)' }), + ).rejects.toMatchObject({ status: 400 }); + + expect(fetchMock.calls().length).toBe(0); + }); + + it('throws an APIError when the URL does not contain the analyze path', async () => { + await expect( + checkDocMediaStatus({ urlMedia: 'http://test.jest/other/path' }), + ).rejects.toMatchObject({ status: 400 }); + + expect(fetchMock.calls().length).toBe(0); + }); + + it('throws an APIError when the fetch response is not ok', async () => { + fetchMock.get(VALID_URL, { + status: 500, + body: JSON.stringify({ detail: 'Internal server error' }), + }); + + await expect( + checkDocMediaStatus({ urlMedia: VALID_URL }), + ).rejects.toMatchObject({ status: 500 }); + }); + + it('forwards the AbortSignal to fetch', async () => { + const controller = new AbortController(); + controller.abort(); + + fetchMock.get(VALID_URL, { body: { status: 'ready' } }); + + await expect( + checkDocMediaStatus({ urlMedia: VALID_URL, signal: controller.signal }), + ).rejects.toThrow(); + }); +}); + +describe('loopCheckDocMediaStatus', () => { + beforeEach(() => { + vi.useFakeTimers(); + fetchMock.restore(); + }); + + afterEach(() => { + vi.useRealTimers(); + fetchMock.restore(); + }); + + it('resolves immediately when the status is already ready', async () => { + fetchMock.get(VALID_URL, { + body: { status: 'ready', file: '/media/file.pdf' }, + }); + + const result = await loopCheckDocMediaStatus( + VALID_URL, + new AbortController().signal, + ); + + expect(result).toEqual({ status: 'ready', file: '/media/file.pdf' }); + expect(fetchMock.calls().length).toBe(1); + }); + + it('retries until the status becomes ready', async () => { + let callCount = 0; + fetchMock.mock(VALID_URL, () => { + callCount++; + return { + status: 200, + body: JSON.stringify( + callCount >= 3 + ? { status: 'ready', file: '/media/file.pdf' } + : { status: 'processing' }, + ), + }; + }); + + const promise = loopCheckDocMediaStatus( + VALID_URL, + new AbortController().signal, + ); + + // Advance timers for each sleep between retries + await vi.runAllTimersAsync(); + + const result = await promise; + + expect(result).toEqual({ status: 'ready', file: '/media/file.pdf' }); + expect(fetchMock.calls().length).toBe(3); + }); + + it('throws an AbortError immediately when the signal is already aborted', async () => { + const controller = new AbortController(); + controller.abort(); + + fetchMock.get(VALID_URL, { body: { status: 'processing' } }); + + await expect( + loopCheckDocMediaStatus(VALID_URL, controller.signal), + ).rejects.toMatchObject({ name: 'AbortError' }); + + expect(fetchMock.calls().length).toBe(0); + }); + + it('stops the loop when the signal is aborted during a sleep', async () => { + fetchMock.get(VALID_URL, { body: { status: 'processing' } }); + + const controller = new AbortController(); + + const rejectExpectation = expect( + loopCheckDocMediaStatus(VALID_URL, controller.signal), + ).rejects.toMatchObject({ name: 'AbortError' }); + + controller.abort(); + + await rejectExpectation; + // Only the first request should have been made + expect(fetchMock.calls().length).toBe(1); + }); + + it('rejects when a fetch error occurs', async () => { + fetchMock.get(VALID_URL, { + status: 500, + body: JSON.stringify({ detail: 'Internal server error' }), + }); + + // Error happens on the first fetch — no timer advancement needed. + await expect( + loopCheckDocMediaStatus(VALID_URL, new AbortController().signal), + ).rejects.toMatchObject({ status: 500 }); + + expect(fetchMock.calls().length).toBe(1); + }); +}); diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/api/checkDocMediaStatus.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/api/checkDocMediaStatus.tsx index 0014547a5..27400223e 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/api/checkDocMediaStatus.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/api/checkDocMediaStatus.tsx @@ -1,5 +1,4 @@ import { APIError, errorCauses } from '@/api'; -import { sleep } from '@/utils'; import { isSafeUrl } from '@/utils/url'; import { ANALYZE_URL } from '../conf'; @@ -11,10 +10,12 @@ interface CheckDocMediaStatusResponse { interface CheckDocMediaStatus { urlMedia: string; + signal?: AbortSignal; } export const checkDocMediaStatus = async ({ urlMedia, + signal, }: CheckDocMediaStatus): Promise => { if (!isSafeUrl(urlMedia) || !urlMedia.includes(ANALYZE_URL)) { throw new APIError('Url invalid', { status: 400 }); @@ -22,6 +23,7 @@ export const checkDocMediaStatus = async ({ const response = await fetch(urlMedia, { credentials: 'include', + signal, }); if (!response.ok) { @@ -34,27 +36,56 @@ export const checkDocMediaStatus = async ({ return response.json() as Promise; }; +/** + * A sleep function that can be aborted using an AbortSignal. + * If the signal is aborted, the promise will reject with an 'Aborted' error. + * @param ms The number of milliseconds to sleep. + * @param signal The AbortSignal to cancel the sleep. + * @returns A promise that resolves after the specified time or rejects if aborted. + */ +const abortableSleep = (ms: number, signal: AbortSignal) => + new Promise((resolve, reject) => { + const timeout = setTimeout(resolve, ms); + signal.addEventListener( + 'abort', + () => { + clearTimeout(timeout); + reject(new DOMException('Aborted', 'AbortError')); + }, + { once: true }, + ); + }); + /** * Upload file can be analyzed on the server side, * we had this function to wait for the analysis to be done * before returning the file url. It will keep the loader * on the upload button until the analysis is done. * @param url + * @param signal AbortSignal to cancel the loop (e.g. on component unmount) * @returns Promise status_code * @description Waits for the upload to be analyzed by checking the status of the file. */ export const loopCheckDocMediaStatus = async ( url: string, + signal: AbortSignal, ): Promise => { const SLEEP_TIME = 5000; - const response = await checkDocMediaStatus({ - urlMedia: url, - }); + + /** + * Check if the signal has been aborted before making the API call. + * This prevents unnecessary API calls and allows for a faster response to cancellation. + */ + if (signal.aborted) { + throw new DOMException('Aborted', 'AbortError'); + } + + const response = await checkDocMediaStatus({ urlMedia: url, signal }); if (response.status === 'ready') { return response; - } else { - await sleep(SLEEP_TIME); - return await loopCheckDocMediaStatus(url); } + + await abortableSleep(SLEEP_TIME, signal); + return loopCheckDocMediaStatus(url, signal); }; diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-blocks/UploadLoaderBlock.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-blocks/UploadLoaderBlock.tsx index 7edffff2d..dc81107b9 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-blocks/UploadLoaderBlock.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-blocks/UploadLoaderBlock.tsx @@ -72,8 +72,9 @@ const UploadLoaderBlockComponent = ({ } const url = block.props.blockUploadUrl; + const controller = new AbortController(); - loopCheckDocMediaStatus(url) + loopCheckDocMediaStatus(url, controller.signal) .then((response) => { // Add random delay to reduce collision probability during collaboration const randomDelay = Math.random() * 800; @@ -101,7 +102,11 @@ const UploadLoaderBlockComponent = ({ } }, randomDelay); }) - .catch((error) => { + .catch((error: unknown) => { + if (error instanceof DOMException && error.name === 'AbortError') { + return; + } + console.error('Error analyzing file:', error); try { @@ -118,6 +123,10 @@ const UploadLoaderBlockComponent = ({ /* During collaboration, another user might have updated the block */ } }); + + return () => { + controller.abort(); + }; }, [block, editor, mediaUrl, isEditable]); return (