From 521f7332fdbdf71b5e46088bb83adf9a185a0c0a Mon Sep 17 00:00:00 2001 From: sanish-bruno Date: Thu, 29 Jan 2026 17:47:15 +0530 Subject: [PATCH] refactor: simplify HookManager by removing error collection and related options --- packages/bruno-js/src/hook-manager.js | 137 ++----------------- packages/bruno-js/tests/hook-manager.spec.js | 36 ----- 2 files changed, 9 insertions(+), 164 deletions(-) diff --git a/packages/bruno-js/src/hook-manager.js b/packages/bruno-js/src/hook-manager.js index fe3451d05..bd794f879 100644 --- a/packages/bruno-js/src/hook-manager.js +++ b/packages/bruno-js/src/hook-manager.js @@ -1,22 +1,3 @@ -/** - * Error information from hook execution - * @typedef {Object} HookError - * @property {string} event - The event that was being processed - * @property {string} handlerName - Name of the handler that failed - * @property {string} message - Error message - * @property {string} [stack] - Error stack trace - * @property {Error} [originalError] - Original error object - */ - -/** - * Hook execution result with error aggregation - * @typedef {Object} HookCallResult - * @property {boolean} success - Whether all handlers completed successfully - * @property {number} handlersExecuted - Number of handlers that were executed - * @property {number} handlersFailed - Number of handlers that failed - * @property {Array} errors - Array of errors that occurred - */ - /** * HookManager provides a simple event system for registering and calling hooks (event listeners). * @@ -40,8 +21,7 @@ * * Error Handling: * - By default, errors in one handler don't stop other handlers from running - * - Errors are logged to console and optionally collected - * - Use the options parameter to customize error handling behavior + * - Errors are logged to console for debugging * * @class */ @@ -109,29 +89,17 @@ class HookManager { * Supports both sync and async handlers - all handlers are awaited * Wildcard handlers ('*') are called for every pattern, in addition to specific pattern handlers * - * Error Isolation: By default, errors in one handler don't stop other handlers. - * Use options.stopOnError = true to stop execution on first error. + * Error Isolation: Errors in one handler don't stop other handlers from running. + * Errors are logged to console but don't affect execution flow. * * @param {string|string[]} pattern - Event pattern(s) to trigger * @param {*} data - Data to pass to handlers - * @param {object} [options] - Call options - * @param {boolean} [options.stopOnError=false] - Stop execution on first error - * @param {boolean} [options.collectErrors=false] - Collect and return errors - * @param {function} [options.onError] - Callback for each error (receives HookError) - * @returns {Promise} Result with error info if collectErrors=true + * @returns {Promise} */ - async call(pattern, data, options = {}) { + async call(pattern, data) { // Validate state - but allow calls on disposed manager to fail gracefully if (this._state === HookManager.State.DISPOSED) { console.warn('HookManager.call() called on disposed instance'); - if (options.collectErrors) { - return { - success: false, - handlersExecuted: 0, - handlersFailed: 0, - errors: [{ event: String(pattern), message: 'HookManager has been disposed' }] - }; - } return; } @@ -139,11 +107,6 @@ class HookManager { throw new TypeError('Pattern must be a string or an array of strings.'); } - const { stopOnError = false, collectErrors = false, onError } = options; - const errors = []; - let handlersExecuted = 0; - let handlersFailed = 0; - const patternList = [].concat(pattern).map((d) => String(d).trim()); const hasWildcard = patternList.includes('*'); @@ -151,38 +114,16 @@ class HookManager { * Execute a single handler with error handling * @param {Function} handler - Handler to execute * @param {string} event - Event name - * @returns {Promise} True if should continue, false if should stop */ const executeHandler = async (handler, event) => { - handlersExecuted++; - const result = await callHandler(handler, data, event, { onError, collectErrors }); - - if (!result.success) { - handlersFailed++; - - if (collectErrors && result.error) { - errors.push(result.error); - } - - if (stopOnError) { - return false; // Signal to stop execution - } - } - - return true; // Continue execution + await callHandler(handler, data, event); }; if (hasWildcard) { for (const ptn of Object.keys(this.listeners)) { const handlers = this.listeners[ptn]; for (const handler of handlers) { - const shouldContinue = await executeHandler(handler, ptn); - if (!shouldContinue) { - if (collectErrors) { - return { success: false, handlersExecuted, handlersFailed, errors }; - } - return; - } + await executeHandler(handler, ptn); } } } else { @@ -190,25 +131,10 @@ class HookManager { for (const ptn of patternList) { if (!this.listeners[ptn]) continue; for (const handler of this.listeners[ptn]) { - const shouldContinue = await executeHandler(handler, ptn); - if (!shouldContinue) { - if (collectErrors) { - return { success: false, handlersExecuted, handlersFailed, errors }; - } - return; - } + await executeHandler(handler, ptn); } } } - - if (collectErrors) { - return { - success: handlersFailed === 0, - handlersExecuted, - handlersFailed, - errors - }; - } } /** @@ -314,13 +240,8 @@ class HookManager { * @param {Function} handler - Handler function to call * @param {*} data - Data to pass to handler * @param {string} event - Event name for error reporting - * @param {object} [options] - Options for error handling - * @param {function} [options.onError] - Callback for errors - * @param {boolean} [options.collectErrors] - Whether to collect error details - * @returns {Promise<{success: boolean, error?: HookError}>} Result object */ -async function callHandler(handler, data, event, options = {}) { - const { onError, collectErrors = false } = options; +async function callHandler(handler, data, event) { const handlerName = handler?.name || 'anonymous'; try { @@ -329,53 +250,13 @@ async function callHandler(handler, data, event, options = {}) { if (result && typeof result.then === 'function') { await result; } - return { success: true }; } catch (error) { - const errorInfo = { - event, - handlerName, - message: error?.message || String(error), - stack: error?.stack, - originalError: error - }; - // Log the error with context console.error( `[Hook Error] Event: '${event}', Handler: '${handlerName}'`, error?.message || error ); - - // Call error callback if provided - if (typeof onError === 'function') { - try { - onError(errorInfo); - } catch (callbackError) { - console.error('Error in onError callback:', callbackError); - } - } - - return { - success: false, - error: collectErrors ? errorInfo : undefined - }; } } -/** - * Create a HookError object from an error - * @param {string} event - Event name - * @param {string} handlerName - Handler name - * @param {Error} error - Original error - * @returns {HookError} Formatted error object - */ -function createHookError(event, handlerName, error) { - return { - event, - handlerName, - message: error?.message || String(error), - stack: error?.stack, - originalError: error - }; -} - module.exports = HookManager; diff --git a/packages/bruno-js/tests/hook-manager.spec.js b/packages/bruno-js/tests/hook-manager.spec.js index 6523c6bb3..03bc7100d 100644 --- a/packages/bruno-js/tests/hook-manager.spec.js +++ b/packages/bruno-js/tests/hook-manager.spec.js @@ -138,35 +138,6 @@ describe('HookManager', () => { expect(normalHandler).toHaveBeenCalled(); }); - it('should collect errors when collectErrors is true', async () => { - const errorHandler = jest.fn(() => { throw new Error('Test error'); }); - hookManager.on('test', errorHandler); - const result = await hookManager.call('test', {}, { collectErrors: true }); - expect(result.success).toBe(false); - expect(result.errors).toHaveLength(1); - expect(result.errors[0].message).toBe('Test error'); - }); - - it('should stop on first error when stopOnError is true', async () => { - const errorHandler = jest.fn(() => { throw new Error('Test error'); }); - const normalHandler = jest.fn(); - hookManager.on('test', errorHandler); - hookManager.on('test', normalHandler); - await hookManager.call('test', {}, { stopOnError: true }); - expect(normalHandler).not.toHaveBeenCalled(); - }); - - it('should call onError callback for each error', async () => { - const onError = jest.fn(); - const errorHandler = jest.fn(() => { throw new Error('Test error'); }); - hookManager.on('test', errorHandler); - await hookManager.call('test', {}, { onError }); - expect(onError).toHaveBeenCalledWith(expect.objectContaining({ - event: 'test', - message: 'Test error' - })); - }); - it('should warn when called on disposed instance', async () => { const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); hookManager.dispose(); @@ -174,13 +145,6 @@ describe('HookManager', () => { expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('disposed')); consoleSpy.mockRestore(); }); - - it('should return error result when disposed and collectErrors is true', async () => { - hookManager.dispose(); - const result = await hookManager.call('test', {}, { collectErrors: true }); - expect(result.success).toBe(false); - expect(result.errors[0].message).toContain('disposed'); - }); }); describe('clear()', () => {