Skip to content

RI-7659: Enhance RDI connection testing with per-source error handling #5123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 31, 2025
+139 −30
Merged

RI-7659: Enhance RDI connection testing with per-source error handling #5123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 106 additions & 13 deletions redisinsight/api/src/modules/rdi/client/api.rdi.client.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ const createMockPostImplementation = (
};
};

const axiosError = (status: number, message = 'Request failed') => ({
isAxiosError: true,
message,
status,
});

describe('ApiRdiClient', () => {
let client: ApiRdiClient;

Expand Down Expand Up @@ -432,7 +438,7 @@ describe('ApiRdiClient', () => {
);
});

it('should return targets data even if TestSourcesConnections fails', async () => {
it('should return targets when single source test fails', async () => {
const expectedTargetsResponse = {
targets: { target1: { status: 'success' } },
};
Expand All @@ -453,19 +459,19 @@ describe('ApiRdiClient', () => {

expect(response).toEqual({
targets: expectedTargetsResponse.targets,
sources: {},
sources: {
source1: {
connected: false,
error: 'Failed to test source connection.',
},
},
});

expect(mockedAxios.post).toHaveBeenCalledTimes(2);

expect(loggerErrorSpy).toHaveBeenCalledWith(
'Failed to fetch sources',
expect.any(Error),
);
loggerErrorSpy.mockRestore();
});

it('should return targets data even if TestSourcesConnections fails, asd', async () => {
it('should return targets when one of multiple source tests fails', async () => {
const expectedTargetsResponse = {
targets: { target1: { status: 'success' } },
};
Expand Down Expand Up @@ -496,17 +502,104 @@ describe('ApiRdiClient', () => {
targets: expectedTargetsResponse.targets,
sources: {
source1: { connected: true, error: '' },
source2: {
connected: false,
error: 'Failed to test source connection.',
},
},
});

expect(mockedAxios.post).toHaveBeenCalledTimes(3);

expect(loggerErrorSpy).toHaveBeenCalledWith(
'Failed to fetch sources',
expect.any(Error),
);
loggerErrorSpy.mockRestore();
});

it('should map 405 to the friendly message for a single source', async () => {
const expectedTargetsResponse = {
targets: { target1: { status: 'success' } },
};

mockedAxios.post
.mockResolvedValueOnce({ data: expectedTargetsResponse }) // TestTargetsConnections
.mockRejectedValueOnce(axiosError(405)); // TestSourcesConnections

const response = await client.testConnections({
sources: { source1: {} },
});

expect(response).toEqual({
targets: expectedTargetsResponse.targets,
sources: {
source1: {
connected: false,
error:
'Testing source connections is not supported in your RDI version. Please upgrade to version 1.6.0 or later.',
},
},
});

expect(mockedAxios.post).toHaveBeenCalledTimes(2);
});

it('should map the 405 only to the failing source among multiple sources', async () => {
const expectedTargetsResponse = {
targets: { target1: { status: 'success' } },
};

mockedAxios.post
.mockResolvedValueOnce({ data: expectedTargetsResponse }) // TestTargetsConnections
.mockResolvedValueOnce({ data: { connected: true, error: '' } }) // source1 OK
.mockRejectedValueOnce(axiosError(405)); // source2 405

const response = await client.testConnections({
sources: { source1: {}, source2: {} },
});

expect(response).toEqual({
targets: expectedTargetsResponse.targets,
sources: {
source1: { connected: true, error: '' },
source2: {
connected: false,
error:
'Testing source connections is not supported in your RDI version. Please upgrade to version 1.6.0 or later.',
},
},
});

expect(mockedAxios.post).toHaveBeenCalledTimes(3);
});

it('should handle mixed non-405 and 405 source failures', async () => {
const expectedTargetsResponse = {
targets: { target1: { status: 'success' } },
};

mockedAxios.post
.mockResolvedValueOnce({ data: expectedTargetsResponse }) // targets
.mockRejectedValueOnce(axiosError(500, 'Internal error')) // source1 500
.mockRejectedValueOnce(axiosError(405)); // source2 405

const response = await client.testConnections({
sources: { source1: {}, source2: {} },
});

expect(response).toEqual({
targets: expectedTargetsResponse.targets,
sources: {
source1: {
connected: false,
error: 'Failed to test source connection.',
},
source2: {
connected: false,
error:
'Testing source connections is not supported in your RDI version. Please upgrade to version 1.6.0 or later.',
},
},
});

expect(mockedAxios.post).toHaveBeenCalledTimes(3);
});
});

describe('getPipelineStatus', () => {
Expand Down
50 changes: 33 additions & 17 deletions redisinsight/api/src/modules/rdi/client/api.rdi.client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,25 +225,41 @@ export class ApiRdiClient extends RdiClient {
throw wrapRdiPipelineError(error);
}

try {
const sourceConfigs = Object.keys(config.sources || {});

if (sourceConfigs.length) {
await Promise.all(
sourceConfigs.map(async (source) => {
const response = await this.client.post(
RdiUrl.TestSourcesConnections,
{ ...config.sources[source] },
);
sources[source] = response.data;
}),
);
}
} catch (error) {
// failing is expected on RDI version below 1.6.0 (1.4.3 for example)
this.logger.error('Failed to fetch sources', error);
const sourceConfigs = Object.keys(config.sources || {});

if (sourceConfigs.length === 0) {
return { targets, sources };
}

await Promise.all(
sourceConfigs.map(async (source) => {
try {
const response = await this.client.post(
RdiUrl.TestSourcesConnections,
{ ...config.sources[source] },
);
sources[source] = response.data;
} catch (error: any) {
// Older versions of RDI (below 1.6.0) don't support testing sources connections
// RDI returns 405 Method Not Allowed for non existing endpoints
const status = error?.status;
if (status === 405) {
sources[source] = {
connected: false,
error:
'Testing source connections is not supported in your RDI version. Please upgrade to version 1.6.0 or later.',
Copy link
Collaborator

@ArtemHoruzhenko ArtemHoruzhenko Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we promote latest version instead?

Copy link
Collaborator Author

@pawelangelow pawelangelow Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll merge it this way, as I showed the texts to @ViktarStarastsenka. Honestly, I don't expect this message to be seen that much, as the RDI team is pushing for a lot of releases.

};
} else {
// Something went wrong with testing source connection
sources[source] = {
connected: false,
error: 'Failed to test source connection.',
};
}
}
}),
);

return { targets, sources };
}

Expand Down
Loading