Skip to content

Commit 6723ccb

Browse files
mariasergeenkomariasergeenko
authored
Merge pull request #3694 from RedisInsight/feature/RI-5979-display-detailed-error-messages
RI 5979 show detailed message instead of common unauthorized
2 parents 36d75af + e360f90 commit 6723ccb

20 files changed

+168
-84
lines changed

‎redisinsight/api/src/constants/custom-error-codes.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,5 @@ export enum CustomErrorCodes {
6060
RdiInternalServerError = 11_403,
6161
RdiValidationError = 11_404,
6262
RdiNotFound = 11_405,
63+
RdiForbidden = 11_406,
6364
}

‎redisinsight/api/src/modules/rdi/client/api.rdi.client.spec.ts‎

Lines changed: 28 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import axios, { AxiosError } from 'axios';
1+
import axios from 'axios';
22
import {
33
mockRdi,
44
mockRdiClientMetadata,
@@ -7,13 +7,12 @@ import {
77
mockRdiPipeline,
88
mockRdiSchema,
99
mockRdiStatisticsData,
10+
mockRdiUnauthorizedError,
1011
} from 'src/__mocks__';
11-
import errorMessages from 'src/constants/error-messages';
1212
import { sign } from 'jsonwebtoken';
1313
import { ApiRdiClient } from './api.rdi.client';
1414
import { RdiDyRunJobStatus, RdiPipeline, RdiStatisticsStatus } from '../models';
1515
import { RdiUrl, TOKEN_THRESHOLD } from '../constants';
16-
import { wrapRdiPipelineError } from '../exceptions';
1716

1817
const mockedAxios = axios as jest.Mocked<typeof axios>;
1918
jest.mock('axios');
@@ -40,17 +39,10 @@ describe('ApiRdiClient', () => {
4039
});
4140

4241
it('should throw error if request fails', async () => {
43-
const error = {
44-
message: errorMessages.UNAUTHORIZED,
45-
response: {
46-
status: 401,
47-
},
48-
};
49-
5042
mockedAxios.get.mockResolvedValueOnce({ data: mockRdiConfigSchema });
51-
mockedAxios.get.mockRejectedValueOnce(error);
43+
mockedAxios.get.mockRejectedValueOnce(mockRdiUnauthorizedError);
5244

53-
await expect(client.getSchema()).rejects.toThrow(errorMessages.UNAUTHORIZED);
45+
await expect(client.getSchema()).rejects.toThrow(mockRdiUnauthorizedError.message);
5446
});
5547
});
5648

@@ -70,10 +62,9 @@ describe('ApiRdiClient', () => {
7062
});
7163

7264
it('should throw error if request fails', async () => {
73-
const error = new Error('test error');
74-
mockedAxios.get.mockRejectedValueOnce(error);
65+
mockedAxios.get.mockRejectedValueOnce(mockRdiUnauthorizedError);
7566

76-
await expect(client.getPipeline()).rejects.toThrow(errorMessages.INTERNAL_SERVER_ERROR);
67+
await expect(client.getPipeline()).rejects.toThrow(mockRdiUnauthorizedError.message);
7768
});
7869
});
7970

@@ -89,10 +80,9 @@ describe('ApiRdiClient', () => {
8980
});
9081

9182
it('should throw an error when API call fails', async () => {
92-
const mockError = new Error('API call failed');
93-
mockedAxios.get.mockRejectedValueOnce(mockError);
83+
mockedAxios.get.mockRejectedValueOnce(mockRdiUnauthorizedError);
9484

95-
await expect(client.getStrategies()).rejects.toThrow(errorMessages.INTERNAL_SERVER_ERROR);
85+
await expect(client.getStrategies()).rejects.toThrow(mockRdiUnauthorizedError.message);
9686
expect(axios.get).toHaveBeenCalledWith(RdiUrl.GetStrategies);
9787
});
9888
});
@@ -112,11 +102,10 @@ describe('ApiRdiClient', () => {
112102
});
113103

114104
it('should throw an error when the API call fails', async () => {
115-
const expectedError = new Error('API call failed');
116-
mockedAxios.get.mockRejectedValueOnce(expectedError);
105+
mockedAxios.get.mockRejectedValueOnce(mockRdiUnauthorizedError);
117106

118107
await expect(client.getConfigTemplate(pipelineType, dbType))
119-
.rejects.toThrowError(errorMessages.INTERNAL_SERVER_ERROR);
108+
.rejects.toThrowError(mockRdiUnauthorizedError.message);
120109
});
121110
});
122111

@@ -141,10 +130,9 @@ describe('ApiRdiClient', () => {
141130
});
142131

143132
it('should throw an error when the API call fails', async () => {
144-
const expectedError = new Error('API call failed');
145-
mockedAxios.get.mockRejectedValueOnce(expectedError);
133+
mockedAxios.get.mockRejectedValueOnce(mockRdiUnauthorizedError);
146134

147-
await expect(client.getJobTemplate(pipelineType)).rejects.toThrowError(errorMessages.INTERNAL_SERVER_ERROR);
135+
await expect(client.getJobTemplate(pipelineType)).rejects.toThrowError(mockRdiUnauthorizedError.message);
148136
});
149137
});
150138

@@ -170,11 +158,9 @@ describe('ApiRdiClient', () => {
170158
});
171159

172160
it('should throw an error if the deployment fails', async () => {
173-
const errorMessage = 'Deployment failed';
174-
const errorResponse = { response: { data: { message: errorMessage } } };
175-
mockedAxios.post.mockRejectedValueOnce(errorResponse);
161+
mockedAxios.post.mockRejectedValueOnce(mockRdiUnauthorizedError);
176162

177-
await expect(client.deploy(mockRdiPipeline)).rejects.toThrow(errorMessage);
163+
await expect(client.deploy(mockRdiPipeline)).rejects.toThrow(mockRdiUnauthorizedError.message);
178164
});
179165
});
180166

@@ -197,10 +183,9 @@ describe('ApiRdiClient', () => {
197183
});
198184

199185
it('should throw an error if the client call fails', async () => {
200-
const mockError = new Error('mock error');
201-
mockedAxios.post.mockRejectedValueOnce(mockError);
186+
mockedAxios.post.mockRejectedValueOnce(mockRdiUnauthorizedError);
202187

203-
await expect(client.dryRunJob(mockRdiDryRunJob)).rejects.toThrow(errorMessages.INTERNAL_SERVER_ERROR);
188+
await expect(client.dryRunJob(mockRdiDryRunJob)).rejects.toThrow(mockRdiUnauthorizedError.message);
204189
});
205190
});
206191

@@ -219,10 +204,9 @@ describe('ApiRdiClient', () => {
219204

220205
it('should throw an error if the request fails', async () => {
221206
const config = {};
222-
const error = new Error('Test error');
223-
mockedAxios.post.mockRejectedValueOnce(error);
207+
mockedAxios.post.mockRejectedValueOnce(mockRdiUnauthorizedError);
224208

225-
await expect(client.testConnections(config)).rejects.toThrow(errorMessages.INTERNAL_SERVER_ERROR);
209+
await expect(client.testConnections(config)).rejects.toThrow(mockRdiUnauthorizedError.message);
226210
expect(mockedAxios.post).toHaveBeenCalledWith(RdiUrl.TestConnections, config);
227211
});
228212
});
@@ -239,10 +223,9 @@ describe('ApiRdiClient', () => {
239223
});
240224

241225
it('should throw an error when API call fails', async () => {
242-
const mockError = new Error('API call failed');
243-
mockedAxios.get.mockRejectedValue(mockError);
226+
mockedAxios.get.mockRejectedValue(mockRdiUnauthorizedError);
244227

245-
await expect(client.getPipelineStatus()).rejects.toThrow(errorMessages.INTERNAL_SERVER_ERROR);
228+
await expect(client.getPipelineStatus()).rejects.toThrow(mockRdiUnauthorizedError.message);
246229
expect(mockedAxios.get).toHaveBeenCalledWith(RdiUrl.GetPipelineStatus);
247230
});
248231
});
@@ -262,14 +245,13 @@ describe('ApiRdiClient', () => {
262245
});
263246

264247
it('should return fail status and error message when API call fails', async () => {
265-
const errorMessage = 'API call failed';
266-
mockedAxios.get.mockRejectedValue(new Error(errorMessage));
248+
mockedAxios.get.mockRejectedValue(mockRdiUnauthorizedError);
267249

268250
const result = await client.getStatistics();
269251

270252
expect(mockedAxios.get).toHaveBeenCalledWith(RdiUrl.GetStatistics, { params: { } });
271253
expect(result.status).toBe(RdiStatisticsStatus.Fail);
272-
expect(result.error).toBe(errorMessage);
254+
expect(result.error).toBe(mockRdiUnauthorizedError.message);
273255
});
274256
});
275257

@@ -285,9 +267,9 @@ describe('ApiRdiClient', () => {
285267
});
286268

287269
it('should throw an error if the API call fails', async () => {
288-
mockedAxios.get.mockRejectedValueOnce(new Error('API error'));
270+
mockedAxios.get.mockRejectedValueOnce(mockRdiUnauthorizedError);
289271

290-
await expect(client.getJobFunctions()).rejects.toThrow(errorMessages.INTERNAL_SERVER_ERROR);
272+
await expect(client.getJobFunctions()).rejects.toThrow(mockRdiUnauthorizedError.message);
291273
});
292274
});
293275

@@ -313,10 +295,9 @@ describe('ApiRdiClient', () => {
313295
});
314296

315297
it('should throw an error if login fails', async () => {
316-
const error = new AxiosError('Login failed');
317-
mockedAxios.post.mockRejectedValueOnce(error);
298+
mockedAxios.post.mockRejectedValueOnce(mockRdiUnauthorizedError);
318299

319-
await expect(client.connect()).rejects.toThrow(wrapRdiPipelineError(error));
300+
await expect(client.connect()).rejects.toThrow(mockRdiUnauthorizedError.message);
320301
});
321302
});
322303

@@ -346,7 +327,6 @@ describe('ApiRdiClient', () => {
346327

347328
describe('pollActionStatus', () => {
348329
const responseData = 'some data';
349-
const error = new Error('Test error');
350330
const actionId = 'test-action-id';
351331

352332
it('should return response data on success', async () => {
@@ -365,9 +345,9 @@ describe('ApiRdiClient', () => {
365345
});
366346

367347
it('should throw an error if an error occurs during polling', async () => {
368-
mockedAxios.get.mockRejectedValueOnce(error);
348+
mockedAxios.get.mockRejectedValueOnce(mockRdiUnauthorizedError);
369349

370-
await expect(client['pollActionStatus'](actionId)).rejects.toThrow(errorMessages.INTERNAL_SERVER_ERROR);
350+
await expect(client['pollActionStatus'](actionId)).rejects.toThrow(mockRdiUnauthorizedError.message);
371351
expect(mockedAxios.get).toHaveBeenCalledWith(`${RdiUrl.Action}/${actionId}`, { signal: undefined });
372352
});
373353
});

‎redisinsight/api/src/modules/rdi/client/api.rdi.client.ts‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
import {
2121
RdiPipelineDeployFailedException,
2222
RdiPipelineInternalServerErrorException,
23+
parseErrorMessage,
2324
wrapRdiPipelineError,
2425
} from 'src/modules/rdi/exceptions';
2526
import {
@@ -89,17 +90,17 @@ export class ApiRdiClient extends RdiClient {
8990
try {
9091
const response = await this.client.get(`${RdiUrl.GetConfigTemplate}/${pipelineType}/${dbType}`);
9192
return response.data;
92-
} catch (error) {
93-
throw wrapRdiPipelineError(error);
93+
} catch (e) {
94+
throw wrapRdiPipelineError(e);
9495
}
9596
}
9697

9798
async getJobTemplate(pipelineType: string): Promise<RdiTemplateResponseDto> {
9899
try {
99100
const response = await this.client.get(`${RdiUrl.GetJobTemplate}/${pipelineType}`);
100101
return response.data;
101-
} catch (error) {
102-
throw wrapRdiPipelineError(error);
102+
} catch (e) {
103+
throw wrapRdiPipelineError(e);
103104
}
104105
}
105106

@@ -113,8 +114,8 @@ export class ApiRdiClient extends RdiClient {
113114
const actionId = response.data.action_id;
114115

115116
return await this.pollActionStatus(actionId);
116-
} catch (error) {
117-
throw wrapRdiPipelineError(error, error?.response?.data?.message);
117+
} catch (e) {
118+
throw wrapRdiPipelineError(e);
118119
}
119120
}
120121

@@ -160,7 +161,8 @@ export class ApiRdiClient extends RdiClient {
160161
data: plainToClass(RdiStatisticsData, convertKeysToCamelCase(data)),
161162
};
162163
} catch (e) {
163-
return { status: RdiStatisticsStatus.Fail, error: e.message };
164+
const message: string = parseErrorMessage(e);
165+
return { status: RdiStatisticsStatus.Fail, error: message };
164166
}
165167
}
166168

‎redisinsight/api/src/modules/rdi/exceptions/rdi-pipeline.forbidden.exception.spec.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import { HttpStatus } from '@nestjs/common';
2+
import errorMessages from 'src/constants/error-messages';
3+
import { CustomErrorCodes } from 'src/constants';
4+
import { RdiPipelineForbiddenException } from './rdi-pipeline.forbidden.exception';
5+
6+
describe('RdiPipelineForbiddenException', () => {
7+
it('should create a RdiPipelineForbiddenException with default message and status code', () => {
8+
const exception = new RdiPipelineForbiddenException();
9+
expect(exception.getStatus()).toBe(HttpStatus.FORBIDDEN);
10+
expect(exception.getResponse()).toEqual({
11+
statusCode: HttpStatus.FORBIDDEN,
12+
message: errorMessages.FORBIDDEN,
13+
error: 'RdiForbidden',
14+
errorCode: CustomErrorCodes.RdiForbidden,
15+
});
16+
});
17+
18+
it('should create a RdiPipelineForbiddenException with custom message and status code', () => {
19+
const customMessage = 'Custom forbidden message';
20+
const exception = new RdiPipelineForbiddenException(customMessage);
21+
expect(exception.getStatus()).toBe(HttpStatus.FORBIDDEN);
22+
expect(exception.getResponse()).toEqual({
23+
statusCode: HttpStatus.FORBIDDEN,
24+
message: customMessage,
25+
error: 'RdiForbidden',
26+
errorCode: CustomErrorCodes.RdiForbidden,
27+
});
28+
});
29+
});

‎redisinsight/api/src/modules/rdi/exceptions/rdi-pipeline.forbidden.exception.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { HttpException, HttpExceptionOptions, HttpStatus } from '@nestjs/common';
2+
import { CustomErrorCodes } from 'src/constants';
3+
import ERROR_MESSAGES from 'src/constants/error-messages';
4+
5+
export class RdiPipelineForbiddenException extends HttpException {
6+
constructor(
7+
message = ERROR_MESSAGES.FORBIDDEN,
8+
options?: HttpExceptionOptions & { details?: unknown },
9+
) {
10+
const response = {
11+
message,
12+
statusCode: HttpStatus.FORBIDDEN,
13+
error: 'RdiForbidden',
14+
errorCode: CustomErrorCodes.RdiForbidden,
15+
};
16+
17+
super(response, response.statusCode, options);
18+
}
19+
}

‎redisinsight/api/src/modules/rdi/exceptions/rdi-pipiline.error.handler.spec.ts‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
import { CustomErrorCodes } from 'src/constants';
1010
import errorMessages from 'src/constants/error-messages';
1111
import { wrapRdiPipelineError } from './rdi-pipiline.error.handler';
12+
import { RdiPipelineForbiddenException } from './rdi-pipeline.forbidden.exception';
1213

1314
describe('wrapRdiPipelineError', () => {
1415
it('should return the original error if it is an instance of HttpException', () => {
@@ -28,10 +29,9 @@ describe('wrapRdiPipelineError', () => {
2829
},
2930
},
3031
} as AxiosError;
31-
const result = wrapRdiPipelineError(error, 'Test error');
32+
const result = wrapRdiPipelineError(error);
3233

3334
expect(result).toBeInstanceOf(RdiPipelineUnauthorizedException);
34-
expect(result.message).toBe('Test error');
3535
expect(result.getResponse()).toEqual({
3636
message: result.message,
3737
statusCode: HttpStatus.UNAUTHORIZED,
@@ -54,7 +54,29 @@ describe('wrapRdiPipelineError', () => {
5454
const result = wrapRdiPipelineError(error);
5555

5656
expect(result).toBeInstanceOf(RdiPipelineUnauthorizedException);
57-
expect(result.message).toBe(errorMessages.UNAUTHORIZED);
57+
expect(result.message).toBe('Unauthorized');
58+
});
59+
60+
it('should return a RdiPipelineForbiddenException if the response status is 403', () => {
61+
const error = {
62+
response: {
63+
status: 403,
64+
data: {
65+
detail: {
66+
message: 'Unauthorized',
67+
},
68+
},
69+
},
70+
} as AxiosError;
71+
const result = wrapRdiPipelineError(error);
72+
73+
expect(result).toBeInstanceOf(RdiPipelineForbiddenException);
74+
expect(result.getResponse()).toEqual({
75+
message: result.message,
76+
statusCode: HttpStatus.FORBIDDEN,
77+
error: 'RdiForbidden',
78+
errorCode: CustomErrorCodes.RdiForbidden,
79+
});
5880
});
5981

6082
it('should return a RdiPipelineValidationException if the response status is 422', () => {
@@ -71,9 +93,8 @@ describe('wrapRdiPipelineError', () => {
7193
},
7294
},
7395
} as AxiosError;
74-
const result = wrapRdiPipelineError(error, 'Test error');
96+
const result = wrapRdiPipelineError(error);
7597
expect(result).toBeInstanceOf(RdiPipelineValidationException);
76-
expect(result.message).toBe('Test error');
7798
expect(result.getResponse()).toEqual({
7899
message: result.message,
79100
statusCode: HttpStatus.UNPROCESSABLE_ENTITY,
@@ -116,9 +137,8 @@ describe('wrapRdiPipelineError', () => {
116137
},
117138
},
118139
} as AxiosError;
119-
const result = wrapRdiPipelineError(error, 'Test error');
140+
const result = wrapRdiPipelineError(error);
120141
expect(result).toBeInstanceOf(RdiPipelineNotFoundException);
121-
expect(result.message).toBe('Test error');
122142
expect(result.getResponse()).toEqual({
123143
message: result.message,
124144
statusCode: HttpStatus.NOT_FOUND,

0 commit comments

Comments
 (0)