diff --git a/src/datasources/accounts/accounts.datasource.ts b/src/datasources/accounts/accounts.datasource.ts index 937b4e2ae4..5cf08cd406 100644 --- a/src/datasources/accounts/accounts.datasource.ts +++ b/src/datasources/accounts/accounts.datasource.ts @@ -253,15 +253,14 @@ export class AccountsDatasource implements IAccountsDatasource, OnModuleInit { `Invalid client IP while creating account: ${clientIp}`, ); } else { - const current = - (await this.cacheService.increment( - CacheRouter.getRateLimitCacheKey( - `${AccountsDatasource.ACCOUNT_CREATION_CACHE_PREFIX}_${clientIp}`, - ), - this.accountCreationRateLimitPeriodSeconds, - // If the current value cannot be retrieved from Redis (e.g., due to an error or timeout), - // we allow the user to proceed without blocking their operation. - )) ?? 0; + const current = await this.cacheService.increment( + CacheRouter.getRateLimitCacheKey( + `${AccountsDatasource.ACCOUNT_CREATION_CACHE_PREFIX}_${clientIp}`, + ), + this.accountCreationRateLimitPeriodSeconds, + // If the current value cannot be retrieved from Redis (e.g., due to an error or timeout), + // we allow the user to proceed without blocking their operation. + ); if (current > this.accountCreationRateLimitCalls) { this.loggingService.warn( `Limit of ${this.accountCreationRateLimitCalls} reached for IP ${clientIp}`, diff --git a/src/datasources/accounts/counterfactual-safes/counterfactual-safes.datasource.ts b/src/datasources/accounts/counterfactual-safes/counterfactual-safes.datasource.ts index 5ec78c62fb..c2cfd0d576 100644 --- a/src/datasources/accounts/counterfactual-safes/counterfactual-safes.datasource.ts +++ b/src/datasources/accounts/counterfactual-safes/counterfactual-safes.datasource.ts @@ -160,15 +160,14 @@ export class CounterfactualSafesDatasource } private async checkCreationRateLimit(account: Account): Promise { - const current = - (await this.cacheService.increment( - CacheRouter.getRateLimitCacheKey( - `${CounterfactualSafesDatasource.COUNTERFACTUAL_SAFES_CREATION_CACHE_PREFIX}_${account.address}`, - ), - this.counterfactualSafesCreationRateLimitPeriodSeconds, - // If the current value cannot be retrieved from Redis (e.g., due to an error or timeout), - // we allow the user to proceed without blocking their operation. - )) ?? 0; + const current = await this.cacheService.increment( + CacheRouter.getRateLimitCacheKey( + `${CounterfactualSafesDatasource.COUNTERFACTUAL_SAFES_CREATION_CACHE_PREFIX}_${account.address}`, + ), + this.counterfactualSafesCreationRateLimitPeriodSeconds, + // If the current value cannot be retrieved from Redis (e.g., due to an error or timeout), + // we allow the user to proceed without blocking their operation. + ); if (current > this.counterfactualSafesCreationRateLimitCalls) { this.loggingService.warn( `Limit of ${this.counterfactualSafesCreationRateLimitCalls} reached for account ${account.address}`, diff --git a/src/datasources/balances-api/zerion-balances-api.service.ts b/src/datasources/balances-api/zerion-balances-api.service.ts index c754428ab2..1a75935d07 100644 --- a/src/datasources/balances-api/zerion-balances-api.service.ts +++ b/src/datasources/balances-api/zerion-balances-api.service.ts @@ -380,15 +380,14 @@ export class ZerionBalancesApi implements IBalancesApi { } private async _checkRateLimit(): Promise { - const current = - (await this.cacheService.increment( - CacheRouter.getRateLimitCacheKey( - ZerionBalancesApi.RATE_LIMIT_CACHE_KEY_PREFIX, - ), - this.limitPeriodSeconds, - // If the current value cannot be retrieved from Redis (e.g., due to an error or timeout), - // we allow the user to proceed without blocking their operation. - )) ?? 0; + const current = await this.cacheService.increment( + CacheRouter.getRateLimitCacheKey( + ZerionBalancesApi.RATE_LIMIT_CACHE_KEY_PREFIX, + ), + this.limitPeriodSeconds, + // If the current value cannot be retrieved from Redis (e.g., due to an error or timeout), + // we allow the user to proceed without blocking their operation. + ); if (current > this.limitCalls) throw new LimitReachedError(); } } diff --git a/src/datasources/cache/cache.service.interface.ts b/src/datasources/cache/cache.service.interface.ts index e931e07c7a..2206a4bbdb 100644 --- a/src/datasources/cache/cache.service.interface.ts +++ b/src/datasources/cache/cache.service.interface.ts @@ -13,12 +13,12 @@ export interface ICacheService { hGet(cacheDir: CacheDir): Promise; - deleteByKey(key: string): Promise; + deleteByKey(key: string): Promise; increment( cacheKey: string, expireTimeSeconds: number | undefined, - ): Promise; + ): Promise; setCounter( key: string, diff --git a/src/datasources/cache/redis.cache.service.ts b/src/datasources/cache/redis.cache.service.ts index dd520db835..63450650ca 100644 --- a/src/datasources/cache/redis.cache.service.ts +++ b/src/datasources/cache/redis.cache.service.ts @@ -1,4 +1,9 @@ -import { Inject, Injectable, OnModuleDestroy } from '@nestjs/common'; +import { + Inject, + Injectable, + OnModuleDestroy, + ServiceUnavailableException, +} from '@nestjs/common'; import { RedisClientType } from '@/datasources/cache/cache.module'; import { ICacheService } from '@/datasources/cache/cache.service.interface'; import { CacheDir } from '@/datasources/cache/entities/cache-dir.entity'; @@ -40,11 +45,7 @@ export class RedisCacheService } async getCounter(key: string): Promise { - if (!this.ready()) { - this.logRedisClientUnreadyState('getCounter'); - - return null; - } + this.validatgeRedisClientIsReady(); const value = await this.client.get(this._prefixKey(key)); const numericValue = Number(value); @@ -56,16 +57,12 @@ export class RedisCacheService value: string, expireTimeSeconds: number | undefined, ): Promise { + this.validatgeRedisClientIsReady(); + if (!expireTimeSeconds || expireTimeSeconds <= 0) { return; } - if (!this.ready()) { - this.logRedisClientUnreadyState('hSet'); - - return undefined; - } - const key = this._prefixKey(cacheDir.key); try { @@ -80,22 +77,14 @@ export class RedisCacheService } async hGet(cacheDir: CacheDir): Promise { - if (!this.ready()) { - this.logRedisClientUnreadyState('hGet'); - - return undefined; - } + this.validatgeRedisClientIsReady(); const key = this._prefixKey(cacheDir.key); return await this.timeout(this.client.hGet(key, cacheDir.field)); } - async deleteByKey(key: string): Promise { - if (!this.ready()) { - this.logRedisClientUnreadyState('deleteByKey'); - - return undefined; - } + async deleteByKey(key: string): Promise { + this.validatgeRedisClientIsReady(); const keyWithPrefix = this._prefixKey(key); // see https://redis.io/commands/unlink/ @@ -113,12 +102,8 @@ export class RedisCacheService async increment( cacheKey: string, expireTimeSeconds: number | undefined, - ): Promise { - if (!this.ready()) { - this.logRedisClientUnreadyState('increment'); - - return undefined; - } + ): Promise { + this.validatgeRedisClientIsReady(); const transaction = this.client.multi().incr(cacheKey); if (expireTimeSeconds !== undefined && expireTimeSeconds > 0) { @@ -133,11 +118,7 @@ export class RedisCacheService value: number, expireTimeSeconds: number, ): Promise { - if (!this.ready()) { - this.logRedisClientUnreadyState('setCounter'); - - return undefined; - } + this.validatgeRedisClientIsReady(); await this.client.set(key, value, { EX: expireTimeSeconds, NX: true }); } @@ -167,11 +148,8 @@ export class RedisCacheService * instance is not responding it invokes {@link forceQuit}. */ async onModuleDestroy(): Promise { - if (!this.ready()) { - this.logRedisClientUnreadyState('onModuleDestroy'); + this.validatgeRedisClientIsReady(); - return undefined; - } this.loggingService.info('Closing Redis connection...'); try { await promiseWithTimeout( @@ -190,11 +168,7 @@ export class RedisCacheService * Forces the closing of the Redis connection associated with this service. */ private async forceQuit(): Promise { - if (!this.ready()) { - this.logRedisClientUnreadyState('forceQuit'); - - return undefined; - } + this.validatgeRedisClientIsReady(); this.loggingService.warn('Forcing Redis connection to close...'); try { await this.client.disconnect(); @@ -207,7 +181,7 @@ export class RedisCacheService private async timeout( queryObject: Promise, timeout?: number, - ): Promise { + ): Promise { timeout = timeout ?? this.configurationService.getOrThrow('redis.timeout'); try { @@ -215,17 +189,17 @@ export class RedisCacheService } catch (error) { if (error instanceof PromiseTimeoutError) { this.loggingService.error('Redis Query Timed out!'); - - return undefined; } throw error; } } - private logRedisClientUnreadyState(operation: string): void { - this.loggingService.error( - `Redis client is not ready. Redis ${operation} failed!`, - ); + private validatgeRedisClientIsReady(): void { + if (!this.ready()) { + this.loggingService.error(`Redis client is not ready`); + + throw new ServiceUnavailableException('Redis client is not ready'); + } } } diff --git a/src/domain/common/utils/promise.ts b/src/domain/common/utils/promise.ts index b6bf6aa438..6724ef5985 100644 --- a/src/domain/common/utils/promise.ts +++ b/src/domain/common/utils/promise.ts @@ -1,16 +1,22 @@ import { HttpException, HttpStatus } from '@nestjs/common'; -export const promiseWithTimeout = ( +export const promiseWithTimeout = async ( promise: Promise, timeout: number, -): Promise => { +): Promise => { let timeoutId: NodeJS.Timeout; - return Promise.race([ + return Promise.race([ promise, new Promise((_, reject) => { timeoutId = setTimeout( - () => reject(new PromiseTimeoutError('Promise timed out!', 500)), + () => + reject( + new PromiseTimeoutError( + 'Promise timed out!', + HttpStatus.INTERNAL_SERVER_ERROR, + ), + ), timeout, ); }),