fix: print creationStack error depending on localKeys value (#462)

Co-authored-by: Nathan Friedly <nathan@nfriedly.com>

docs/reference/changelog.mdx

@@ -9,7 +9,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to
 [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [7.3.0](https://github.com/express-rate-limit/express-rate-limit/releases/tag/v7.2.1)
+## [7.3.1](https://github.com/express-rate-limit/express-rate-limit/releases/tag/v7.3.1)
+
+### Fixed
+
+- Narrowed the scope of the `creationStack` validation check in order to prevent
+  false-positives
+
+## [7.3.0](https://github.com/express-rate-limit/express-rate-limit/releases/tag/v7.3.0)
 
 ### Added
 

docs/reference/error-codes.mdx

@@ -270,6 +270,13 @@ it is called at app initialization, not in response to each request:
 + app.use(rateLimitFactory()); // Works
 ```
 
+In certain advanced use cases, such as serving multiple websites from a single
+app, it may be necessary to dynamically create a rate limiter in response to a
+request. In this case the validation check may be disabled, but care should be
+taken to ensure everything works as expected. Consider caching the limiter after
+it's first usage to avoid repeating the initialization cost and any background
+work the store may require.
+
 Set `validate: {creationStack: false}` in the options to disable the check.
 
 ## Warnings

source/lib.ts

@@ -307,8 +307,8 @@ const rateLimit = (
 	const config = parseOptions(passedOptions ?? {})
 	const options = getOptionsFromConfig(config)
 
-	// The limiter shouldn't be created in response to a request
-	config.validations.creationStack()
+	// The limiter shouldn't be created in response to a request (usually)
+	config.validations.creationStack(config.store)
 	// The store instance shouldn't be shared across multiple limiters
 	config.validations.unsharedStore(config.store)
 

source/validations.ts

@@ -200,8 +200,8 @@ const validations = {
 	},
 
 	/**
-	 * Warns the user that the behaviour for `max: 0` / `limit: 0` is changing in the next
-	 * major release.
+	 * Warns the user that the behaviour for `max: 0` / `limit: 0` is
+	 * changing in the next major release.
 	 *
 	 * @param limit {number} - The maximum number of hits per client.
 	 *
@@ -234,8 +234,8 @@ const validations = {
 	},
 
 	/**
-	 * Warns the user that the `onLimitReached` option is deprecated and will be removed in the next
-	 * major release.
+	 * Warns the user that the `onLimitReached` option is deprecated and
+	 * will be removed in the next major release.
 	 *
 	 * @param onLimitReached {any | undefined} - The maximum number of hits per client.
 	 *
@@ -268,9 +268,11 @@ const validations = {
 	},
 
 	/**
-	 * Checks the options.validate setting to ensure that only recognized validations are enabled or disabled.
+	 * Checks the options.validate setting to ensure that only recognized
+	 * validations are enabled or disabled.
 	 *
-	 * If any unrecognized values are found, an error is logged that includes the list of supported vaidations.
+	 * If any unrecognized values are found, an error is logged that
+	 * includes the list of supported vaidations.
 	 */
 	validationsConfig() {
 		const supportedValidations = Object.keys(this).filter(
@@ -290,13 +292,29 @@ const validations = {
 	},
 
 	/**
-	 * Checks to see if the instance was created inside of a request handler, which would prevent it from working correctly.
+	 * Checks to see if the instance was created inside of a request handler,
+	 * which would prevent it from working correctly, with the default memory
+	 * store (or any other store with localKeys.)
 	 */
-	creationStack() {
+	creationStack(store: Store) {
 		const { stack } = new Error(
 			'express-rate-limit validation check (set options.validate.creationStack=false to disable)',
 		)
+
 		if (stack?.includes('Layer.handle [as handle_request]')) {
+			if (!store.localKeys) {
+				// This means the user is using an external store, which may be safe.
+				// Print out an error anyways, to alert them of the possibility that
+				// the rate limiter may not work as intended.
+
+				// See the discussion here: https://github.com/express-rate-limit/express-rate-limit/pull/461#discussion_r1626940562.
+				throw new ValidationError(
+					'ERR_ERL_CREATED_IN_REQUEST_HANDLER',
+					'express-rate-limit instance should *usually* be created at app initialization, not when responding to a request.',
+				)
+			}
+
+			// Otherwise, make sure they know not to do this.
 			throw new ValidationError(
 				'ERR_ERL_CREATED_IN_REQUEST_HANDLER',
 				`express-rate-limit instance should be created at app initialization, not when responding to a request.`,
@@ -308,8 +326,10 @@ const validations = {
 export type Validations = typeof validations
 
 /**
- * Creates a copy of the validations object where each method is wrapped to catch and log any thrown errors.
- * Sets `enabled` to the provided value, allowing different instances of express-rate-limit to have different validations settings.
+ * Creates a copy of the validations object where each method is
+ * wrapped to catch and log any thrown errors. Sets `enabled` to the
+ * provided value, allowing different instances of express-rate-limit
+ * to have different validations settings.
  *
  * @param enabled {boolean} - The list of enabled validations.
  *

test/library/validation-test.ts

@@ -13,6 +13,7 @@ import express from 'express'
 import supertest from 'supertest'
 import { getValidations } from '../../source/validations.js'
 import type { Store } from '../../source/types'
+import { MemoryStore } from '../../source/index.js'
 
 describe('validations tests', () => {
 	let validations = getValidations(true)
@@ -360,20 +361,42 @@ describe('validations tests', () => {
 	})
 
 	describe('creationStack', () => {
-		it('should log an error if called in an express request handler', async () => {
+		it('should log an error if called in an express request handler with a memory store', async () => {
 			const app = express()
+			const store = new MemoryStore()
+
 			app.get('/', (request, response) => {
-				validations.creationStack()
+				validations.creationStack(store)
 				response.send('hello')
 			})
+
 			await supertest(app).get('/').expect('hello')
 			expect(console.error).toHaveBeenCalledWith(
 				expect.objectContaining({ code: 'ERR_ERL_CREATED_IN_REQUEST_HANDLER' }),
 			)
 			expect(console.warn).not.toBeCalled()
 		})
+
+		it('should log a different error when used with an external store', async () => {
+			const app = express()
+			const store: Store = { localKeys: false } as any
+
+			app.get('/', (request, response) => {
+				validations.creationStack(store)
+				response.send('hello')
+			})
+
+			await supertest(app).get('/').expect('hello')
+			expect(console.error).toHaveBeenCalledWith(
+				expect.objectContaining({ code: 'ERR_ERL_CREATED_IN_REQUEST_HANDLER' }),
+			)
+			expect(console.warn).not.toBeCalled()
+		})
+
 		it('should not log an error if called elsewhere', async () => {
-			validations.creationStack()
+			const store = new MemoryStore()
+
+			validations.creationStack(store)
 			expect(console.error).not.toBeCalled()
 			expect(console.warn).not.toBeCalled()
 		})