mirror of
https://github.com/usebruno/bruno.git
synced 2026-06-11 09:51:30 +00:00
refactor: simplify HookManager by removing error collection and related options
This commit is contained in:
@@ -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<HookError>} 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<HookCallResult|void>} Result with error info if collectErrors=true
|
||||
* @returns {Promise<void>}
|
||||
*/
|
||||
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<boolean>} 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;
|
||||
|
||||
@@ -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()', () => {
|
||||
|
||||
Reference in New Issue
Block a user