Skip to content

Commit 194a10e

Browse files
fix(backend): improve reliability of rate limiting, leaderboards, and PB updates (@chinmaydwivedi)
- Migrate bad auth rate limiter from in-memory to Redis-backed store with memory fallback, fixing per-instance bypass in multi-instance deployments - Replace full document fetch with $count aggregation for friends leaderboard count, avoiding loading all entries into Node memory - Add 10-minute TTL to leaderboard count cache to prevent stale data and unbounded memory growth - Merge two separate updateOne calls in checkIfPb into a single atomic operation to prevent inconsistent personalBests/lbPersonalBests state - Remove stale hourOffset migration code in updateStreak - Remove leftover console.log in addImportantLog that leaked event data to stdout
1 parent 5f43b94 commit 194a10e

File tree

5 files changed

+61
-33
lines changed

5 files changed

+61
-33
lines changed

backend/src/dal/leaderboards.ts

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ export async function get(
9494
}
9595
}
9696

97-
const cachedCounts = new Map<string, number>();
97+
const CACHE_TTL_MS = 10 * 60 * 1000;
98+
const cachedCounts = new Map<string, { count: number; expiresAt: number }>();
9899

99100
export async function getCount(
100101
mode: string,
@@ -103,27 +104,33 @@ export async function getCount(
103104
uid?: string,
104105
): Promise<number> {
105106
const key = `${language}_${mode}_${mode2}`;
106-
if (uid === undefined && cachedCounts.has(key)) {
107-
return cachedCounts.get(key) as number;
107+
const cached = cachedCounts.get(key);
108+
if (
109+
uid === undefined &&
110+
cached !== undefined &&
111+
cached.expiresAt > Date.now()
112+
) {
113+
return cached.count;
108114
} else {
109115
if (uid === undefined) {
110116
const count = await getCollection({
111117
language,
112118
mode,
113119
mode2,
114120
}).estimatedDocumentCount();
115-
cachedCounts.set(key, count);
121+
cachedCounts.set(key, { count, expiresAt: Date.now() + CACHE_TTL_MS });
116122
return count;
117123
} else {
118-
return (
119-
await aggregateWithAcceptedConnections(
120-
{
121-
collectionName: getCollectionName({ language, mode, mode2 }),
122-
uid,
123-
},
124-
[{ $project: { _id: true } }],
125-
)
126-
).length;
124+
const result = await aggregateWithAcceptedConnections<{
125+
total: number;
126+
}>(
127+
{
128+
collectionName: getCollectionName({ language, mode, mode2 }),
129+
uid,
130+
},
131+
[{ $count: "total" }],
132+
);
133+
return result[0]?.total ?? 0;
127134
}
128135
}
129136
}

backend/src/dal/logs.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ export async function addImportantLog(
5656
message: string | Record<string, unknown>,
5757
uid = "",
5858
): Promise<void> {
59-
console.log("log", event, message, uid);
6059
await insertIntoDb(event, message, uid, true);
6160
}
6261

backend/src/dal/user.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -493,17 +493,14 @@ export async function checkIfPb(
493493

494494
if (!pb.isPb) return false;
495495

496-
await getUsersCollection().updateOne(
497-
{ uid },
498-
{ $set: { personalBests: pb.personalBests } },
499-
);
500-
496+
const setFields: Record<string, unknown> = {
497+
personalBests: pb.personalBests,
498+
};
501499
if (pb.lbPersonalBests) {
502-
await getUsersCollection().updateOne(
503-
{ uid },
504-
{ $set: { lbPersonalBests: pb.lbPersonalBests } },
505-
);
500+
setFields["lbPersonalBests"] = pb.lbPersonalBests;
506501
}
502+
503+
await getUsersCollection().updateOne({ uid }, { $set: setFields });
507504
return true;
508505
}
509506

@@ -1149,11 +1146,6 @@ export async function updateStreak(
11491146

11501147
streak.lastResultTimestamp = timestamp;
11511148

1152-
if (user.streak?.hourOffset === 0) {
1153-
// todo this needs to be removed after a while
1154-
delete streak.hourOffset;
1155-
}
1156-
11571149
await getUsersCollection().updateOne({ uid }, { $set: { streak } });
11581150

11591151
return streak.length;

backend/src/middlewares/rate-limit.ts

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import MonkeyError from "../utils/error";
22
import type { Response, NextFunction, Request } from "express";
3-
import { RateLimiterMemory } from "rate-limiter-flexible";
3+
import {
4+
RateLimiterMemory,
5+
RateLimiterRedis,
6+
type RateLimiterAbstract,
7+
} from "rate-limiter-flexible";
48
import {
59
rateLimit,
610
RateLimitRequestHandler,
@@ -21,6 +25,7 @@ import {
2125
TsRestRequestWithContext,
2226
} from "../api/types";
2327
import { AppRoute, AppRouter } from "@ts-rest/core";
28+
import * as RedisClient from "../init/redis";
2429

2530
export const REQUEST_MULTIPLIER = isDevEnvironment() ? 100 : 1;
2631

@@ -149,10 +154,31 @@ export const rootRateLimiter = rateLimit({
149154
});
150155

151156
// Bad Authentication Rate Limiter
152-
const badAuthRateLimiter = new RateLimiterMemory({
153-
points: 30 * REQUEST_MULTIPLIER,
154-
duration: 60 * 60, //one hour seconds
155-
});
157+
// Uses Redis when available for consistency across instances, falls back to memory
158+
function createBadAuthRateLimiter(): RateLimiterAbstract {
159+
const opts = {
160+
points: 30 * REQUEST_MULTIPLIER,
161+
duration: 60 * 60,
162+
};
163+
164+
const redisConnection = RedisClient.getConnection();
165+
if (redisConnection !== null) {
166+
return new RateLimiterRedis({
167+
...opts,
168+
storeClient: redisConnection,
169+
keyPrefix: "bad_auth",
170+
insuranceLimiter: new RateLimiterMemory(opts),
171+
});
172+
}
173+
174+
return new RateLimiterMemory(opts);
175+
}
176+
177+
let badAuthRateLimiter: RateLimiterAbstract = createBadAuthRateLimiter();
178+
179+
export function reinitBadAuthRateLimiter(): void {
180+
badAuthRateLimiter = createBadAuthRateLimiter();
181+
}
156182

157183
export async function badAuthRateLimiterHandler(
158184
req: ExpressRequestWithContext,

backend/src/server.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { createIndicies as leaderboardDbSetup } from "./dal/leaderboards";
1919
import { createIndicies as blocklistDbSetup } from "./dal/blocklist";
2020
import { createIndicies as connectionsDbSetup } from "./dal/connections";
2121
import { getErrorMessage } from "./utils/error";
22+
import { reinitBadAuthRateLimiter } from "./middlewares/rate-limit";
2223

2324
async function bootServer(port: number): Promise<Server> {
2425
try {
@@ -46,6 +47,9 @@ async function bootServer(port: number): Promise<Server> {
4647
Logger.success("Connected to redis");
4748
const connection = RedisClient.getConnection();
4849

50+
Logger.info("Upgrading bad auth rate limiter to Redis...");
51+
reinitBadAuthRateLimiter();
52+
4953
Logger.info("Initializing queues...");
5054
queues.forEach((queue) => {
5155
queue.init(connection ?? undefined);

0 commit comments

Comments
 (0)