From 111a09514cdabeee83f3f89bb27bc9cf987c3b16 Mon Sep 17 00:00:00 2001 From: nischit Date: Tue, 14 Jan 2025 21:47:13 +0545 Subject: [PATCH 1/6] track ratelimit only on creation --- packages/service-utils/src/core/rateLimit/index.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/service-utils/src/core/rateLimit/index.ts b/packages/service-utils/src/core/rateLimit/index.ts index 000253ac9e3..334010b2fba 100644 --- a/packages/service-utils/src/core/rateLimit/index.ts +++ b/packages/service-utils/src/core/rateLimit/index.ts @@ -71,9 +71,15 @@ export async function rateLimit(args: { limitPerSecond * sampleRate * RATE_LIMIT_WINDOW_SECONDS; if (requestCount > limitPerWindow) { - // Report rate limit hits. - if (project?.id) { - await updateRateLimitedAt(project.id, serviceConfig); + /** + * Report rate limit hits. + * Only track rate limit when its hit for the first time. + * Not waiting for tracking to complete as user doesn't need to wait. + */ + if (requestCount === 1 && project?.id) { + updateRateLimitedAt(project.id, serviceConfig).catch(() => { + // no-op + }); } // Reject requests when they've exceeded 2x the rate limit. From 91f07d8c8bc845914b63dfd5c365910aff80e1c8 Mon Sep 17 00:00:00 2001 From: nischit Date: Tue, 14 Jan 2025 21:56:49 +0545 Subject: [PATCH 2/6] check fix --- packages/service-utils/src/core/rateLimit/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/service-utils/src/core/rateLimit/index.ts b/packages/service-utils/src/core/rateLimit/index.ts index 334010b2fba..cc0594c4fbd 100644 --- a/packages/service-utils/src/core/rateLimit/index.ts +++ b/packages/service-utils/src/core/rateLimit/index.ts @@ -76,7 +76,7 @@ export async function rateLimit(args: { * Only track rate limit when its hit for the first time. * Not waiting for tracking to complete as user doesn't need to wait. */ - if (requestCount === 1 && project?.id) { + if (requestCount <= limitPerWindow + 1 && project?.id) { updateRateLimitedAt(project.id, serviceConfig).catch(() => { // no-op }); From c711c18e74d10dae4025b595df730ca6d0c0dc81 Mon Sep 17 00:00:00 2001 From: nischit Date: Tue, 14 Jan 2025 22:27:14 +0545 Subject: [PATCH 3/6] remove 2x limit window leeway --- .../service-utils/src/core/rateLimit/index.ts | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/service-utils/src/core/rateLimit/index.ts b/packages/service-utils/src/core/rateLimit/index.ts index cc0594c4fbd..494e14fa8f6 100644 --- a/packages/service-utils/src/core/rateLimit/index.ts +++ b/packages/service-utils/src/core/rateLimit/index.ts @@ -82,17 +82,14 @@ export async function rateLimit(args: { }); } - // Reject requests when they've exceeded 2x the rate limit. - if (requestCount > 2 * limitPerWindow) { - return { - rateLimited: true, - requestCount, - rateLimit: limitPerWindow, - status: 429, - errorMessage: `You've exceeded your ${serviceScope} rate limit at ${limitPerSecond} reqs/sec. To get higher rate limits, contact us at https://thirdweb.com/contact-us.`, - errorCode: "RATE_LIMIT_EXCEEDED", - }; - } + return { + rateLimited: true, + requestCount, + rateLimit: limitPerWindow, + status: 429, + errorMessage: `You've exceeded your ${serviceScope} rate limit at ${limitPerSecond} reqs/sec. To get higher rate limits, contact us at https://thirdweb.com/contact-us.`, + errorCode: "RATE_LIMIT_EXCEEDED", + }; } return { From d077061bf1b2883bb14f2bb69f11e168e510be9d Mon Sep 17 00:00:00 2001 From: nischit Date: Wed, 15 Jan 2025 11:42:44 +0545 Subject: [PATCH 4/6] minor change --- packages/service-utils/src/core/rateLimit/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/service-utils/src/core/rateLimit/index.ts b/packages/service-utils/src/core/rateLimit/index.ts index 494e14fa8f6..10a90fabcbc 100644 --- a/packages/service-utils/src/core/rateLimit/index.ts +++ b/packages/service-utils/src/core/rateLimit/index.ts @@ -76,7 +76,7 @@ export async function rateLimit(args: { * Only track rate limit when its hit for the first time. * Not waiting for tracking to complete as user doesn't need to wait. */ - if (requestCount <= limitPerWindow + 1 && project?.id) { + if (requestCount === limitPerWindow + 1 && project?.id) { updateRateLimitedAt(project.id, serviceConfig).catch(() => { // no-op }); From f2754a1a169576e381629c363042e46e157db01f Mon Sep 17 00:00:00 2001 From: nischit Date: Wed, 15 Jan 2025 12:14:55 +0545 Subject: [PATCH 5/6] tests update --- .../src/core/rateLimit/rateLimit.test.ts | 31 +++++-------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/packages/service-utils/src/core/rateLimit/rateLimit.test.ts b/packages/service-utils/src/core/rateLimit/rateLimit.test.ts index f18c30c6e55..203bd0e08de 100644 --- a/packages/service-utils/src/core/rateLimit/rateLimit.test.ts +++ b/packages/service-utils/src/core/rateLimit/rateLimit.test.ts @@ -10,7 +10,7 @@ const mockRedis = { // Mocking the updateRateLimitedAt function vi.mock("../../../src/core/api", () => ({ - updateRateLimitedAt: vi.fn(), + updateRateLimitedAt: vi.fn().mockResolvedValue({}), })); describe("rateLimit", () => { @@ -59,27 +59,8 @@ describe("rateLimit", () => { expect(mockRedis.expire).not.toHaveBeenCalled(); }); - it("should report rate limit if exceeded but not block", async () => { - mockRedis.incr.mockResolvedValue(51); // Current count is 51 requests in 10 seconds. - - const result = await rateLimit({ - project: validProjectResponse, - limitPerSecond: 5, - serviceConfig: validServiceConfig, - redis: mockRedis, - }); - - expect(result).toEqual({ - rateLimited: false, - requestCount: 51, - rateLimit: 50, - }); - expect(updateRateLimitedAt).toHaveBeenCalled(); - expect(mockRedis.expire).not.toHaveBeenCalled(); - }); - it("should rate limit if exceeded hard limit", async () => { - mockRedis.incr.mockResolvedValue(101); + mockRedis.incr.mockResolvedValue(51); const result = await rateLimit({ project: validProjectResponse, @@ -90,7 +71,7 @@ describe("rateLimit", () => { expect(result).toEqual({ rateLimited: true, - requestCount: 101, + requestCount: 51, rateLimit: 50, status: 429, errorMessage: `You've exceeded your storage rate limit at 5 reqs/sec. To get higher rate limits, contact us at https://thirdweb.com/contact-us.`, @@ -131,9 +112,13 @@ describe("rateLimit", () => { }); expect(result).toEqual({ - rateLimited: false, + rateLimited: true, requestCount: 10, rateLimit: 5, + status: 429, + errorMessage: + "You've exceeded your storage rate limit at 5 reqs/sec. To get higher rate limits, contact us at https://thirdweb.com/contact-us.", + errorCode: "RATE_LIMIT_EXCEEDED", }); }); From 45c046f93e33cb13e0d21e5af880e69632acca5b Mon Sep 17 00:00:00 2001 From: nischit Date: Wed, 15 Jan 2025 13:01:13 +0545 Subject: [PATCH 6/6] minor version bump --- .changeset/green-tips-beam.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/green-tips-beam.md diff --git a/.changeset/green-tips-beam.md b/.changeset/green-tips-beam.md new file mode 100644 index 00000000000..aa05755e587 --- /dev/null +++ b/.changeset/green-tips-beam.md @@ -0,0 +1,5 @@ +--- +"@thirdweb-dev/service-utils": minor +--- + +track usage call once per ratelimit window