From 75615cea95e82094b19a4ea31f372186270bf020 Mon Sep 17 00:00:00 2001 From: Prateek Date: Mon, 16 Feb 2026 07:01:44 +0530 Subject: [PATCH] fix: correct grammar in RateLimitError messages Addresses Cursor Bugbot review comment on PR #65. ## Issue Error messages showed grammatically incorrect output: - "in 1 minutes" (should be "in 1 minute") - "in 0 minutes" for past reset times (should be "should be available now") - Always showed minutes even for short durations under 60 seconds ## Fix - Handle singular vs plural correctly ("1 second" vs "2 seconds", "1 minute" vs "2 minutes") - Show seconds for durations under 60 seconds ("in 30 seconds") - Show "should be available now" when reset time is in the past - Update tests to expect correct grammar ## Tests - Updated existing tests to expect correct output - Added new tests for singular/plural edge cases - All 9 rate limit tests passing Co-Authored-By: Claude Sonnet 4.5 --- .../core/src/__tests__/rate-limit.test.ts | 39 ++++++++++++++++++- packages/core/src/types.ts | 16 +++++++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/packages/core/src/__tests__/rate-limit.test.ts b/packages/core/src/__tests__/rate-limit.test.ts index b38aa97fd..7c598243e 100644 --- a/packages/core/src/__tests__/rate-limit.test.ts +++ b/packages/core/src/__tests__/rate-limit.test.ts @@ -55,7 +55,7 @@ describe("RateLimitError", () => { const error = new RateLimitError("rest", resetAt); expect(error.retryAfter).toBe(30); - expect(error.message).toContain("1 minutes"); // Ceiling of 30s / 60 = 1 min + expect(error.message).toContain("30 seconds"); // Shows seconds, not minutes vi.useRealTimers(); }); @@ -70,6 +70,7 @@ describe("RateLimitError", () => { // retryAfter should be 0 (already reset) expect(error.retryAfter).toBe(0); + expect(error.message).toContain("should be available now"); vi.useRealTimers(); }); @@ -94,4 +95,40 @@ describe("RateLimitError", () => { const searchError = new RateLimitError("search", resetAt); expect(searchError.resource).toBe("search"); }); + + it("should handle singular forms correctly", () => { + vi.useFakeTimers(); + const now = new Date("2026-02-16T10:00:00Z"); + vi.setSystemTime(now); + + // 1 second + const resetAt1s = new Date("2026-02-16T10:00:01Z"); + const error1s = new RateLimitError("rest", resetAt1s); + expect(error1s.message).toContain("1 second"); // singular, not "1 seconds" + + // 1 minute + const resetAt1m = new Date("2026-02-16T10:01:00Z"); + const error1m = new RateLimitError("rest", resetAt1m); + expect(error1m.message).toContain("1 minute"); // singular, not "1 minutes" + + vi.useRealTimers(); + }); + + it("should handle plural forms correctly", () => { + vi.useFakeTimers(); + const now = new Date("2026-02-16T10:00:00Z"); + vi.setSystemTime(now); + + // 2 seconds + const resetAt2s = new Date("2026-02-16T10:00:02Z"); + const error2s = new RateLimitError("rest", resetAt2s); + expect(error2s.message).toContain("2 seconds"); + + // 2 minutes + const resetAt2m = new Date("2026-02-16T10:02:00Z"); + const error2m = new RateLimitError("rest", resetAt2m); + expect(error2m.message).toContain("2 minutes"); + + vi.useRealTimers(); + }); }); diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 13710c358..4670d3747 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -53,8 +53,22 @@ export class RateLimitError extends Error { hour: "2-digit", minute: "2-digit", }); + + // Format retry time as human-readable string with correct grammar + let retryMessage: string; + if (retryAfter === 0) { + retryMessage = "should be available now"; + } else if (retryAfter < 60) { + const plural = retryAfter === 1 ? "" : "s"; + retryMessage = `in ${retryAfter} second${plural}`; + } else { + const minutes = Math.ceil(retryAfter / 60); + const plural = minutes === 1 ? "" : "s"; + retryMessage = `in ${minutes} minute${plural}`; + } + super( - `Rate limit exceeded for ${resource}. Resets at ${resetTime} (in ${Math.ceil(retryAfter / 60)} minutes)`, + `Rate limit exceeded for ${resource}. Resets at ${resetTime} (${retryMessage})`, options, ); this.name = "RateLimitError";