Refactor transport event handlers based on code review feedback

Co-authored-by: samanhappy <2755122+samanhappy@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
2025-10-31 15:20:37 +00:00
parent 29c1b332b6
commit dc8ff4bedd
2 changed files with 19 additions and 13 deletions

View File

@@ -166,8 +166,11 @@ To manually test the fix:
## Compatibility ## Compatibility
- **Backwards Compatible**: No breaking changes to existing functionality - **Backwards Compatible**: No breaking changes to existing functionality
- **SDK Version**: Works with `@modelcontextprotocol/sdk` v1.20.2+ - **SDK Version**: Requires `@modelcontextprotocol/sdk` v1.20.2 or higher (current version in use)
- **Node.js**: Compatible with all supported Node.js versions - **Node.js**: Compatible with all supported Node.js versions
- **Transport Types**: Works with SSEClientTransport, StreamableHTTPClientTransport, and StdioClientTransport
Note: The `onclose` and `onerror` event handlers are part of the Transport interface in the MCP SDK and have been available since early versions. The current implementation has been tested with SDK v1.20.2.
## Future Enhancements ## Future Enhancements

View File

@@ -136,6 +136,15 @@ const setupKeepAlive = (serverInfo: ServerInfo, serverConfig: ServerConfig): voi
); );
}; };
// Helper function to clean up server resources on disconnection
const cleanupServerResources = (serverInfo: ServerInfo): void => {
// Clear keep-alive interval if it exists
if (serverInfo.keepAliveIntervalId) {
clearInterval(serverInfo.keepAliveIntervalId);
serverInfo.keepAliveIntervalId = undefined;
}
};
// Helper function to set up transport event handlers for connection monitoring // Helper function to set up transport event handlers for connection monitoring
const setupTransportEventHandlers = (serverInfo: ServerInfo): void => { const setupTransportEventHandlers = (serverInfo: ServerInfo): void => {
if (!serverInfo.transport) { if (!serverInfo.transport) {
@@ -145,31 +154,25 @@ const setupTransportEventHandlers = (serverInfo: ServerInfo): void => {
// Set up onclose handler to update status when connection closes // Set up onclose handler to update status when connection closes
serverInfo.transport.onclose = () => { serverInfo.transport.onclose = () => {
console.log(`Transport closed for server: ${serverInfo.name}`); console.log(`Transport closed for server: ${serverInfo.name}`);
if (serverInfo.status === 'connected') { // Update status to disconnected if not already in a terminal state
if (serverInfo.status === 'connected' || serverInfo.status === 'connecting') {
serverInfo.status = 'disconnected'; serverInfo.status = 'disconnected';
serverInfo.error = 'Connection closed'; serverInfo.error = 'Connection closed';
} }
// Clear keep-alive interval if it exists cleanupServerResources(serverInfo);
if (serverInfo.keepAliveIntervalId) {
clearInterval(serverInfo.keepAliveIntervalId);
serverInfo.keepAliveIntervalId = undefined;
}
}; };
// Set up onerror handler to update status on connection errors // Set up onerror handler to update status on connection errors
serverInfo.transport.onerror = (error: Error) => { serverInfo.transport.onerror = (error: Error) => {
console.error(`Transport error for server ${serverInfo.name}:`, error); console.error(`Transport error for server ${serverInfo.name}:`, error);
if (serverInfo.status === 'connected') { // Update status to disconnected if not already in a terminal state
if (serverInfo.status === 'connected' || serverInfo.status === 'connecting') {
serverInfo.status = 'disconnected'; serverInfo.status = 'disconnected';
serverInfo.error = `Transport error: ${error.message}`; serverInfo.error = `Transport error: ${error.message}`;
} }
// Clear keep-alive interval if it exists cleanupServerResources(serverInfo);
if (serverInfo.keepAliveIntervalId) {
clearInterval(serverInfo.keepAliveIntervalId);
serverInfo.keepAliveIntervalId = undefined;
}
}; };
console.log(`Transport event handlers set up for server: ${serverInfo.name}`); console.log(`Transport event handlers set up for server: ${serverInfo.name}`);