Skip to content

Commit b874200

Browse files
fix retries
1 parent b40db05 commit b874200

16 files changed

+290
-137
lines changed

redisinsight/api/src/__mocks__/cloud-capi-key.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,4 +69,6 @@ export const mockCloudCapiKeyService = jest.fn(() => ({
6969
export const mockCloudCapiKeyAnalytics = jest.fn(() => ({
7070
sendCloudAccountKeyGenerated: jest.fn(),
7171
sendCloudAccountKeyGenerationFailed: jest.fn(),
72+
sendCloudAccountSecretGenerated: jest.fn(),
73+
sendCloudAccountSecretGenerationFailed: jest.fn(),
7274
}));
Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,46 @@
1+
import { HttpStatus } from '@nestjs/common';
2+
import ERROR_MESSAGES from 'src/constants/error-messages';
3+
import { CustomErrorCodes } from 'src/constants';
4+
15
export const mockCapiUnauthorizedError = {
26
message: 'Request failed with status code 401',
37
response: {
48
status: 401,
59
},
610
};
711

8-
export const mockApiInternalServerError = {
12+
export const mockSmApiUnauthorizedError = mockCapiUnauthorizedError;
13+
14+
export const mockSmApiInternalServerError = {
915
message: 'Something wrong',
1016
response: {
1117
status: 500,
1218
},
1319
};
1420

21+
export const mockSmApiBadRequestError = {
22+
message: 'Bad Request',
23+
response: {
24+
status: 400,
25+
},
26+
};
27+
1528
export const mockUtm = {
1629
source: 'redisinsight',
1730
medium: 'sso',
1831
campaign: 'workbench',
1932
};
33+
34+
export const mockCloudApiUnauthorizedExceptionResponse = {
35+
error: 'CloudApiUnauthorized',
36+
errorCode: CustomErrorCodes.CloudApiUnauthorized,
37+
message: ERROR_MESSAGES.UNAUTHORIZED,
38+
statusCode: HttpStatus.UNAUTHORIZED,
39+
};
40+
41+
export const mockCloudApiBadRequestExceptionResponse = {
42+
error: 'CloudApiBadRequest',
43+
errorCode: CustomErrorCodes.CloudApiBadRequest,
44+
message: ERROR_MESSAGES.BAD_REQUEST,
45+
statusCode: HttpStatus.BAD_REQUEST,
46+
};

redisinsight/api/src/__mocks__/cloud-user.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ export const mockCloudUserRepository = jest.fn(() => ({
136136
export const mockCloudUserApiService = jest.fn(() => ({
137137
getCapiKeys: jest.fn().mockResolvedValue(mockCloudCapiAuthDto),
138138
me: jest.fn().mockResolvedValue(mockCloudUser),
139+
getCloudUser: jest.fn().mockResolvedValue(mockCloudUser),
139140
setCurrentAccount: jest.fn(),
140141
updateUser: jest.fn(),
141142
}));

redisinsight/api/src/constants/telemetry-events.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ export enum TelemetryEvents {
3939
// Event for cloud CAPI keys
4040
CloudAccountKeyGenerated = 'CLOUD_ACCOUNT_KEY_GENERATED',
4141
CloudAccountKeyGenerationFailed = 'CLOUD_ACCOUNT_KEY_GENERATION_FAILED',
42+
CloudAccountSecretGenerated = 'CLOUD_ACCOUNT_SECRET_GENERATED',
43+
CloudAccountSecretGenerationFailed = 'CLOUD_ACCOUNT_SECRET_GENERATION_FAILED',
4244

4345
// Events for cli tool
4446
CliClientCreated = 'CLI_CLIENT_CREATED',

redisinsight/api/src/modules/ai/query/exceptions/ai-query.bad-request.exception.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export class AiQueryBadRequestException extends HttpException {
88
message,
99
statusCode: HttpStatus.BAD_REQUEST,
1010
error: 'AiQueryBadRequest',
11-
errorCode: CustomErrorCodes.QueryAiInternalServerError,
11+
errorCode: CustomErrorCodes.QueryAiBadRequest,
1212
};
1313

1414
super(response, response.statusCode, options);

redisinsight/api/src/modules/cloud/autodiscovery/me.cloud-autodiscovery.service.spec.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {
44
mockCloudAutodiscoveryService,
55
mockCloudCapiAuthDto, mockCloudCapiKeyService,
66
mockCloudDatabase,
7-
mockCloudDatabaseFixed,
7+
mockCloudDatabaseFixed, mockCloudSessionService,
88
mockCloudSubscription,
99
mockCloudSubscriptionFixed,
1010
mockImportCloudDatabaseDto, mockImportCloudDatabaseDtoFixed,
@@ -22,6 +22,8 @@ import {
2222
} from 'src/modules/cloud/common/exceptions';
2323
import { MeCloudAutodiscoveryService } from 'src/modules/cloud/autodiscovery/me.cloud-autodiscovery.service';
2424
import { CloudCapiKeyService } from 'src/modules/cloud/capi-key/cloud-capi-key.service';
25+
import { CloudCapiKeyApiProvider } from 'src/modules/cloud/capi-key/cloud-capi-key.api.provider';
26+
import { CloudSessionService } from 'src/modules/cloud/session/cloud-session.service';
2527

2628
describe('MeCloudAutodiscoveryService', () => {
2729
let service: MeCloudAutodiscoveryService;
@@ -33,6 +35,7 @@ describe('MeCloudAutodiscoveryService', () => {
3335
const module: TestingModule = await Test.createTestingModule({
3436
providers: [
3537
MeCloudAutodiscoveryService,
38+
CloudCapiKeyApiProvider,
3639
{
3740
provide: CloudAutodiscoveryService,
3841
useFactory: mockCloudAutodiscoveryService,
@@ -41,6 +44,10 @@ describe('MeCloudAutodiscoveryService', () => {
4144
provide: CloudCapiKeyService,
4245
useFactory: mockCloudCapiKeyService,
4346
},
47+
{
48+
provide: CloudSessionService,
49+
useFactory: mockCloudSessionService,
50+
},
4451
],
4552
}).compile();
4653

@@ -53,7 +60,8 @@ describe('MeCloudAutodiscoveryService', () => {
5360
it('successfully get cloud account info', async () => {
5461
expect(await service.getAccount(mockSessionMetadata)).toEqual(mockCloudAccountInfo);
5562
});
56-
it('should throw CloudApiUnauthorizedException exception', async () => {
63+
it('should throw CloudApiUnauthorizedException exception if failed twice', async () => {
64+
cloudCapiKeyService.getCapiCredentials.mockRejectedValueOnce(new CloudApiUnauthorizedException());
5765
cloudCapiKeyService.getCapiCredentials.mockRejectedValueOnce(new CloudApiUnauthorizedException());
5866
await expect(service.getAccount(mockSessionMetadata)).rejects.toThrow(
5967
CloudApiUnauthorizedException,
@@ -76,7 +84,8 @@ describe('MeCloudAutodiscoveryService', () => {
7684
CloudAutodiscoveryAuthType.Sso,
7785
);
7886
});
79-
it('should throw CloudApiUnauthorizedException exception', async () => {
87+
it('should throw CloudApiUnauthorizedException exception if failed twice', async () => {
88+
cloudCapiKeyService.getCapiCredentials.mockRejectedValueOnce(new CloudApiUnauthorizedException());
8089
cloudCapiKeyService.getCapiCredentials.mockRejectedValueOnce(new CloudApiUnauthorizedException());
8190
await expect(service.discoverSubscriptions(mockSessionMetadata)).rejects.toThrow(
8291
CloudApiUnauthorizedException,
@@ -128,7 +137,8 @@ describe('MeCloudAutodiscoveryService', () => {
128137
CloudAutodiscoveryAuthType.Sso,
129138
);
130139
});
131-
it('should throw CloudApiUnauthorizedException exception', async () => {
140+
it('should throw CloudApiUnauthorizedException exception if failed twice', async () => {
141+
cloudCapiKeyService.getCapiCredentials.mockRejectedValueOnce(new CloudApiUnauthorizedException());
132142
cloudCapiKeyService.getCapiCredentials.mockRejectedValueOnce(new CloudApiUnauthorizedException());
133143
await expect(service.discoverDatabases(
134144
mockSessionMetadata,
@@ -183,7 +193,8 @@ describe('MeCloudAutodiscoveryService', () => {
183193
],
184194
);
185195
});
186-
it('should throw CloudApiUnauthorizedException exception', async () => {
196+
it('should throw CloudApiUnauthorizedException exception if failed twice', async () => {
197+
cloudCapiKeyService.getCapiCredentials.mockRejectedValueOnce(new CloudApiUnauthorizedException());
187198
cloudCapiKeyService.getCapiCredentials.mockRejectedValueOnce(new CloudApiUnauthorizedException());
188199
await expect(service.addRedisCloudDatabases(
189200
mockSessionMetadata,

redisinsight/api/src/modules/cloud/autodiscovery/me.cloud-autodiscovery.service.ts

Lines changed: 42 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@ import { CloudDatabase } from 'src/modules/cloud/database/models';
1616
import { CloudAutodiscoveryService } from 'src/modules/cloud/autodiscovery/cloud-autodiscovery.service';
1717
import { CloudRequestUtm } from 'src/modules/cloud/common/models';
1818
import { CloudCapiKeyService } from 'src/modules/cloud/capi-key/cloud-capi-key.service';
19+
import { CloudCapiKeyApiProvider } from 'src/modules/cloud/capi-key/cloud-capi-key.api.provider';
1920

2021
@Injectable()
2122
export class MeCloudAutodiscoveryService {
2223
constructor(
2324
private readonly cloudAutodiscoveryService: CloudAutodiscoveryService,
2425
private readonly cloudCapiKeyService: CloudCapiKeyService,
26+
private readonly api: CloudCapiKeyApiProvider,
2527
) {}
2628

2729
private async getCapiCredentials(sessionMetadata: SessionMetadata, utm?: CloudRequestUtm): Promise<CloudCapiAuthDto> {
@@ -34,13 +36,15 @@ export class MeCloudAutodiscoveryService {
3436
* @param utm
3537
*/
3638
async getAccount(sessionMetadata: SessionMetadata, utm?: CloudRequestUtm): Promise<CloudAccountInfo> {
37-
try {
38-
return await this.cloudAutodiscoveryService.getAccount(
39-
await this.getCapiCredentials(sessionMetadata, utm),
40-
);
41-
} catch (e) {
42-
throw wrapHttpError(await this.cloudCapiKeyService.handleCapiKeyUnauthorizedError(e, sessionMetadata));
43-
}
39+
return this.api.callWithAuthRetry(sessionMetadata.sessionId, async () => {
40+
try {
41+
return await this.cloudAutodiscoveryService.getAccount(
42+
await this.getCapiCredentials(sessionMetadata, utm),
43+
);
44+
} catch (e) {
45+
throw wrapHttpError(await this.cloudCapiKeyService.handleCapiKeyUnauthorizedError(e, sessionMetadata));
46+
}
47+
});
4448
}
4549

4650
/**
@@ -49,14 +53,16 @@ export class MeCloudAutodiscoveryService {
4953
* @param utm
5054
*/
5155
async discoverSubscriptions(sessionMetadata: SessionMetadata, utm?: CloudRequestUtm): Promise<CloudSubscription[]> {
52-
try {
53-
return await this.cloudAutodiscoveryService.discoverSubscriptions(
54-
await this.getCapiCredentials(sessionMetadata, utm),
55-
CloudAutodiscoveryAuthType.Sso,
56-
);
57-
} catch (e) {
58-
throw wrapHttpError(await this.cloudCapiKeyService.handleCapiKeyUnauthorizedError(e, sessionMetadata));
59-
}
56+
return this.api.callWithAuthRetry(sessionMetadata.sessionId, async () => {
57+
try {
58+
return await this.cloudAutodiscoveryService.discoverSubscriptions(
59+
await this.getCapiCredentials(sessionMetadata, utm),
60+
CloudAutodiscoveryAuthType.Sso,
61+
);
62+
} catch (e) {
63+
throw wrapHttpError(await this.cloudCapiKeyService.handleCapiKeyUnauthorizedError(e, sessionMetadata));
64+
}
65+
});
6066
}
6167

6268
/**
@@ -70,15 +76,17 @@ export class MeCloudAutodiscoveryService {
7076
dto: DiscoverCloudDatabasesDto,
7177
utm?: CloudRequestUtm,
7278
): Promise<CloudDatabase[]> {
73-
try {
74-
return await this.cloudAutodiscoveryService.discoverDatabases(
75-
await this.getCapiCredentials(sessionMetadata, utm),
76-
dto,
77-
CloudAutodiscoveryAuthType.Sso,
78-
);
79-
} catch (e) {
80-
throw wrapHttpError(await this.cloudCapiKeyService.handleCapiKeyUnauthorizedError(e, sessionMetadata));
81-
}
79+
return this.api.callWithAuthRetry(sessionMetadata.sessionId, async () => {
80+
try {
81+
return await this.cloudAutodiscoveryService.discoverDatabases(
82+
await this.getCapiCredentials(sessionMetadata, utm),
83+
dto,
84+
CloudAutodiscoveryAuthType.Sso,
85+
);
86+
} catch (e) {
87+
throw wrapHttpError(await this.cloudCapiKeyService.handleCapiKeyUnauthorizedError(e, sessionMetadata));
88+
}
89+
});
8290
}
8391

8492
/**
@@ -92,13 +100,15 @@ export class MeCloudAutodiscoveryService {
92100
addDatabasesDto: ImportCloudDatabaseDto[],
93101
utm?: CloudRequestUtm,
94102
): Promise<ImportCloudDatabaseResponse[]> {
95-
try {
96-
return await this.cloudAutodiscoveryService.addRedisCloudDatabases(
97-
await this.getCapiCredentials(sessionMetadata, utm),
98-
addDatabasesDto,
99-
);
100-
} catch (e) {
101-
throw wrapHttpError(await this.cloudCapiKeyService.handleCapiKeyUnauthorizedError(e, sessionMetadata));
102-
}
103+
return this.api.callWithAuthRetry(sessionMetadata.sessionId, async () => {
104+
try {
105+
return await this.cloudAutodiscoveryService.addRedisCloudDatabases(
106+
await this.getCapiCredentials(sessionMetadata, utm),
107+
addDatabasesDto,
108+
);
109+
} catch (e) {
110+
throw wrapHttpError(await this.cloudCapiKeyService.handleCapiKeyUnauthorizedError(e, sessionMetadata));
111+
}
112+
});
103113
}
104114
}

redisinsight/api/src/modules/cloud/capi-key/cloud-capi-key.analytics.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,21 @@ export class CloudCapiKeyAnalytics extends TelemetryBaseService {
2525
exception,
2626
);
2727
}
28+
29+
sendCloudAccountSecretGenerated() {
30+
try {
31+
this.sendEvent(
32+
TelemetryEvents.CloudAccountSecretGenerated,
33+
);
34+
} catch (e) {
35+
// continue regardless of error
36+
}
37+
}
38+
39+
sendCloudAccountSecretGenerationFailed(exception: HttpException) {
40+
this.sendFailedEvent(
41+
TelemetryEvents.CloudAccountSecretGenerationFailed,
42+
exception,
43+
);
44+
}
2845
}

redisinsight/api/src/modules/cloud/capi-key/cloud-capi-key.module.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ import { CloudCapiKeyAnalytics } from 'src/modules/cloud/capi-key/cloud-capi-key
2323
useClass: LocalCloudCapiKeyRepository,
2424
},
2525
],
26-
exports: [CloudCapiKeyService],
26+
exports: [
27+
CloudCapiKeyService,
28+
CloudCapiKeyApiProvider,
29+
],
2730
})
2831
export class CloudCapiKeyModule {}

redisinsight/api/src/modules/cloud/capi-key/cloud-capi-key.service.spec.ts

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ describe('CloudCapiKeyService', () => {
116116
});
117117
it('Should generate new capi key but enable CAPI before', async () => {
118118
repository.getByUserAccount.mockResolvedValueOnce(null);
119-
cloudUserApiService.me.mockResolvedValueOnce({
119+
cloudUserApiService.getCloudUser.mockResolvedValueOnce({
120120
...mockCloudUser,
121121
capiKey: undefined,
122122
accounts: [{
@@ -133,17 +133,6 @@ describe('CloudCapiKeyService', () => {
133133
expect(mockedAxios.post)
134134
.toHaveBeenNthCalledWith(2, '/accounts/cloud-api/cloudApiKeys', expect.anything(), expect.anything());
135135
});
136-
it('Should generate new capi key from 2nd attempt', async () => {
137-
repository.getByUserAccount.mockResolvedValueOnce(null);
138-
when(mockedAxios.post).calledWith('/accounts/cloud-api/cloudApiKeys', expect.anything(), expect.anything())
139-
.mockRejectedValueOnce(mockCapiUnauthorizedError);
140-
141-
expect(await service['ensureCapiKeys'](mockSessionMetadata, mockUtm))
142-
.toEqual(mockCloudUser.capiKey);
143-
expect(mockedAxios.post).toHaveBeenCalledTimes(1);
144-
expect(mockedAxios.post)
145-
.toHaveBeenNthCalledWith(1, '/accounts/cloud-api/cloudApiKeys', expect.anything(), expect.anything());
146-
});
147136
it('Should throw CloudCapiKeyUnauthorizedException if capiKey is not valid', async () => {
148137
repository.getByUserAccount.mockResolvedValueOnce({ ...mockCloudCapiKey, valid: false });
149138

@@ -152,7 +141,7 @@ describe('CloudCapiKeyService', () => {
152141
expect(mockedAxios.post).toHaveBeenCalledTimes(0);
153142
});
154143
it('Should throw CloudApiBadRequestException', async () => {
155-
cloudUserApiService.me.mockResolvedValue(null);
144+
cloudUserApiService.getCloudUser.mockResolvedValue(null);
156145
CloudUserApiService.getCurrentAccount(null);
157146
await expect(service['ensureCapiKeys'](mockSessionMetadata, mockUtm))
158147
.rejects.toThrowError(CloudApiBadRequestException);

0 commit comments

Comments
 (0)