Skip to content

Commit a0ef753

Browse files
committed
fix: phantom ping issue
1 parent 018567f commit a0ef753

File tree

3 files changed

+4
-124
lines changed

3 files changed

+4
-124
lines changed

src/transports/http/server.ts

Lines changed: 2 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -20,21 +20,13 @@ export class HttpStreamTransport extends AbstractTransport {
2020
private _serverConfig: any;
2121
private _serverSetupCallback?: (server: McpServer) => Promise<void>;
2222

23-
private _pingInterval?: NodeJS.Timeout;
24-
private _pingTimeouts: Map<string | number, NodeJS.Timeout> = new Map();
25-
private _pingFrequency: number;
26-
private _pingTimeout: number;
27-
2823
constructor(config: HttpStreamTransportConfig = {}) {
2924
super();
3025

3126
this._port = config.port || 8080;
3227
this._endpoint = config.endpoint || '/mcp';
3328
this._enableJsonResponse = config.responseMode === 'batch';
3429

35-
this._pingFrequency = config.ping?.frequency ?? 30000;
36-
this._pingTimeout = config.ping?.timeout ?? 10000;
37-
3830
logger.debug(
3931
`HttpStreamTransport configured with: ${JSON.stringify({
4032
port: this._port,
@@ -44,10 +36,6 @@ export class HttpStreamTransport extends AbstractTransport {
4436
maxMessageSize: config.maxMessageSize,
4537
auth: config.auth ? true : false,
4638
cors: config.cors ? true : false,
47-
ping: {
48-
frequency: this._pingFrequency,
49-
timeout: this._pingTimeout,
50-
},
5139
})}`
5240
);
5341
}
@@ -97,7 +85,6 @@ export class HttpStreamTransport extends AbstractTransport {
9785
this._server.listen(this._port, () => {
9886
logger.info(`HTTP server listening on port ${this._port}, endpoint ${this._endpoint}`);
9987
this._isRunning = true;
100-
this.startPingInterval();
10188
resolve();
10289
});
10390
});
@@ -202,52 +189,10 @@ export class HttpStreamTransport extends AbstractTransport {
202189
);
203190
}
204191

205-
private startPingInterval(): void {
206-
if (this._pingFrequency > 0) {
207-
logger.debug(
208-
`Starting ping interval with frequency ${this._pingFrequency}ms and timeout ${this._pingTimeout}ms`
209-
);
210-
this._pingInterval = setInterval(() => this.sendPing(), this._pingFrequency);
211-
}
212-
}
213-
214-
private async sendPing(): Promise<void> {
215-
if (!this._isRunning || Object.keys(this._transports).length === 0) {
216-
return;
217-
}
218-
219-
const pingId = `ping-${Date.now()}`;
220-
const pingRequest: JSONRPCMessage = {
221-
jsonrpc: '2.0' as const,
222-
id: pingId,
223-
method: 'ping',
224-
};
225-
226-
logger.debug(
227-
`Broadcasting ping to ${Object.keys(this._transports).length} sessions: ${JSON.stringify(pingRequest)}`
228-
);
229-
230-
for (const [sessionId, transport] of Object.entries(this._transports)) {
231-
try {
232-
await transport.send(pingRequest);
233-
234-
const timeoutId = setTimeout(() => {
235-
logger.warn(`Ping timeout for session ${sessionId}`);
236-
delete this._transports[sessionId];
237-
this._pingTimeouts.delete(pingId);
238-
}, this._pingTimeout);
239-
240-
this._pingTimeouts.set(pingId, timeoutId);
241-
} catch (error) {
242-
logger.error(`Error sending ping to session ${sessionId}: ${error}`);
243-
delete this._transports[sessionId];
244-
}
245-
}
246-
}
247-
248192
async send(message: JSONRPCMessage): Promise<void> {
249193
if (!this._isRunning) {
250-
throw new Error('HttpStreamTransport not running');
194+
logger.warn('Attempted to send message, but HTTP transport is not running');
195+
return;
251196
}
252197

253198
const activeSessions = Object.entries(this._transports);
@@ -282,16 +227,6 @@ export class HttpStreamTransport extends AbstractTransport {
282227
return;
283228
}
284229

285-
if (this._pingInterval) {
286-
clearInterval(this._pingInterval);
287-
this._pingInterval = undefined;
288-
}
289-
290-
for (const timeout of this._pingTimeouts.values()) {
291-
clearTimeout(timeout);
292-
}
293-
this._pingTimeouts.clear();
294-
295230
for (const transport of Object.values(this._transports)) {
296231
try {
297232
await transport.close();

src/transports/http/types.ts

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -61,24 +61,6 @@ export interface HttpStreamTransportConfig {
6161
*/
6262
endpoint?: string;
6363

64-
/**
65-
* Configure ping mechanism for connection health verification
66-
*/
67-
ping?: {
68-
/**
69-
* Interval in milliseconds for sending ping requests
70-
* Set to 0 to disable pings
71-
* Default: 30000 (30 seconds)
72-
*/
73-
frequency?: number;
74-
75-
/**
76-
* Timeout in milliseconds for waiting for a ping response
77-
* Default: 10000 (10 seconds)
78-
*/
79-
timeout?: number;
80-
};
81-
8264
/**
8365
* Response mode: stream (Server-Sent Events) or batch (JSON)
8466
* Defaults to 'stream'
@@ -127,8 +109,4 @@ export const DEFAULT_HTTP_STREAM_CONFIG: HttpStreamTransportConfig = {
127109
batchTimeout: 30000,
128110
maxMessageSize: 4 * 1024 * 1024,
129111
session: DEFAULT_SESSION_CONFIG,
130-
ping: {
131-
frequency: 30000,
132-
timeout: 10000,
133-
},
134112
};

tests/transports/http/server.test.ts

Lines changed: 2 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,6 @@ describe('HttpStreamTransport', () => {
3434
port: 9999,
3535
endpoint: '/custom-mcp',
3636
responseMode: 'batch',
37-
ping: {
38-
frequency: 60000,
39-
timeout: 15000,
40-
},
4137
});
4238

4339
expect(customTransport).toBeDefined();
@@ -117,32 +113,6 @@ describe('HttpStreamTransport', () => {
117113
});
118114

119115
describe('Configuration Validation', () => {
120-
it('should accept valid ping configuration', () => {
121-
const transportWithPing = new HttpStreamTransport({
122-
port: 8080,
123-
ping: {
124-
frequency: 30000,
125-
timeout: 10000,
126-
},
127-
});
128-
129-
expect(transportWithPing).toBeDefined();
130-
expect(transportWithPing.type).toBe('http-stream');
131-
});
132-
133-
it('should accept disabled ping configuration', () => {
134-
const transportNoPing = new HttpStreamTransport({
135-
port: 8080,
136-
ping: {
137-
frequency: 0, // Disabled
138-
timeout: 10000,
139-
},
140-
});
141-
142-
expect(transportNoPing).toBeDefined();
143-
expect(transportNoPing.type).toBe('http-stream');
144-
});
145-
146116
it('should handle different response modes', () => {
147117
const streamTransport = new HttpStreamTransport({
148118
responseMode: 'stream',
@@ -170,8 +140,7 @@ describe('HttpStreamTransport', () => {
170140

171141
it('should support proper session management structure', () => {
172142
const serverConfig = { name: 'multi-client-server', version: '1.0.0' };
173-
const setupCallback = async () => {
174-
};
143+
const setupCallback = async () => {};
175144

176145
// Should accept configuration for multi-client support
177146
expect(() => transport.setServerConfig(serverConfig, setupCallback)).not.toThrow();
@@ -226,14 +195,12 @@ describe('HttpStreamTransport', () => {
226195

227196
describe('Integration with Framework Pattern', () => {
228197
it('should follow the multi-session architecture', () => {
229-
230198
const serverConfig = {
231199
name: 'http-stream-server',
232200
version: '1.0.0',
233201
};
234202

235-
const setupCallback = async () => {
236-
};
203+
const setupCallback = async () => {};
237204

238205
// Should accept the configuration pattern used by working examples
239206
expect(() => transport.setServerConfig(serverConfig, setupCallback)).not.toThrow();

0 commit comments

Comments
 (0)