diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnection.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnection.test.tsx index 127b7343758..2baaf3abef2 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnection.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnection.test.tsx @@ -173,6 +173,7 @@ describe('Test Connection Component', () => { await act(async () => { render(); }); + const controller = new AbortController(); const testConnectionButton = screen.getByTestId('test-connection-btn'); @@ -180,13 +181,22 @@ describe('Test Connection Component', () => { userEvent.click(testConnectionButton); }); - expect(addWorkflow).toHaveBeenCalledWith(CREATE_WORKFLOW_PAYLOAD); + expect(addWorkflow).toHaveBeenCalledWith( + CREATE_WORKFLOW_PAYLOAD, + controller.signal + ); - expect(triggerWorkflowById).toHaveBeenCalledWith(WORKFLOW_DETAILS.id); + expect(triggerWorkflowById).toHaveBeenCalledWith( + WORKFLOW_DETAILS.id, + controller.signal + ); jest.advanceTimersByTime(2000); - expect(getWorkflowById).toHaveBeenCalledWith(WORKFLOW_DETAILS.id); + expect(getWorkflowById).toHaveBeenCalledWith( + WORKFLOW_DETAILS.id, + controller.signal + ); }); it('Should show success message if test connection successful', async () => { @@ -293,6 +303,13 @@ describe('Test Connection Component', () => { it('Should timeout message after two minutes', async () => { jest.useFakeTimers(); + (addWorkflow as jest.Mock).mockImplementationOnce(() => + Promise.resolve({ + ...WORKFLOW_DETAILS, + status: WorkflowStatus.Pending, + }) + ); + (getWorkflowById as jest.Mock).mockImplementationOnce(() => Promise.resolve({ ...WORKFLOW_DETAILS, @@ -485,15 +502,23 @@ describe('Test Connection Component', () => { render(); }); + const controller = new AbortController(); + const testConnectionButton = screen.getByTestId('test-connection-btn'); await act(async () => { userEvent.click(testConnectionButton); }); - expect(addWorkflow).toHaveBeenCalledWith(CREATE_WORKFLOW_PAYLOAD); + expect(addWorkflow).toHaveBeenCalledWith( + CREATE_WORKFLOW_PAYLOAD, + controller.signal + ); - expect(triggerWorkflowById).toHaveBeenCalledWith(WORKFLOW_DETAILS.id); + expect(triggerWorkflowById).toHaveBeenCalledWith( + WORKFLOW_DETAILS.id, + controller.signal + ); jest.advanceTimersByTime(2000); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnection.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnection.tsx index 733f7eca853..cab55452ab7 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnection.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnection.tsx @@ -42,6 +42,7 @@ import { WorkflowType, } from '../../../generated/entity/automations/workflow'; import { TestConnectionStep } from '../../../generated/entity/services/connections/testConnectionDefinition'; +import useAbortController from '../../../hooks/AbortController/useAbortController'; import { useAirflowStatus } from '../../../hooks/useAirflowStatus'; import { addWorkflow, @@ -108,6 +109,8 @@ const TestConnection: FC = ({ */ const currentWorkflowRef = useRef(currentWorkflow); + const { controller } = useAbortController(); + const serviceType = useMemo(() => { return getServiceType(serviceCategory); }, [serviceCategory]); @@ -138,9 +141,12 @@ const TestConnection: FC = ({ } }; - const getWorkflowData = async (workflowId: string) => { + const getWorkflowData = async ( + workflowId: string, + apiCancelSignal: AbortSignal + ) => { try { - const response = await getWorkflowById(workflowId); + const response = await getWorkflowById(workflowId, apiCancelSignal); const testConnectionStepResult = response.response?.steps ?? []; setTestConnectionStepResult(testConnectionStepResult); @@ -169,11 +175,87 @@ const TestConnection: FC = ({ try { await deleteWorkflowById(workflowId, true); + setCurrentWorkflow(undefined); } catch (error) { // do not throw error for this API } }; + const handleCompletionStatus = async ( + isTestConnectionSuccess: boolean, + steps: TestConnectionStepResult[] + ) => { + setProgress(TEST_CONNECTION_PROGRESS_PERCENTAGE.HUNDRED); + if (isTestConnectionSuccess) { + setTestStatus(StatusType.Successful); + setMessage(TEST_CONNECTION_SUCCESS_MESSAGE); + } else { + const isMandatoryStepsFailing = steps.some( + (step) => step.mandatory && !step.passed + ); + setTestStatus(isMandatoryStepsFailing ? StatusType.Failed : 'Warning'); + setMessage( + isMandatoryStepsFailing + ? TEST_CONNECTION_FAILURE_MESSAGE + : TEST_CONNECTION_WARNING_MESSAGE + ); + } + }; + + const handleWorkflowPolling = async ( + response: Workflow, + intervalId: number | undefined + ) => { + // return a promise that wraps the interval and handles errors inside it + return new Promise((resolve, reject) => { + /** + * fetch workflow repeatedly with 2s interval + * until status is either Failed or Successful + */ + intervalId = toNumber( + setInterval(async () => { + setProgress((prev) => prev + TEST_CONNECTION_PROGRESS_PERCENTAGE.ONE); + try { + const workflowResponse = await getWorkflowData( + response.id, + controller.signal + ); + const { response: testConnectionResponse } = workflowResponse; + const { status: testConnectionStatus, steps = [] } = + testConnectionResponse || {}; + + const isWorkflowCompleted = WORKFLOW_COMPLETE_STATUS.includes( + workflowResponse.status as WorkflowStatus + ); + + const isTestConnectionSuccess = + testConnectionStatus === StatusType.Successful; + + if (!isWorkflowCompleted) { + return; + } + + // Handle completion status + await handleCompletionStatus(isTestConnectionSuccess, steps); + + // clear the current interval + clearInterval(intervalId); + + // set testing connection to false + setIsTestingConnection(false); + + // delete the workflow once it's finished + await handleDeleteWorkflow(workflowResponse.id); + + resolve(); + } catch (error) { + reject(error as AxiosError); + } + }, FETCH_INTERVAL) + ); + }); + }; + // handlers const testConnection = async () => { setIsTestingConnection(true); @@ -203,12 +285,14 @@ const TestConnection: FC = ({ setProgress(TEST_CONNECTION_PROGRESS_PERCENTAGE.TEN); // create the workflow - const response = await addWorkflow(createWorkflowData); + const response = await addWorkflow(createWorkflowData, controller.signal); + + setCurrentWorkflow(response); setProgress(TEST_CONNECTION_PROGRESS_PERCENTAGE.TWENTY); // trigger the workflow - const status = await triggerWorkflowById(response.id); + const status = await triggerWorkflowById(response.id, controller.signal); setProgress(TEST_CONNECTION_PROGRESS_PERCENTAGE.FORTY); @@ -223,58 +307,6 @@ const TestConnection: FC = ({ return; } - /** - * fetch workflow repeatedly with 2s interval - * until status is either Failed or Successful - */ - intervalId = toNumber( - setInterval(async () => { - setProgress((prev) => prev + TEST_CONNECTION_PROGRESS_PERCENTAGE.ONE); - const workflowResponse = await getWorkflowData(response.id); - const { response: testConnectionResponse } = workflowResponse; - const { status: testConnectionStatus, steps = [] } = - testConnectionResponse || {}; - - const isWorkflowCompleted = WORKFLOW_COMPLETE_STATUS.includes( - workflowResponse.status as WorkflowStatus - ); - - const isTestConnectionSuccess = - testConnectionStatus === StatusType.Successful; - - if (!isWorkflowCompleted) { - return; - } - - setProgress(TEST_CONNECTION_PROGRESS_PERCENTAGE.HUNDRED); - if (isTestConnectionSuccess) { - setTestStatus(StatusType.Successful); - setMessage(TEST_CONNECTION_SUCCESS_MESSAGE); - } else { - const isMandatoryStepsFailing = steps.some( - (step) => step.mandatory && !step.passed - ); - setTestStatus( - isMandatoryStepsFailing ? StatusType.Failed : 'Warning' - ); - setMessage( - isMandatoryStepsFailing - ? TEST_CONNECTION_FAILURE_MESSAGE - : TEST_CONNECTION_WARNING_MESSAGE - ); - } - - // clear the current interval - clearInterval(intervalId); - - // set testing connection to false - setIsTestingConnection(false); - - // delete the workflow once it's finished - await handleDeleteWorkflow(workflowResponse.id); - }, FETCH_INTERVAL) - ); - // stop fetching the workflow after 2 minutes setTimeout(() => { // clear the current interval @@ -296,6 +328,9 @@ const TestConnection: FC = ({ setIsTestingConnection(false); setProgress(TEST_CONNECTION_PROGRESS_PERCENTAGE.HUNDRED); }, FETCHING_EXPIRY_TIME); + + // Handle workflow polling and completion + await handleWorkflowPolling(response, intervalId); } catch (error) { setProgress(TEST_CONNECTION_PROGRESS_PERCENTAGE.HUNDRED); clearInterval(intervalId); @@ -324,6 +359,11 @@ const TestConnection: FC = ({ } }; + const handleCancelTestConnectionModal = () => { + controller.abort(); + setDialogOpen(false); + }; + useEffect(() => { currentWorkflowRef.current = currentWorkflow; // update ref with latest value of currentWorkflow state variable }, [currentWorkflow]); @@ -435,7 +475,7 @@ const TestConnection: FC = ({ progress={progress} testConnectionStep={testConnectionStep} testConnectionStepResult={testConnectionStepResult} - onCancel={() => setDialogOpen(false)} + onCancel={handleCancelTestConnectionModal} onConfirm={() => setDialogOpen(false)} onTestConnection={handleTestConnection} /> diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnectionModal/TestConnectionModal.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnectionModal/TestConnectionModal.tsx index bade4e45467..c0c5c126a84 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnectionModal/TestConnectionModal.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnectionModal/TestConnectionModal.tsx @@ -44,8 +44,8 @@ const TestConnectionModal: FC = ({ isTestingConnection, testConnectionStep, testConnectionStepResult, - onCancel, onConfirm, + onCancel, isConnectionTimeout, onTestConnection, }) => { diff --git a/openmetadata-ui/src/main/resources/ui/src/hooks/AbortController/useAbortController.test.tsx b/openmetadata-ui/src/main/resources/ui/src/hooks/AbortController/useAbortController.test.tsx new file mode 100644 index 00000000000..1b91a899820 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/hooks/AbortController/useAbortController.test.tsx @@ -0,0 +1,77 @@ +/* + * Copyright 2025 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { act, renderHook } from '@testing-library/react-hooks'; +import axios from 'axios'; +import useAbortController from './useAbortController'; // Path to the custom hook + +jest.mock('axios'); + +// Mock the global AbortController +beforeAll(() => { + global.AbortController = jest.fn().mockImplementation(() => ({ + signal: {}, + abort: jest.fn(), + })); +}); + +describe('useAbortController', () => { + it('should call abort on cleanup or component unmount', () => { + const abortMock = jest.fn(); + + const { result, unmount } = renderHook(() => useAbortController()); + + // assign the mock to the abort method of the controller + result.current.controller.abort = abortMock; + + act(() => { + unmount(); + }); + + expect(abortMock).toHaveBeenCalled(); + }); + + // Test that the signal is passed correctly to Axios for canceling requests + it('should pass signal to Axios request for cancellation', async () => { + const { result, unmount } = renderHook(() => useAbortController()); + + // mock axios and track calls + axios.get = jest.fn().mockResolvedValue({ data: 'test' }); + + act(() => { + const fetchData = async () => { + try { + await axios.get('/api/test', { + signal: result.current.controller.signal, + }); + } catch (error) { + // Request aborted + } + }; + + fetchData(); + }); + + // Simulate the component unmounting to trigger abortion + act(() => { + unmount(); + }); + + // Check that Axios's cancel logic was triggered + expect(axios.get).toHaveBeenCalledWith( + '/api/test', + expect.objectContaining({ + signal: new AbortController().signal, + }) + ); + }); +}); diff --git a/openmetadata-ui/src/main/resources/ui/src/hooks/AbortController/useAbortController.ts b/openmetadata-ui/src/main/resources/ui/src/hooks/AbortController/useAbortController.ts new file mode 100644 index 00000000000..5b37263d492 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/hooks/AbortController/useAbortController.ts @@ -0,0 +1,35 @@ +/* + * Copyright 2024 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { useEffect, useRef } from 'react'; + +const useAbortController = () => { + const controllerRef = useRef(new AbortController()); + + useEffect(() => { + if (controllerRef.current.signal.aborted) { + controllerRef.current = new AbortController(); + } + }, [controllerRef.current.signal.aborted]); + + useEffect(() => { + return () => { + controllerRef.current.abort(); + }; + }, []); + + return { + controller: controllerRef.current, + }; +}; + +export default useAbortController; diff --git a/openmetadata-ui/src/main/resources/ui/src/rest/workflowAPI.ts b/openmetadata-ui/src/main/resources/ui/src/rest/workflowAPI.ts index 1b72d127900..8f46d62d380 100644 --- a/openmetadata-ui/src/main/resources/ui/src/rest/workflowAPI.ts +++ b/openmetadata-ui/src/main/resources/ui/src/rest/workflowAPI.ts @@ -29,11 +29,16 @@ export const getTestConnectionDefinitionByName = async ( return response.data; }; -export const addWorkflow = async (data: CreateWorkflow) => { +export const addWorkflow = async ( + data: CreateWorkflow, + apiCancelSignal: AbortSignal +) => { const response = await APIClient.post< CreateWorkflow, AxiosResponse - >(`automations/workflows`, data); + >(`automations/workflows`, data, { + signal: apiCancelSignal, + }); return response.data; }; @@ -43,17 +48,33 @@ export const addWorkflow = async (data: CreateWorkflow) => { * @param workflowId workflow to run * @returns status code like 200, 400, etc. */ -export const triggerWorkflowById = async (workflowId: string) => { +export const triggerWorkflowById = async ( + workflowId: string, + apiCancelSignal: AbortSignal +) => { const response = await APIClient.post( - `automations/workflows/trigger/${workflowId}` + `automations/workflows/trigger/${workflowId}`, + null, + { + signal: apiCancelSignal, + headers: { + 'Content-Type': 'application/json', + }, + } ); return response.status; }; -export const getWorkflowById = async (workflowId: string) => { +export const getWorkflowById = async ( + workflowId: string, + apiCancelSignal: AbortSignal +) => { const response = await APIClient.get( - `automations/workflows/${workflowId}` + `automations/workflows/${workflowId}`, + { + signal: apiCancelSignal, + } ); return response.data; diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/ToastUtils.ts b/openmetadata-ui/src/main/resources/ui/src/utils/ToastUtils.ts index 2d96c5dadf1..099c5d6cee1 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/ToastUtils.ts +++ b/openmetadata-ui/src/main/resources/ui/src/utils/ToastUtils.ts @@ -11,7 +11,7 @@ * limitations under the License. */ -import { AxiosError } from 'axios'; +import { AxiosError, isCancel } from 'axios'; import { isEmpty, isString } from 'lodash'; import React from 'react'; import { toast } from 'react-toastify'; @@ -47,11 +47,14 @@ export const showErrorToast = ( autoCloseTimer?: number, callback?: (value: React.SetStateAction) => void ) => { + if (isCancel(error)) { + return; + } let errorMessage; if (isString(error)) { errorMessage = error.toString(); } else { - const method = error.config?.method?.toUpperCase(); + const method = (error as AxiosError).config?.method?.toUpperCase(); const fallback = fallbackText && fallbackText.length > 0 ? fallbackText @@ -62,8 +65,8 @@ export const showErrorToast = ( // except for principal domain mismatch errors if ( error && - (error.response?.status === ClientErrors.UNAUTHORIZED || - (error.response?.status === ClientErrors.FORBIDDEN && + ((error as AxiosError).response?.status === ClientErrors.UNAUTHORIZED || + ((error as AxiosError).response?.status === ClientErrors.FORBIDDEN && method === 'GET')) && !errorMessage.includes('principal domain') ) {