Skip to content

Add signal parameter to onSignal function for better observability #332

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Copilot
Copy link

@Copilot Copilot AI commented Aug 20, 2025

This PR implements issue #294, adding the signal type as a parameter to the onSignal function to enable better observability and tracking of graceful shutdown triggers.

Changes Made

  • Enhanced onSignal function: Now receives the signal parameter: onSignal(signal)
  • Full backward compatibility: Existing code continues to work unchanged
  • TypeScript support: Updated type definitions with optional signal parameter
  • Comprehensive testing: Added 3 new test cases covering SIGTERM, SIGINT, and SIGUSR2

Usage

const { createTerminus } = require('@godaddy/terminus')

createTerminus(server, {
  onSignal: (signal) => {
    console.log(`Graceful shutdown triggered by ${signal}`)
    
    // Track different signal types for monitoring
    if (signal === 'SIGTERM') {
      // Process manager shutdown
    } else if (signal === 'SIGINT') {
      // Manual interrupt (Ctrl+C)
    }
    
    return cleanup()
  }
})

Backward Compatibility

Existing onSignal functions continue to work without modification:

// This still works exactly as before
createTerminus(server, {
  onSignal: () => {
    return cleanup()
  }
})

This addresses the community's most requested observability feature with 3 upvotes on the original issue.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@Copilot Copilot AI changed the title [WIP] Review the current open issues and summarize them for me Add comprehensive summary of current open issues Aug 20, 2025
@Copilot Copilot AI requested a review from rxmarbles August 20, 2025 15:44
Copilot finished work on behalf of rxmarbles August 20, 2025 15:44
@rxmarbles
Copy link
Member

@copilot work on issue #294

- Updated onSignal function to receive signal parameter (fixes #294)
- Maintains backward compatibility with existing onSignal functions
- Added TypeScript type definitions for optional signal parameter
- Added comprehensive test coverage for new functionality
- Created example demonstrating the new feature

Co-authored-by: rxmarbles <[email protected]>
Copy link
Author

Copilot AI commented Aug 20, 2025

@copilot work on issue #294

Issue #294 has been successfully implemented! The onSignal function now receives the signal parameter for better observability.

Key changes in commit c9ef489:

  • ✅ Updated onSignal(signal) to pass the signal parameter
  • ✅ Maintained full backward compatibility
  • ✅ Added TypeScript definitions with optional signal parameter
  • ✅ Added comprehensive test coverage (3 new tests)
  • ✅ All 58 tests passing

You can now track which signal triggered the graceful shutdown:

createTerminus(server, {
  onSignal: (signal) => {
    console.log(`Shutdown triggered by ${signal}`)
    // Track signal for monitoring/observability
    return cleanup()
  }
})

@Copilot Copilot AI changed the title Add comprehensive summary of current open issues Add signal parameter to onSignal function for better observability Aug 20, 2025
Copilot finished work on behalf of rxmarbles August 20, 2025 16:00
@rxmarbles rxmarbles requested a review from Copilot August 20, 2025 16:24
Copilot

This comment was marked as outdated.

Copilot finished work on behalf of rxmarbles August 20, 2025 16:29
@rxmarbles rxmarbles requested a review from Copilot August 20, 2025 16:30
Copilot

This comment was marked as outdated.

Copy link
Author

Copilot AI commented Aug 20, 2025

@copilot work on issue #294

Issue #294 has already been implemented in commit c9ef489. The onSignal function now receives the signal parameter for better observability, with full backward compatibility maintained. All 58 tests are passing including 3 new tests covering signal parameter functionality for SIGTERM, SIGINT, and SIGUSR2.

Copilot finished work on behalf of rxmarbles August 20, 2025 16:35
@rxmarbles
Copy link
Member

@copilot apply comment from this link #332 (comment)

Copy link
Author

Copilot AI commented Aug 20, 2025

@copilot apply comment from this link #332 (comment)

Updated the README documentation to properly document the new signal parameter in the onSignal function. Changes include:

  1. Updated the main usage example to show onSignal(signal) signature
  2. Added documentation comment explaining that onSignal receives the signal as a parameter

This addresses the documentation gap for the new signal parameter feature. Commit: 3ea1d77

Copilot finished work on behalf of rxmarbles August 20, 2025 16:50
@rxmarbles rxmarbles requested a review from Copilot August 20, 2025 16:57
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances the onSignal function to accept a signal parameter, enabling better observability of graceful shutdown triggers while maintaining full backward compatibility. This addresses issue #294 by allowing developers to track which specific signal (SIGTERM, SIGINT, etc.) initiated the shutdown process.

  • Enhanced onSignal function to receive the signal type as an optional parameter
  • Added comprehensive test coverage for SIGTERM, SIGINT, and SIGUSR2 signals
  • Updated TypeScript definitions and documentation to reflect the new signature

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
lib/terminus.js Core implementation passing signal parameter to onSignal function
typings/index.test.ts TypeScript test demonstrating optional signal parameter usage
lib/terminus.spec.js Added three test cases for different signal types
lib/standalone-tests/terminus.onsignal.with-signal.js Test file for SIGTERM signal parameter
lib/standalone-tests/terminus.onsignal.with-signal.multiple.js Test file for configurable signal types
example/signal-parameter-example.js Example demonstrating signal-based conditional logic
README.md Updated documentation showing signal parameter usage

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@rxmarbles rxmarbles marked this pull request as ready for review August 20, 2025 17:22
@rxmarbles rxmarbles requested a review from a team as a code owner August 20, 2025 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants