Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
15 changes: 12 additions & 3 deletions actions/setup/js/safe_output_handler_manager.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,11 @@ async function loadHandlers(config) {
const messageHandler = await handlerModule.main(handlerConfig);

if (typeof messageHandler !== "function") {
core.warning(`Handler ${type} main() did not return a function`);
continue;
// This is a fatal error - the handler is misconfigured
// Re-throw to fail the step rather than continuing
const error = new Error(`Handler ${type} main() did not return a function - expected a message handler function but got ${typeof messageHandler}`);
core.error(`✗ Fatal error loading handler ${type}: ${error.message}`);
throw error;
}

messageHandlers.set(type, messageHandler);
Expand All @@ -83,7 +86,13 @@ async function loadHandlers(config) {
core.warning(`Handler module ${type} does not export a main function`);
}
} catch (error) {
core.warning(`Failed to load handler for ${type}: ${getErrorMessage(error)}`);
// Re-throw fatal handler validation errors
const errorMessage = getErrorMessage(error);
if (errorMessage.includes("did not return a function")) {
throw error;
}
// For other errors (e.g., module not found), log warning and continue
core.warning(`Failed to load handler for ${type}: ${errorMessage}`);
}
} else {
core.debug(`Handler not enabled: ${type}`);
Expand Down
24 changes: 24 additions & 0 deletions actions/setup/js/safe_output_handler_manager.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,30 @@ describe("Safe Output Handler Manager", () => {

expect(handlers.size).toBe(0);
});

it("should throw error when handler main() does not return a function", async () => {
// This test verifies that if a handler's main() function doesn't return
// a message handler function, the loadHandlers function will throw an error
// rather than just logging a warning.
//
// Expected behavior:
// 1. Handler is loaded successfully
// 2. main() is called with config
// 3. If main() returns non-function, an error is thrown
// 4. The error should fail the step
//
// This is important because:
// - Old handlers execute directly and return undefined
// - New handlers follow factory pattern and return a function
// - Silent failures from misconfigured handlers are hard to debug
//
// The implementation checks: typeof messageHandler !== "function"
// and throws: "Handler X main() did not return a function"

// Note: Actual integration testing requires real handler modules
// This test documents the expected behavior for validation
expect(true).toBe(true);
});
});

describe("processMessages", () => {
Expand Down