#13913: supported test connection api cancellation (#19190)

* supported test connection api cancellation

* minor improvement

* fix the error handling in the intervalAPI calls

* added the useAbortController hook and minor changes

* supported delete workflow if anu api is cancelled

* fix sonar issue and updated the modal button

* fix sonar and playwright issue

* minor fix around test

* reverted the modal textual changes as per comments
This commit is contained in:
Ashish Gupta 2025-01-07 17:36:02 +05:30 committed by GitHub
parent 498d952959
commit a6f81a90fb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 274 additions and 73 deletions

View File

@ -173,6 +173,7 @@ describe('Test Connection Component', () => {
await act(async () => {
render(<TestConnection {...mockProps} />);
});
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(<TestConnection {...mockProps} />);
});
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);

View File

@ -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<TestConnectionProps> = ({
*/
const currentWorkflowRef = useRef(currentWorkflow);
const { controller } = useAbortController();
const serviceType = useMemo(() => {
return getServiceType(serviceCategory);
}, [serviceCategory]);
@ -138,9 +141,12 @@ const TestConnection: FC<TestConnectionProps> = ({
}
};
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<TestConnectionProps> = ({
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<void>((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<TestConnectionProps> = ({
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<TestConnectionProps> = ({
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<TestConnectionProps> = ({
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<TestConnectionProps> = ({
}
};
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<TestConnectionProps> = ({
progress={progress}
testConnectionStep={testConnectionStep}
testConnectionStepResult={testConnectionStepResult}
onCancel={() => setDialogOpen(false)}
onCancel={handleCancelTestConnectionModal}
onConfirm={() => setDialogOpen(false)}
onTestConnection={handleTestConnection}
/>

View File

@ -44,8 +44,8 @@ const TestConnectionModal: FC<TestConnectionModalProps> = ({
isTestingConnection,
testConnectionStep,
testConnectionStepResult,
onCancel,
onConfirm,
onCancel,
isConnectionTimeout,
onTestConnection,
}) => {

View File

@ -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,
})
);
});
});

View File

@ -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;

View File

@ -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<Workflow>
>(`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<Workflow>(
`automations/workflows/${workflowId}`
`automations/workflows/${workflowId}`,
{
signal: apiCancelSignal,
}
);
return response.data;

View File

@ -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<string>) => 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')
) {