fix(KVStore): resolve KVStore time window and expiration issues

[lumpinif]

Fixed KV Store Rate Limiter Implementation

• Fix increment/decrement methods to properly handle active time windows
• Add KV_MIN_EXPIRATION_BUFFER to enforce Cloudflare's 60s minimum requirement
• Extract updateRecord method to ensure consistent KV updates
• Add calculateExpiration helper to handle expiration times correctly
• Prevent totalHits from going below 0 in decrement method
• Update docstrings with accurate behavior descriptions

Problems

The original implementation had two key issues with Cloudflare KV time handling:

1. KV expiration was set equal to the rate limit window's reset time
2. This might violate Cloudflare's requirement that KV expiration must be at least 60 seconds in the future

Example of error ❌
It might work for the first request with ANY window > 60s, but it fails for late requests in ANY window size when:

(resetTime - currentTime) < 60 seconds

// Original code issue example
// User makes first request at 16:58:58
const nowMS = Date.now();  // 16:58:58
const windowMs = 60000;    // 1-minute window

let payload = {
    totalHits: 1,
    resetTime: new Date(nowMS + windowMs)  // 16:59:58 (window end)
};

// KV expiration is set to:
expiration: payload.resetTime.getTime() / 1000  // 16:59:58 

// User makes another request at 16:59:40 (42s into window)
const record = await this.get(key);  // Gets existing record
payload = {
    totalHits: record.totalHits + 1,
    resetTime: new Date(record.resetTime)  // Still 16:59:58
};

// KV expiration is set to:
expiration: payload.resetTime.getTime() / 1000  // 16:59:58 ❌
// Only 18s in future! Violates Cloudflare's 60s minimum

Visual Timeline:

16:58:58 -------- 16:59:40 -- 16:59:58
[First Req]      [Late Req]  [Window End/KV Expires]
     |               |            |
     |<------ 1-minute window -->|
                    |<- Only 18s ->| ❌ Too short!

Solution

Enhanced the KVStore implementation to properly handle both rate limiting windows and KV expiration:

1. Separated Concerns

   • Rate limit window: Controls when users can make new requests
   • KV expiration: Ensures data storage meets Cloudflare's requirements

2. Window Management

   const nowMS = Date.now();
   const existingResetTimeMS = record?.resetTime && new Date(record.resetTime).getTime();
   const isActiveWindow = existingResetTimeMS && existingResetTimeMS > nowMS;

The fix ensures:

// For the same late request at 16:59:40
const expirationSeconds = Math.max(
    resetTimeSeconds + 60,  // 17:00:58
    nowSeconds + 60        // 17:00:40
);
// Uses 17:00:58 ✅ Always at least 60s in future

Another case where nowSeconds + 60 is used:

// Example: 5-minute window (300s), and 5 limits 
const windowMs = 300000;  // 5 minutes

// User makes 1st request at 17:04:51, valid hit 1/5 🎯
const nowMS = Date.now();                    // 17:04:51
const defaultResetTime = nowMS + windowMs;   // 17:09:51 (window ends)
const expirationSeconds = Math.max(
    resetTimeSeconds + 60,  // 17:10:51 ✅ used (later)
    nowSeconds + 60        // 17:05:51
);

// Now at 17:09:30 (21s before window ends)
// User makes 2nd request, valid hit 2/5 🎯
nowMS = Date.now();                         // 17:09:30
resetTimeSeconds = 17:09:51                 // Original window ends
const expirationSeconds = Math.max(
    resetTimeSeconds + 60,  // 17:10:51 ✅ still used (later)
    nowSeconds + 60        // 17:10:30
);
// Still uses resetTimeSeconds + 60 because it's later

// BUT! At 17:09:52 (5s after window ends)
// User makes 3rd request, still valid hit 3/5 🎯
nowMS = Date.now();                         // 17:09:55
resetTimeSeconds = 17:09:51                 // old window that just ended
const expirationSeconds = Math.max(
    resetTimeSeconds + 60,  // 17:10:51
    nowSeconds + 60        // 17:10:55 ✅ used instead because it's later, and still compliant with the min-60s rule
);

Benefits

✅ Compliant with Cloudflare's 60-second minimum expiration requirement
✅ Accurate rate limiting for any window duration (even < 60s)
✅ Clean separation between rate limiting logic and KV storage requirements
✅ Improved code organization with helper methods
✅ Better type safety and error handling

Testing

Works correctly for all window durations:

• Short windows (< 60s)
• Equal windows (60s)
• Long windows (> 60s)

[vercel]

@lumpinif is attempting to deploy a commit to the rhinobase Team on Vercel.

A member of the Team first needs to authorize it.

[MathurAditya724]

This looks amazing!

I have left some small changes; once completed, I will merge this into the main. Thank you for this!

[MathurAditya724]

Can you lint the code so that your changes only show up? You can run pnpm run format, for biome to format this code. This looks like there might be some issue with lineEnding formatting in biome.json

[lumpinif]

This looks amazing!

I have left some small changes; once completed, I will merge this into the main. Thank you for this!

Thanks:)

Btw, I've noticed an error in the example at the end of the part where the nowSecond+60 is used: It should be showcasing the nowSecond+60 is used when the reset time is passed while the expiration is still not ended - this should not be affecting the implementation though, just an example error.

Please ensure making sufficient tests on my bad codes!

[lumpinif]

This looks amazing!
I have left some small changes; once completed, I will merge this into the main. Thank you for this!

Thanks:)

Btw, I've noticed an error in the example at the end of the part where the nowSecond+60 is used: It should be showcasing the nowSecond+60 is used when the reset time is passed while the expiration is still not ended - this should not be affecting the implementation though, just an example error.

Please ensure making sufficient tests on my bad codes!

pr comment has been updated with correct showcase of where nowSeconds + 60 is used.

[MathurAditya724]

Great! I will take a look 🙌🏼