refactor: improve error handling in rate limit middleware to allow key generator and store errors to bubble up

Author: jkyberneees

lib/middleware/rate-limit.js

@@ -111,82 +111,50 @@ function createRateLimit(options = {}) {
       return next()
     }
 
-    try {
-      const key = await keyGenerator(req)
-      const {totalHits, resetTime} = await activeStore.increment(key, windowMs)
-
-      if (totalHits > max) {
-        let response
-
-        // If a custom message is provided, use it as plain text
-        if (message) {
-          response = new Response(message, {status: 429})
-        } else {
-          response = await handler(req, totalHits, max, resetTime)
-          if (typeof response === 'string') {
-            response = new Response(response, {status: 429})
-          }
+    // Generate key and record the request immediately - let errors bubble up
+    const key = await keyGenerator(req)
+    const {totalHits, resetTime} = await activeStore.increment(key, windowMs)
+
+    // Check if rate limit exceeded
+    if (totalHits > max) {
+      let response
+
+      // If a custom message is provided, use it as plain text
+      if (message) {
+        response = new Response(message, {status: 429})
+      } else {
+        response = await handler(req, totalHits, max, resetTime)
+        if (typeof response === 'string') {
+          response = new Response(response, {status: 429})
         }
-
-        return addRateLimitHeaders(response, totalHits, resetTime)
       }
 
-      // Set rate limit context
-      req.ctx = req.ctx || {}
-      req.ctx.rateLimit = {
-        limit: max,
-        used: totalHits,
-        remaining: Math.max(0, max - totalHits),
-        resetTime,
-        current: totalHits,
-        reset: resetTime,
-      }
-      req.rateLimit = {
-        limit: max,
-        remaining: Math.max(0, max - totalHits),
-        current: totalHits,
-        reset: resetTime,
-      }
+      return addRateLimitHeaders(response, totalHits, resetTime)
+    }
 
-      const response = await next()
-      if (response instanceof Response) {
-        return addRateLimitHeaders(response, totalHits, resetTime)
-      }
-      return response
-    } catch (error) {
-      // If key generation fails, fallback to a default key
-      try {
-        const key = 'unknown'
-        const {totalHits, resetTime} = await activeStore.increment(
-          key,
-          windowMs,
-        )
-
-        req.ctx = req.ctx || {}
-        req.ctx.rateLimit = {
-          limit: max,
-          used: totalHits,
-          remaining: Math.max(0, max - totalHits),
-          resetTime,
-          current: totalHits,
-          reset: resetTime,
-        }
-        req.rateLimit = {
-          limit: max,
-          remaining: Math.max(0, max - totalHits),
-          current: totalHits,
-          reset: resetTime,
-        }
+    // Set rate limit context
+    req.ctx = req.ctx || {}
+    req.ctx.rateLimit = {
+      limit: max,
+      used: totalHits,
+      remaining: Math.max(0, max - totalHits),
+      resetTime,
+      current: totalHits,
+      reset: resetTime,
+    }
+    req.rateLimit = {
+      limit: max,
+      remaining: Math.max(0, max - totalHits),
+      current: totalHits,
+      reset: resetTime,
+    }
 
-        const response = await next()
-        if (response instanceof Response) {
-          return addRateLimitHeaders(response, totalHits, resetTime)
-        }
-        return response
-      } catch (e) {
-        return next()
-      }
+    // Continue with request processing - let any errors bubble up
+    const response = await next()
+    if (response instanceof Response) {
+      return addRateLimitHeaders(response, totalHits, resetTime)
     }
+    return response
   }
 }
 
@@ -253,47 +221,43 @@ function createSlidingWindowRateLimit(options = {}) {
   const requests = new Map() // key -> array of timestamps
 
   return async function slidingWindowRateLimitMiddleware(req, next) {
-    try {
-      const key = await keyGenerator(req)
-      const now = Date.now()
-
-      // Get existing requests for this key
-      let userRequests = requests.get(key) || []
+    // Generate key and record the request immediately - let errors bubble up
+    const key = await keyGenerator(req)
+    const now = Date.now()
 
-      // Remove old requests outside the window
-      userRequests = userRequests.filter(
-        (timestamp) => now - timestamp < windowMs,
+    // Get existing requests for this key
+    let userRequests = requests.get(key) || []
+
+    // Remove old requests outside the window
+    userRequests = userRequests.filter(
+      (timestamp) => now - timestamp < windowMs,
+    )
+
+    // Check if limit exceeded
+    if (userRequests.length >= max) {
+      const response = await handler(
+        req,
+        userRequests.length,
+        max,
+        new Date(now + windowMs),
       )
+      return response
+    }
 
-      // Check if limit exceeded
-      if (userRequests.length >= max) {
-        const response = await handler(
-          req,
-          userRequests.length,
-          max,
-          new Date(now + windowMs),
-        )
-        return response
-      }
-
-      // Add current request
-      userRequests.push(now)
-      requests.set(key, userRequests)
-
-      // Add rate limit info to context
-      req.ctx = req.ctx || {}
-      req.ctx.rateLimit = {
-        limit: max,
-        used: userRequests.length,
-        remaining: max - userRequests.length,
-        resetTime: new Date(userRequests[0] + windowMs),
-      }
-
-      return next()
-    } catch (error) {
-      console.error('Sliding window rate limiting error:', error)
-      return next()
+    // Add current request
+    userRequests.push(now)
+    requests.set(key, userRequests)
+
+    // Add rate limit info to context
+    req.ctx = req.ctx || {}
+    req.ctx.rateLimit = {
+      limit: max,
+      used: userRequests.length,
+      remaining: max - userRequests.length,
+      resetTime: new Date(userRequests[0] + windowMs),
     }
+
+    return next()
   }
 }