-
Notifications
You must be signed in to change notification settings - Fork 309
Reverted sse support, as its breaking the existing flow #129
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -4,55 +4,40 @@ import { Server } from "@modelcontextprotocol/sdk/server/index.js"; | |||||||||||||||||||
import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; | ||||||||||||||||||||
import { createToolDefinitions } from "./tools.js"; | ||||||||||||||||||||
import { setupRequestHandlers } from "./requestHandler.js"; | ||||||||||||||||||||
import { SseServer } from './sseServer.js'; | ||||||||||||||||||||
import http from 'http'; | ||||||||||||||||||||
|
||||||||||||||||||||
export const SSE_SERVER_SYMBOL = Symbol('sseServer'); | ||||||||||||||||||||
|
||||||||||||||||||||
// Custom interface to allow symbol property | ||||||||||||||||||||
interface McpServerWithSSE extends Server { | ||||||||||||||||||||
[key: symbol]: any; | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
async function runServer() { | ||||||||||||||||||||
const server: McpServerWithSSE = new Server( | ||||||||||||||||||||
const server = new Server( | ||||||||||||||||||||
{ | ||||||||||||||||||||
name: "executeautomation/playwright-mcp-server", | ||||||||||||||||||||
version: "1.0.4", | ||||||||||||||||||||
version: "1.0.5", | ||||||||||||||||||||
}, | ||||||||||||||||||||
{ | ||||||||||||||||||||
capabilities: { | ||||||||||||||||||||
resources: {}, | ||||||||||||||||||||
tools: {}, | ||||||||||||||||||||
}, | ||||||||||||||||||||
} | ||||||||||||||||||||
) as McpServerWithSSE; | ||||||||||||||||||||
); | ||||||||||||||||||||
|
||||||||||||||||||||
// Create tool definitions | ||||||||||||||||||||
const TOOLS = createToolDefinitions(); | ||||||||||||||||||||
|
||||||||||||||||||||
// Setup request handlers | ||||||||||||||||||||
setupRequestHandlers(server, TOOLS); | ||||||||||||||||||||
|
||||||||||||||||||||
// Start HTTP server for SSE only if not in test environment | ||||||||||||||||||||
let sseServer; | ||||||||||||||||||||
if (process.env.NODE_ENV !== 'test') { | ||||||||||||||||||||
let httpServer; | ||||||||||||||||||||
try { | ||||||||||||||||||||
httpServer = http.createServer(); | ||||||||||||||||||||
sseServer = new SseServer(); | ||||||||||||||||||||
sseServer.attachToServer(httpServer); | ||||||||||||||||||||
httpServer.listen(3001, () => { | ||||||||||||||||||||
console.log('SSE server listening on http://localhost:3001/events'); | ||||||||||||||||||||
}); | ||||||||||||||||||||
} catch (err) { | ||||||||||||||||||||
console.error('Failed to initialize SSE server:', err); | ||||||||||||||||||||
} | ||||||||||||||||||||
if (sseServer) { | ||||||||||||||||||||
server[SSE_SERVER_SYMBOL] = sseServer; | ||||||||||||||||||||
} | ||||||||||||||||||||
// Graceful shutdown logic | ||||||||||||||||||||
function shutdown() { | ||||||||||||||||||||
console.log('Shutdown signal received'); | ||||||||||||||||||||
process.exit(0); | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
process.on('SIGINT', shutdown); | ||||||||||||||||||||
process.on('SIGTERM', shutdown); | ||||||||||||||||||||
process.on('exit', shutdown); | ||||||||||||||||||||
process.on('uncaughtException', (err) => { | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Description: The uncaughtException handler logs the error but doesn't terminate the process, potentially leaving it in an unstable state. Consider terminating the process after logging the uncaught exception. Severity: High There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix addresses the issue of potential instability after an uncaught exception by adding
Suggested change
|
||||||||||||||||||||
console.error('Uncaught Exception:', err); | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Description: Log Injection occurs when untrusted user input is directly written to log files without proper sanitization. This can allow attackers to manipulate log entries, potentially leading to security issues like log forging or cross-site scripting. To prevent this, always sanitize user input before logging by removing or encoding newline characters, using string encoding functions, and leveraging built-in sanitization features of logging libraries when available. Learn more - https://cwe.mitre.org/data/definitions/117.html Severity: High There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix involves using a sanitization function (sanitizeLog) to clean the error message before logging it, preventing potential log injection vulnerabilities.
Suggested change
|
||||||||||||||||||||
}); | ||||||||||||||||||||
|
||||||||||||||||||||
// Create transport and connect | ||||||||||||||||||||
const transport = new StdioServerTransport(); | ||||||||||||||||||||
await server.connect(transport); | ||||||||||||||||||||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description: The shutdown function is called for both graceful shutdown and process exit, which may lead to confusion. Consider separating the graceful shutdown logic from the process exit handling.
Severity: Medium
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fix separates the graceful shutdown logic from the process exit handling. A new
gracefulShutdown
function is introduced to handle SIGINT and SIGTERM signals, which initiates a graceful shutdown by disconnecting the server before exiting. ThehandleExit
function is added to handle the 'exit' event separately. This separation provides clearer distinction between graceful shutdown and immediate process exit scenarios.