Skip to content

Commit b83fd5f

Browse files
author
Artem
committed
#RI-3902 rework errors to return
1 parent 84cea2b commit b83fd5f

File tree

6 files changed

+58
-29
lines changed

6 files changed

+58
-29
lines changed

redisinsight/api/src/__mocks__/database-import.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export const mockDatabaseImportResultFail = {
2424
status: DatabaseImportStatus.Fail,
2525
host: mockDatabase.host,
2626
port: mockDatabase.port,
27-
error: new BadRequestException(),
27+
errors: [new BadRequestException()],
2828
};
2929

3030
export const mockDatabaseImportResponse = Object.assign(new DatabaseImportResponse(), {
@@ -34,10 +34,14 @@ export const mockDatabaseImportResponse = Object.assign(new DatabaseImportRespon
3434
index: index + 3,
3535
})),
3636
partial: [],
37-
fail: [new ValidationException([]), new BadRequestException(), new ForbiddenException()].map((error, index) => ({
37+
fail: [
38+
new ValidationException('Bad request'),
39+
new BadRequestException(),
40+
new ForbiddenException(),
41+
].map((error, index) => ({
3842
...mockDatabaseImportResultFail,
3943
index,
40-
error,
44+
errors: [error],
4145
})),
4246
});
4347

redisinsight/api/src/modules/database-import/database-import.analytics.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1+
import { uniq } from 'lodash';
12
import { Injectable } from '@nestjs/common';
23
import { TelemetryBaseService } from 'src/modules/analytics/telemetry.base.service';
34
import { EventEmitter2 } from '@nestjs/event-emitter';
45
import { TelemetryEvents } from 'src/constants';
5-
import { DatabaseImportResponse } from 'src/modules/database-import/dto/database-import.response';
6+
import { DatabaseImportResponse, DatabaseImportResult } from 'src/modules/database-import/dto/database-import.response';
67

78
@Injectable()
89
export class DatabaseImportAnalytics extends TelemetryBaseService {
@@ -25,7 +26,7 @@ export class DatabaseImportAnalytics extends TelemetryBaseService {
2526
TelemetryEvents.DatabaseImportFailed,
2627
{
2728
failed: importResult.fail.length,
28-
errors: importResult.fail.map((res) => (res?.error?.constructor?.name || 'UncaughtError')),
29+
errors: DatabaseImportAnalytics.getUniqueErrorNamesFromResults(importResult.fail),
2930
},
3031
);
3132
}
@@ -35,7 +36,7 @@ export class DatabaseImportAnalytics extends TelemetryBaseService {
3536
TelemetryEvents.DatabaseImportPartiallySucceeded,
3637
{
3738
partially: importResult.partial.length,
38-
errors: importResult.partial.map((res) => (res?.error?.constructor?.name || 'UncaughtError')),
39+
errors: DatabaseImportAnalytics.getUniqueErrorNamesFromResults(importResult.partial),
3940
},
4041
);
4142
}
@@ -49,4 +50,16 @@ export class DatabaseImportAnalytics extends TelemetryBaseService {
4950
},
5051
);
5152
}
53+
54+
static getUniqueErrorNamesFromResults(results: DatabaseImportResult[]) {
55+
return uniq(
56+
[].concat(
57+
...results.map(
58+
(res) => (res?.errors || []).map(
59+
(error) => error?.constructor?.name || 'UncaughtError',
60+
),
61+
),
62+
),
63+
);
64+
}
5265
}

redisinsight/api/src/modules/database-import/database-import.service.spec.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,7 @@ describe('DatabaseImportService', () => {
5959
it('should import databases from json', async () => {
6060
const response = await service.import(mockDatabaseImportFile);
6161

62-
expect(response).toEqual({
63-
...mockDatabaseImportResponse,
64-
errors: undefined, // errors omitted from response
65-
});
62+
expect(response).toEqual(mockDatabaseImportResponse);
6663
expect(analytics.sendImportResults).toHaveBeenCalledWith(mockDatabaseImportResponse);
6764
});
6865

@@ -75,7 +72,6 @@ describe('DatabaseImportService', () => {
7572

7673
expect(response).toEqual({
7774
...mockDatabaseImportResponse,
78-
errors: undefined, // errors omitted from response
7975
});
8076
expect(analytics.sendImportResults).toHaveBeenCalledWith(mockDatabaseImportResponse);
8177
});

redisinsight/api/src/modules/database-import/database-import.service.ts

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
import { HttpException, Injectable, InternalServerErrorException, Logger } from '@nestjs/common';
1+
import {
2+
HttpException, Injectable, InternalServerErrorException, Logger,
3+
} from '@nestjs/common';
24
import { get, isArray, set } from 'lodash';
35
import { Database } from 'src/modules/database/models/database';
46
import { plainToClass } from 'class-transformer';
@@ -163,27 +165,32 @@ export class DatabaseImportService {
163165
port: database.port,
164166
};
165167
} catch (e) {
166-
let error = e;
168+
let errors = [e];
167169
if (isArray(e)) {
168-
[error] = e;
170+
errors = e;
169171
}
170172

171-
if (error instanceof ValidationError) {
172-
error = new ValidationException(Object.values(error?.constraints || {}) || 'Bad request');
173-
}
173+
errors = errors.map((error) => {
174+
if (error instanceof ValidationError) {
175+
const messages = Object.values(error?.constraints || {});
176+
return new ValidationException(messages[messages.length - 1] || 'Bad request');
177+
}
174178

175-
if (!(error instanceof HttpException)) {
176-
error = new InternalServerErrorException(error?.message);
177-
}
179+
if (!(error instanceof HttpException)) {
180+
return new InternalServerErrorException(error?.message);
181+
}
182+
183+
return error;
184+
});
178185

179-
this.logger.warn(`Unable to import database: ${error?.constructor?.name || 'UncaughtError'}`, error);
186+
this.logger.warn(`Unable to import database: ${errors[0]?.constructor?.name || 'UncaughtError'}`, errors[0]);
180187

181188
return {
182189
index,
183190
status: DatabaseImportStatus.Fail,
184191
host: item?.host,
185192
port: item?.port,
186-
error,
193+
errors,
187194
};
188195
}
189196
}

redisinsight/api/src/modules/database-import/dto/database-import.response.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { isArray, isString, isNumber } from 'lodash';
1+
import { isString, isNumber } from 'lodash';
22
import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
33
import { Expose, Transform, Type } from 'class-transformer';
44

@@ -49,13 +49,19 @@ export class DatabaseImportResult {
4949
return undefined;
5050
}
5151

52-
if (e?.response?.message) {
53-
return isArray(e.response.message) ? e.response.message[e.response.message.length - 1] : e.response.message;
54-
}
52+
return e.map((error) => {
53+
if (error?.response) {
54+
return error.response;
55+
}
5556

56-
return e?.message || 'Unhandled Error';
57+
return {
58+
statusCode: 500,
59+
message: error?.message || 'Unhandled Error',
60+
error: 'Unhandled Error',
61+
};
62+
});
5763
}, { toPlainOnly: true })
58-
error?: Error;
64+
errors?: Error[];
5965
}
6066

6167
export class DatabaseImportResponse {

redisinsight/api/test/api/database-import/POST-databases-import.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,10 @@ describe('POST /databases/import', () => {
7373
expect(body.fail.length).to.eq(1);
7474
expect(body.fail[0].status).to.eq('fail');
7575
expect(body.fail[0].index).to.eq(0);
76-
expect(body.fail[0].error).to.be.a('string');
76+
expect(body.fail[0].errors.length).to.eq(1);
77+
expect(body.fail[0].errors[0].message).to.be.a('string');
78+
expect(body.fail[0].errors[0].statusCode).to.eq(400);
79+
expect(body.fail[0].errors[0].error).to.eq('Bad Request');
7780
if (body.fail[0].host) {
7881
expect(body.fail[0].host).to.be.a('string');
7982
}

0 commit comments

Comments
 (0)