SYSTEST-9607- Event error message bug fix

CHANGELOG.md

@@ -2,6 +2,20 @@
 
 All notable changes to this project will be documented in this file.
 
+## [1.0.3] - 2023-11-30
+
+### Added
+
+* New unit test to validate updated error handling function and functional test update to capture updated error message
+
+### Changed
+
+* Refined `logErr()` and `fErr()` functions which handles event error messages to appropriately throw both event validation and event registration errors. 
+
+### Fixed
+
+* Fixed bug related to event error message handling wherein all event errors were previously thrown as an improper generic error which was misleading to users. Implemented a mechanism to correctly distinguish different types of event errors and throw them appropriately
+
 ## [1.0.2] - 2023-11-10
 
 ### Added

cli/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@firebolt-js/mock-firebolt-cli",
-  "version": "1.0.2",
+  "version": "1.0.3",
   "description": "Command-line interface for controlling the Mock Firebolt server",
   "main": "./src/cli.mjs",
   "scripts": {},

conduit/package.json

@@ -1,7 +1,7 @@
 {
   "name": "conduit",
   "description": "A TV app that works in concert with Mock Firebolt in Reverse Proxy mode",
-  "version": "1.0.2",
+  "version": "1.0.3",
   "private": true,
   "license": "Apache 2.0",
   "author": "Mike Fine <[email protected]>",

functional/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@firebolt-js/mock-firebolt-functional-tests",
-  "version": "1.0.2",
+  "version": "1.0.3",
   "description": "Command-line interface for controlling the Mock Firebolt server",
   "scripts": {
     "test": "NODE_ENV=test npx jest --config=jest.config.js --silent -i",

functional/tests/openRpcCall.test.mjs

@@ -211,7 +211,7 @@ test(`Validate send event without an active listener`, async () => {
   );
   expect(
     result.includes(
-      ` "message": "Could not send accessibility.onClosedCaptionsSettingsChanged event because no listener is active"`
+      `"message": "accessibility.onClosedCaptionsSettingsChanged event not registered"`
     )
   ).toBe(true);
 });

server/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@firebolt-js/mock-firebolt",
-  "version": "1.0.2",
+  "version": "1.0.3",
   "description": "Controllable mock Firebolt server",
   "main": "./build/index.mjs",
   "scripts": {

server/src/events.mjs

@@ -38,12 +38,19 @@ function logSuccess(onMethod, result, msg) {
   );
 };
 
-function logErr(onMethod) {
-  logger.info(
-    `Could not send ${onMethod} event because no listener is active`
-  );
-};
-
+function logErr(onMethod, eventErrorType) {
+  switch (eventErrorType) {
+    case 'validationError':
+      logger.info(`Event validation failed for ${onMethod}. Please ensure the event data meets the required format and try again`);
+      break;
+    case 'registrationError':
+      logger.info(`${onMethod} event not registered`);
+      break;
+    default:
+      logger.info(`Error of unknown error type occurred for ${onMethod} event (type: ${eventErrorType})`);
+      break;
+  }
+}
 function logFatalErr() {
   logger.info(`Internal error`);
 };
@@ -301,11 +308,12 @@ function coreSendEvent(isBroadcast, ws, userId, method, result, msg, fSuccess, f
   try {
     if (  ! isBroadcast && !isRegisteredEventListener(userId, method) ) {
       logger.info(`${method} event not registered`);
-      fErr.call(null, method);
+
+      fErr.call(null, 'registrationError', method);
 
     } else if ( isBroadcast && !isAnyRegisteredInGroup(userId, method) ){
       logger.info(`${method} event not registered`);
-      fErr.call(null, method);
+      fErr.call(null, 'registrationError', method);
 
     } else {
        // Fire pre trigger if there is one for this method
@@ -321,10 +329,10 @@ function coreSendEvent(isBroadcast, ws, userId, method, result, msg, fSuccess, f
               delete: function ds(key, scope) { return stateManagement.deleteScratch(userId, key, scope)},
               uuid: function cuuid() {return stateManagement.createUuid()},
               sendEvent: function(method, result, msg) {
-                sendEvent( ws, userId, method, result, msg, logSuccess.bind(this, method, result, msg), logErr.bind(this, method), logFatalErr.bind(this) );
+                sendEvent( ws, userId, method, result, msg, logSuccess.bind(this, method, result, msg), logErr.bind(this, method, null), logFatalErr.bind(this) );
               },
               sendBroadcastEvent: function(onMethod, result, msg) {
-                sendBroadcastEvent( ws, userId, onMethod, result, msg, logSuccess.bind(this, onMethod, result, msg), logErr.bind(this, onMethod), logFatalErr.bind(this) );
+                sendBroadcastEvent( ws, userId, onMethod, result, msg, logSuccess.bind(this, onMethod, result, msg), logErr.bind(this, onMethod, null), logFatalErr.bind(this) );
               }
             };
             logger.debug(`Calling pre trigger for event ${method}`);
@@ -351,10 +359,10 @@ function coreSendEvent(isBroadcast, ws, userId, method, result, msg, fSuccess, f
               delete: function ds(key, scope) { return stateManagement.deleteScratch(userId, key, scope)},
               uuid: function cuuid() {return stateManagement.createUuid()},
               sendEvent: function(method, result, msg) {
-                sendEvent( ws, userId, method, result, msg, logSuccess.bind(this, method, result, msg), logErr.bind(this, method), logFatalErr.bind(this) );
+                sendEvent( ws, userId, method, result, msg, logSuccess.bind(this, method, result, msg), logErr.bind(this, method, null), logFatalErr.bind(this) );
               },
               sendBroadcastEvent: function(onMethod, result, msg) {
-                sendBroadcastEvent( ws, userId, onMethod, result, msg, logSuccess.bind(this, onMethod, result, msg), logErr.bind(this, onMethod), logFatalErr.bind(this) );
+                sendBroadcastEvent( ws, userId, onMethod, result, msg, logSuccess.bind(this, onMethod, result, msg), logErr.bind(this, onMethod, null), logFatalErr.bind(this) );
               },
               ...response
             };
@@ -375,7 +383,7 @@ function coreSendEvent(isBroadcast, ws, userId, method, result, msg, fSuccess, f
       if( config.validate.includes("events") ) {
         const resultErrors = fireboltOpenRpc.validateMethodResult(finalResult, method);
         if ( resultErrors && resultErrors.length > 0 ) {
-          fErr.call(null, method);
+          fErr.call(null, 'validationError', method);
           return
         }
       }

server/src/messageHandler.mjs

@@ -37,8 +37,17 @@ const { dotConfig: { eventConfig } } = config;
 function fSuccess(msg, onMethod, result) {
   logger.info(`${msg}: Sent event ${onMethod} with result ${JSON.stringify(result)}`)
 }
-function fErr(onMethod) {
-  logger.info(`Could not send ${onMethod} event because no listener is active`)
+function fErr(onMethod, eventErrorType) {
+  switch (eventErrorType) {
+    case 'validationError':
+      logger.info(`Event validation failed for ${onMethod}. Please ensure the event data meets the required format and try again`);
+      break;
+    case 'registrationError':
+      logger.info(`${onMethod} event not registered`);
+      break;
+    default:
+      break;
+  }
 }
 function fFatalErr() {
   logger.info(`Internal error`)
@@ -173,10 +182,10 @@ async function handleMessage(message, userId, ws) {
           delete: function ds(key, scope) { return stateManagement.deleteScratch(userId, key, scope)},
           uuid: function cuuid() {return stateManagement.createUuid()},
           sendEvent: function (onMethod, result, msg) {
-            events.sendEvent(ws, userId, onMethod, result, msg, fSuccess.bind(this, msg, onMethod, result), fErr.bind(this, onMethod), fFatalErr.bind(this));
+            events.sendEvent(ws, userId, onMethod, result, msg, fSuccess.bind(this, msg, onMethod, result), fErr.bind(this, onMethod, null), fFatalErr.bind(this));
           },
           sendBroadcastEvent: function (onMethod, result, msg) {
-            events.sendBroadcastEvent(ws, userId, onMethod, result, msg, fSuccess.bind(this, msg, onMethod, result), fErr.bind(this, onMethod), fFatalErr.bind(this));
+            events.sendBroadcastEvent(ws, userId, onMethod, result, msg, fSuccess.bind(this, msg, onMethod, result), fErr.bind(this, onMethod, null), fFatalErr.bind(this));
           }
         };
         logger.debug(`Calling pre trigger for method ${oMsg.method}`);
@@ -278,10 +287,10 @@ async function handleMessage(message, userId, ws) {
           delete: function ds(key, scope) { return stateManagement.deleteScratch(userId, key, scope)},
           uuid: function cuuid() {return stateManagement.createUuid()},
           sendEvent: function (onMethod, result, msg) {
-            events.sendEvent(ws, userId, onMethod, result, msg, fSuccess.bind(this, msg, onMethod, result), fErr.bind(this, onMethod), fFatalErr.bind(this));
+            events.sendEvent(ws, userId, onMethod, result, msg, fSuccess.bind(this, msg, onMethod, result), fErr.bind(this, onMethod, null), fFatalErr.bind(this));
           },
           sendBroadcastEvent: function (onMethod, result, msg) {
-            events.sendBroadcastEvent(ws, userId, onMethod, result, msg, fSuccess.bind(this, msg, onMethod, result), fErr.bind(this, onMethod), fFatalErr.bind(this));
+            events.sendBroadcastEvent(ws, userId, onMethod, result, msg, fSuccess.bind(this, msg, onMethod, result), fErr.bind(this, onMethod, null), fFatalErr.bind(this));
           },
           ...response  // As returned either by the mock override or via Conduit from a real device
         };

server/src/routes/api/event.mjs

@@ -38,11 +38,15 @@ function sendEvent(req, res) {
     });
   }
 
-  function fErr(method) {
+  function fErr(method, eventErrorType) {
+    const errorCode = (eventErrorType === 'validationError') ? 'EVENT-VALIDATION-FAILED' : 'NO-EVENT-HANDLER-REGISTERED';
+    const message = (errorCode === 'EVENT-VALIDATION-FAILED') ?
+    `Event validation failed for ${method}. Please ensure the event data meets the required format and try again.` :
+    `${method} event not registered`
     res.status(400).send({
       status: 'ERROR',
-      errorCode: 'NO-EVENT-HANDLER-REGISTERED',
-      message: `Could not send ${method} event because no listener is active`
+      errorCode: errorCode,
+      message: message
     });
   }
 
@@ -55,7 +59,7 @@ function sendEvent(req, res) {
     });
   }
 
-  events.sendEvent(ws, userId, method, result, `${method}`, fSuccess, fErr, fFatalErr);
+  events.sendEvent(ws, userId, method, result, `${method}`, fSuccess,  fErr.bind(this, method, null), fFatalErr);
 }
 
 // POST /api/v1/broadcastEvent
@@ -71,11 +75,15 @@ function sendBroadcastEvent(req, res) {
     });
   }
 
-  function fErr(method) {
+  function fErr(method, eventErrorType) {
+    const errorCode = (eventErrorType === 'validationError') ? 'EVENT-VALIDATION-FAILED' : 'NO-EVENT-HANDLER-REGISTERED';
+    const message = (errorCode === 'EVENT-VALIDATION-FAILED') ?
+    `Event validation failed for ${method}. Please ensure the event data meets the required format and try again.` :
+    `${method} event not registered`
     res.status(400).send({
       status: 'ERROR',
-      errorCode: 'NO-EVENT-HANDLER-REGISTERED',
-      message: `Could not send ${method} event because no listener is active`
+      errorCode: errorCode,
+      message: message
     });
   }
 
@@ -88,7 +96,7 @@ function sendBroadcastEvent(req, res) {
     });
   }
 
-  events.sendBroadcastEvent(ws, userId, method, result, `${method}`, fSuccess, fErr, fFatalErr);
+  events.sendBroadcastEvent(ws, userId, method, result, `${method}`, fSuccess,  fErr.bind(this, method, null), fFatalErr);
 }
 
 // --- Exports ---

server/test/suite/events.test.mjs

@@ -295,10 +295,16 @@ test(`events.logSuccess works properly`, () => {
   expect(spy).toHaveBeenCalled();
 });
 
-test(`events.logErr works properly`, () => {
-  const spy = jest.spyOn(logger, "info");
-  events.logErr("dummyMethod");
-  expect(spy).toHaveBeenCalled();
+describe('events.logErr works properly', () => {
+  it('should handle validation error', () => {
+    events.logErr('methodName', 'validationError');
+    expect(logger.info).toHaveBeenCalledWith('Event validation failed for methodName. Please ensure the event data meets the required format and try again');
+  });
+
+  it('should handle registration error', () => {
+    events.logErr('unregisteredMethod', 'registrationError');
+    expect(logger.info).toHaveBeenCalledWith('unregisteredMethod event not registered');
+});
 });
 
 test(`events.logFatalErr works properly`, () => {