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 <noreply@anthropic.com>
This commit is contained in:
Prateek 2026-02-16 07:01:44 +05:30
parent 3674229388
commit 75615cea95
2 changed files with 53 additions and 2 deletions

View File

@ -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();
});
});

View File

@ -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";