add test (/ fix code) to prevent overriding built in push handlers

redis · Apr 6, 2024 · eb10194 · eb10194
1 parent 3340d49
commit eb10194
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 4 deletions.
diff --git a/packages/client/lib/client/commands-queue.ts b/packages/client/lib/client/commands-queue.ts
@@ -42,6 +42,8 @@ const RESP2_PUSH_TYPE_MAPPING = {
   [RESP_TYPES.SIMPLE_STRING]: Buffer
 };
 
+export const pushHandlerError = 'Cannot override built in push message handler';
+
 export default class RedisCommandsQueue {
   readonly #respVersion;
   readonly #maxLength;
@@ -79,7 +81,7 @@ export default class RedisCommandsQueue {
 
     const s = new Set<string>();
     this.#builtInSet = s;
-    for (const str in this.#pushHandlers.keys) {
+    for (const str of this.#pushHandlers.keys()) {
       s.add(str);
     }
 
@@ -118,15 +120,15 @@ export default class RedisCommandsQueue {
 
   addPushHandler(messageType: string, handler: (pushMsg: Array<any>) => unknown) {
     if (this.#builtInSet.has(messageType)) {
-      throw new Error("Cannot override built in push message handler");
+      throw new Error(pushHandlerError);
     }
 
     this.#pushHandlers.set(messageType, handler);
   }
 
   removePushHandler(messageType: string) {
     if (this.#builtInSet.has(messageType)) {
-      throw new Error("Cannot override built in push message handler");
+      throw new Error(pushHandlerError);
     }
 
     this.#pushHandlers.delete(messageType);

diff --git a/packages/client/lib/client/index.spec.ts b/packages/client/lib/client/index.spec.ts
@@ -10,6 +10,8 @@ import { RESP_TYPES } from '../RESP/decoder';
 import { BlobStringReply, NumberReply } from '../RESP/types';
 import { SortedSetMember } from '../commands/generic-transformers';
 import { createClient } from '../..';
+import { COMMANDS, PUBSUB_TYPE } from './pub-sub';
+import { pushHandlerError } from './commands-queue';
 
 export const SQUARE_SCRIPT = defineScript({
   SCRIPT:
@@ -771,7 +773,14 @@ describe('Client', () => {
     }, GLOBAL.SERVERS.OPEN);
   });
 
-  describe('Push Handlers', () => {
+  describe.only('Push Handlers', () => {
+    testUtils.testWithClient('prevent overriding a built in handler', async client => {
+      assert.throws(() => {client.addPushHandler(COMMANDS[PUBSUB_TYPE.CHANNELS].message.toString(), (push: Array<any>) => {})}, new Error(pushHandlerError));
+      assert.throws(() => {client.removePushHandler(COMMANDS[PUBSUB_TYPE.CHANNELS].message.toString())}, new Error(pushHandlerError));
+    }, {
+      ...GLOBAL.SERVERS.OPEN
+    });
+
     testUtils.testWithClient('RESP2: add/remove invalidate handler, and validate its called', async client => {
       const key = 'x'