From c1e104a686f5f6bb2c4f6ed282f5987dc69a135c Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Thu, 9 Apr 2026 11:51:19 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(frontend)=20abort=20check=20media?= =?UTF-8?q?=20status=20unmount?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a media file is uploaded, the application checks its status every 5 seconds until it becomes 'ready'. If the user navigates away from the page before the media is ready, the application should stop checking the status to avoid unnecessary API calls. This can be achieved by using an AbortController to signal when the component is unmounted, allowing the loop to exit gracefully. --- CHANGELOG.md | 4 + .../__tests__/checkDocMediaStatus.test.tsx | 177 ++++++++++++++++++ .../doc-editor/api/checkDocMediaStatus.tsx | 45 ++++- .../custom-blocks/UploadLoaderBlock.tsx | 13 +- 4 files changed, 230 insertions(+), 9 deletions(-) create mode 100644 src/frontend/apps/impress/src/features/docs/doc-editor/api/__tests__/checkDocMediaStatus.test.tsx 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 (