feat: migrate HTTP server to Express and deprecate sse #861
Conversation
enesgules changed the title 
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the HTTP server from Node.js's native HTTP module to Express framework and removes the deprecated SSE (Server-Sent Events) transport support. The changes modernize the server implementation by using Express middleware, streamline the codebase by removing SSE-related code, and update documentation across multiple languages to reflect the new HTTP-only approach.
Key Changes
- Replaced native HTTP server with Express framework for better middleware support and cleaner request handling
- Removed SSE transport support and all related deprecation tracking code
- Implemented AsyncLocalStorage for cleaner context propagation of client IP and API key throughout request handling
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.ts | Migrated HTTP server to Express, removed SSE transport logic, implemented AsyncLocalStorage for request context, and refactored server initialization to use a singleton pattern |
| package.json | Added Express and its TypeScript definitions as dependencies |
| docs/README.zh-TW.md | Updated server URL from /sse to /mcp endpoint and removed SSE transport documentation |
| docs/README.zh-CN.md | Removed SSE transport examples and updated CLI flag documentation |
| docs/README.vi.md | Updated server URLs and removed SSE transport references |
| docs/README.uk.md | Updated server URLs and CLI documentation to remove SSE references |
| docs/README.pt-BR.md | Removed SSE connection examples and updated transport documentation |
| docs/README.ko.md | Updated server URLs and removed SSE transport CLI examples |
| docs/README.ja.md | Changed transport from SSE to HTTP in remote server connection example |
| docs/README.id-ID.md | Removed SSE transport documentation and updated CLI flags |
| README.md | Removed SSE deprecation warning and cleaned up formatting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Extract client IP address using socket remote address (most reliable) | ||
| const clientIp = getClientIp(req); | ||
| const apiKey = extractApiKey(req); | ||
| |
There was a problem hiding this comment.
The enableJsonResponse parameter should be documented or explained, as it's not clear what this option controls or when it should be enabled. Consider adding a comment explaining its purpose.
| // Enable JSON responses for HTTP transport. When set to true, the server will respond with JSON-formatted | |
| // data instead of other formats (e.g., streaming or binary). This should be enabled when clients expect | |
| // standard JSON responses (such as for RESTful API compatibility). |
| ); | ||
| }); | ||
| | ||
| httpServer.on("error", (err: NodeJS.ErrnoException) => { |
There was a problem hiding this comment.
The error handler is registered with .on() instead of .once(), which means the same error handler will be called for all subsequent errors. Since the handler calls startServer() recursively for EADDRINUSE, this could lead to infinite error loops if the server encounters an error after successful startup. Use .once('error', ...) instead.
| httpServer.on("error", (err: NodeJS.ErrnoException) => { | |
| httpServer.once("error", (err: NodeJS.ErrnoException) => { |
| }); | ||
| | ||
| res.on("close", () => { | ||
| transport.close(); |
There was a problem hiding this comment.
The transport is closed but the server connection is not closed. This could lead to resource leaks. Consider calling server.close() after transport.close() to properly clean up the server connection.
| transport.close(); | |
| transport.close(); | |
| server.close(); |
fixes ctx7-765