Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SYSTEST-9607- Event error message bug fix #148

Merged
merged 10 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cli/package.json
Original file line number Diff line number Diff line change
@@ -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": {},
Expand Down
2 changes: 1 addition & 1 deletion conduit/package.json
Original file line number Diff line number Diff line change
@@ -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]>",
Expand Down
2 changes: 1 addition & 1 deletion functional/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
2 changes: 1 addition & 1 deletion functional/tests/openRpcCall.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
2 changes: 1 addition & 1 deletion server/package.json
Original file line number Diff line number Diff line change
@@ -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": {
Expand Down
17 changes: 11 additions & 6 deletions server/src/events.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { config } from './config.mjs';
import { updateCallWithResponse } from './sessionManagement.mjs';

const { dotConfig: { eventConfig } } = config;
let eventErrorType;

function logSuccess(onMethod, result, msg) {
logger.info(
Expand All @@ -39,11 +40,12 @@ function logSuccess(onMethod, result, msg) {
};

function logErr(onMethod) {
logger.info(
`Could not send ${onMethod} event because no listener is active`
);
};

if (eventErrorType == 'validationError') {
Nandana-NNR marked this conversation as resolved.
Show resolved Hide resolved
Nandana-NNR marked this conversation as resolved.
Show resolved Hide resolved
logger.info(`Event validation failed for ${onMethod}. Please ensure the event data meets the required format and try again`);
} else if (eventErrorType == 'registrationError') {
logger.info(`${onMethod} event not registered`);
}
}
function logFatalErr() {
logger.info(`Internal error`);
};
Expand Down Expand Up @@ -301,10 +303,12 @@ function coreSendEvent(isBroadcast, ws, userId, method, result, msg, fSuccess, f
try {
if ( ! isBroadcast && !isRegisteredEventListener(userId, method) ) {
logger.info(`${method} event not registered`);
eventErrorType = 'registrationError'
fErr.call(null, method);
Nandana-NNR marked this conversation as resolved.
Show resolved Hide resolved

} else if ( isBroadcast && !isAnyRegisteredInGroup(userId, method) ){
logger.info(`${method} event not registered`);
eventErrorType = 'registrationError'
Nandana-NNR marked this conversation as resolved.
Show resolved Hide resolved
fErr.call(null, method);

} else {
Expand Down Expand Up @@ -375,6 +379,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 ) {
eventErrorType = 'validationError'
fErr.call(null, method);
return
}
Expand Down Expand Up @@ -427,5 +432,5 @@ export {
isEventListenerOnMessage, isEventListenerOffMessage,
sendEventListenerAck, sendUnRegistrationAck,
sendEvent, sendBroadcastEvent, logSuccess, logErr,
logFatalErr, extractEventData
logFatalErr, extractEventData, eventErrorType
};
6 changes: 5 additions & 1 deletion server/src/messageHandler.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ 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`)
if (events.eventErrorType == 'validationError') {
Nandana-NNR marked this conversation as resolved.
Show resolved Hide resolved
logger.info(`Event validation failed for ${onMethod}. Please ensure the event data meets the required format and try again`);
} else if (events.eventErrorType == 'registrationError') {
logger.info(`${onMethod} event not registered`);
}
}
function fFatalErr() {
logger.info(`Internal error`)
Expand Down
14 changes: 10 additions & 4 deletions server/src/routes/api/event.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,13 @@ function sendEvent(req, res) {
}

function fErr(method) {
const errorCode = (events.eventErrorType === 'validationError') ? 'EVENT-VALIDATION-FAILED' : 'NO-EVENT-HANDLER-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: (errorCode === 'EVENT-VALIDATION-FAILED') ?
`Event validation failed for ${method}. Please ensure the event data meets the required format and try again.` :
Nandana-NNR marked this conversation as resolved.
Show resolved Hide resolved
`${method} event not registered`
});
}

Expand Down Expand Up @@ -72,10 +75,13 @@ function sendBroadcastEvent(req, res) {
}

function fErr(method) {
const errorCode = (events.eventErrorType === 'validationError') ? 'EVENT-VALIDATION-FAILED' : 'NO-EVENT-HANDLER-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: (errorCode === 'EVENT-VALIDATION-FAILED') ?
`Event validation failed for ${method}. Please ensure the event data meets the required format and try again.` :
Nandana-NNR marked this conversation as resolved.
Show resolved Hide resolved
`${method} event not registered`
});
}

Expand Down
14 changes: 10 additions & 4 deletions server/test/suite/events.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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`, () => {
Expand Down
Loading