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

Author: sjpotter

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);

packages/client/lib/client/index.spec.ts

@@ -9,7 +9,8 @@ import { MATH_FUNCTION, loadMathFunction } from '../commands/FUNCTION_LOAD.spec'
 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 +772,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'