Merge pull request #1427 from RedisInsight/bugfix/regresion-fixes

Bugfix/regresion fixes

Author: AlenaSY

redisinsight/api/src/common/interceptors/timeout.interceptor.ts

@@ -18,7 +18,7 @@ export class TimeoutInterceptor implements NestInterceptor {
 
   private readonly message: string;
 
-  constructor(message?: string) {
+  constructor(message: string = 'Request timeout') {
     this.message = message;
   }
 

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

@@ -61,3 +61,5 @@ export enum CommandType {
   Core = 'core',
   Module = 'module',
 }
+
+export const unknownCommand = 'unknown';

redisinsight/api/src/modules/analytics/telemetry.base.service.spec.ts

@@ -33,11 +33,11 @@ describe('TelemetryBaseService', () => {
 
   describe('sendEvent', () => {
     it('should emit event', () => {
-      service.sendEvent(TelemetryEvents.RedisInstanceAdded, { data: 'Some data' });
+      service.sendEvent(TelemetryEvents.RedisInstanceAdded, { data: 'Some data', command: 'lowercase' });
 
       expect(eventEmitter.emit).toHaveBeenCalledWith(AppAnalyticsEvents.Track, {
         event: TelemetryEvents.RedisInstanceAdded,
-        eventData: { data: 'Some data' },
+        eventData: { data: 'Some data', command: 'LOWERCASE' },
       });
     });
     it('should emit event with empty event data', () => {
@@ -87,13 +87,18 @@ describe('TelemetryBaseService', () => {
       });
     });
     it('should emit event with additional event data', () => {
-      service.sendFailedEvent(TelemetryEvents.RedisInstanceAddFailed, httpException, { data: 'Some data' });
+      service.sendFailedEvent(
+        TelemetryEvents.RedisInstanceAddFailed,
+        httpException,
+        { data: 'Some data', command: 'lowercase' },
+      );
 
       expect(eventEmitter.emit).toHaveBeenCalledWith(AppAnalyticsEvents.Track, {
         event: TelemetryEvents.RedisInstanceAddFailed,
         eventData: {
           error: 'Internal Server Error',
           data: 'Some data',
+          command: 'LOWERCASE',
         },
       });
     });

redisinsight/api/src/modules/analytics/telemetry.base.service.ts

@@ -1,3 +1,4 @@
+import { isString } from 'lodash';
 import { EventEmitter2 } from '@nestjs/event-emitter';
 import { HttpException } from '@nestjs/common';
 import { AppAnalyticsEvents } from 'src/constants';
@@ -13,7 +14,10 @@ export abstract class TelemetryBaseService {
     try {
       this.eventEmitter.emit(AppAnalyticsEvents.Track, {
         event,
-        eventData,
+        eventData: {
+          ...eventData,
+          command: isString(eventData['command']) ? eventData['command'].toUpperCase() : eventData['command'],
+        },
       });
     } catch (e) {
       // continue regardless of error
@@ -27,6 +31,7 @@ export abstract class TelemetryBaseService {
         eventData: {
           error: exception.getResponse()['error'] || exception.message,
           ...eventData,
+          command: isString(eventData['command']) ? eventData['command'].toUpperCase() : eventData['command'],
         },
       });
     } catch (e) {

redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/cluster.strategy.spec.ts

@@ -943,7 +943,7 @@ describe('Cluster Scanner Strategy', () => {
             scanned: 10,
           },
         ]);
-        expect(strategy.getKeyInfo).toHaveBeenCalledWith(clusterClient, key);
+        expect(strategy.getKeyInfo).toHaveBeenCalledWith(clusterClient, Buffer.from(key));
         expect(strategy.scanNodes).not.toHaveBeenCalled();
       });
       it('should find exact key when match is escaped glob patter', async () => {
@@ -973,7 +973,7 @@ describe('Cluster Scanner Strategy', () => {
             scanned: 10,
           },
         ]);
-        expect(strategy.getKeyInfo).toHaveBeenCalledWith(clusterClient, searchPattern);
+        expect(strategy.getKeyInfo).toHaveBeenCalledWith(clusterClient, Buffer.from(searchPattern));
         expect(strategy.scanNodes).not.toHaveBeenCalled();
       });
       it('should find exact key with correct type', async () => {

redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/cluster.strategy.ts

@@ -49,7 +49,7 @@ export class ClusterStrategy extends AbstractStrategy {
     await this.calculateNodesTotalKeys(clientOptions, currentDbIndex, nodes);
 
     if (!isGlob(match, { strict: false })) {
-      const keyName = unescapeGlob(match);
+      const keyName = Buffer.from(unescapeGlob(match));
       nodes.forEach((node) => {
         // eslint-disable-next-line no-param-reassign
         node.cursor = 0;

redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/standalone.strategy.spec.ts

@@ -476,7 +476,7 @@ describe('Standalone Scanner Strategy', () => {
           },
         ]);
         expect(strategy.getKeysInfo).toHaveBeenCalledWith(nodeClient, [
-          key,
+          Buffer.from(key),
         ]);
         expect(strategy.scan).not.toHaveBeenCalled();
       });
@@ -497,7 +497,7 @@ describe('Standalone Scanner Strategy', () => {
             keys: [{ ...getKeyInfoResponse, name: mockSearchPattern }],
           },
         ]);
-        expect(strategy.getKeysInfo).toHaveBeenCalledWith(nodeClient, [mockSearchPattern]);
+        expect(strategy.getKeysInfo).toHaveBeenCalledWith(nodeClient, [Buffer.from(mockSearchPattern)]);
         expect(strategy.scan).not.toHaveBeenCalled();
       });
       it('should find exact key with correct type', async () => {

redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/standalone.strategy.ts

@@ -62,7 +62,7 @@ export class StandaloneStrategy extends AbstractStrategy {
     }
 
     if (!isGlob(match, { strict: false })) {
-      const keyName = unescapeGlob(match);
+      const keyName = Buffer.from(unescapeGlob(match));
       node.cursor = 0;
       node.scanned = isNull(node.total) ? 1 : node.total;
       node.keys = await this.getKeysInfo(client, [keyName]);

redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.spec.ts

@@ -240,7 +240,7 @@ describe('RedisearchService', () => {
           'LIMIT', `${mockSearchRedisearchDto.offset}`, `${mockSearchRedisearchDto.limit}`,
         ],
       }));
-      expect(nodeClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining( {
+      expect(nodeClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining({
         name: 'FT.CONFIG',
         args: [
           'GET',
@@ -277,7 +277,7 @@ describe('RedisearchService', () => {
           'LIMIT', `${mockSearchRedisearchDto.offset}`, `${mockSearchRedisearchDto.limit}`,
         ],
       }));
-      expect(clusterClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining( {
+      expect(clusterClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining({
         name: 'FT.CONFIG',
         args: [
           'GET',

redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts

@@ -3,6 +3,7 @@ import { toNumber, uniq } from 'lodash';
 import {
   BadRequestException,
   ConflictException,
+  HttpException,
   Injectable,
   Logger,
 } from '@nestjs/common';
@@ -147,11 +148,6 @@ export class RedisearchService {
 
       const client = await this.browserTool.getRedisClient(clientOptions);
 
-      const [total, ...keyNames] = await client.sendCommand(
-        new Command('FT.SEARCH', [index, query, 'NOCONTENT', 'LIMIT', offset, limit]),
-      );
-
-
       try {
         const [[, maxSearchResults]] = await client.sendCommand(
           // response: [ [ 'MAXSEARCHRESULTS', '10000' ] ]
@@ -165,19 +161,34 @@ export class RedisearchService {
         maxResults = null;
       }
 
+      // Workaround: recalculate limit to not query more then MAXSEARCHRESULTS
+      let safeLimit = limit;
+      if (maxResults && offset + limit > maxResults) {
+        safeLimit = offset <= maxResults ? maxResults - offset : limit;
+      }
+
+      const [total, ...keyNames] = await client.sendCommand(
+        new Command('FT.SEARCH', [index, query, 'NOCONTENT', 'LIMIT', offset, safeLimit]),
+      );
+
       return plainToClass(GetKeysWithDetailsResponse, {
         cursor: limit + offset,
         total,
         scanned: keyNames.length + offset,
         keys: keyNames.map((name) => ({ name })),
         maxResults,
       });
-    } catch (error) {
-      this.logger.error('Failed to search keys using redisearch index', error);
-      if (error.message?.includes(RedisErrorCodes.RedisearchLimit)) {
+    } catch (e) {
+      this.logger.error('Failed to search keys using redisearch index', e);
+
+      if (e instanceof HttpException) {
+        throw e;
+      }
+
+      if (e.message?.includes(RedisErrorCodes.RedisearchLimit)) {
         throw new BadRequestException(ERROR_MESSAGES.INCREASE_MINIMUM_LIMIT(numberWithSpaces(dto.limit)));
       }
-      throw catchAclError(error);
+      throw catchAclError(e);
     }
   }