-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Open
Labels
bugSomething isn't workingSomething isn't working
Description
Summary
The TypeScript SDK violates the MCP specification by not implementing logging for cancellation reasons, which is explicitly required for debugging purposes.
Specification Violation
According to the MCP specification, cancellation notifications should include logging requirements:
Receivers of cancellation notifications SHOULD:
Both parties SHOULD log cancellation reasons for debugging
Current Implementation Issue
Location
File: src/shared/protocol.ts
Functions:
Protocol.request()
method -cancel
function (lines 582-601)Protocol
constructor - cancellation notification handler (lines 231-236)
Architecture Context
Protocol
is a base class inherited by bothClient
andServer
- Both sending and receiving cancellation notifications happen in the shared base class
- The specification requires both parties to log cancellation reasons
Problem
The current implementation lacks logging for cancellation reasons on both sides:
1. Sender Side (cancel function)
const cancel = (reason: unknown) => {
this._responseHandlers.delete(messageId);
this._progressHandlers.delete(messageId);
this._cleanupTimeout(messageId);
this._transport?.send({
jsonrpc: "2.0",
method: "notifications/cancelled",
params: {
requestId: messageId,
reason: String(reason), // ✅ Reason is sent
},
});
// ❌ Missing: No logging of why the request was cancelled
};
2. Receiver Side (notification handler)
this.setNotificationHandler(CancelledNotificationSchema, (notification) => {
const controller = this._requestHandlerAbortControllers.get(
notification.params.requestId,
);
controller?.abort(notification.params.reason);
// ❌ Missing: No logging of received cancellation notification
});
Impact
- Specification Compliance: Violates explicit MCP requirement for logging cancellation reasons
- Debugging Difficulty: No visibility into why requests are being cancelled
- Troubleshooting: Difficult to diagnose cancellation-related issues in production
- Monitoring: No audit trail for cancellation events
Proposed Fix
1. Sender Side Logging
Add logging in the cancel
function:
const cancel = (reason: unknown) => {
// Log cancellation reason for debugging
console.log(`[${this.constructor.name}] Cancelling request ${messageId}: ${String(reason)}`);
this._responseHandlers.delete(messageId);
this._progressHandlers.delete(messageId);
this._cleanupTimeout(messageId);
this._transport?.send({
jsonrpc: "2.0",
method: "notifications/cancelled",
params: {
requestId: messageId,
reason: String(reason),
},
});
};
2. Receiver Side Logging
Add logging in the notification handler:
this.setNotificationHandler(CancelledNotificationSchema, (notification) => {
// Log received cancellation notification for debugging
console.log(`[${this.constructor.name}] Received cancellation for request ${notification.params.requestId}: ${notification.params.reason || 'No reason provided'}`);
const controller = this._requestHandlerAbortControllers.get(
notification.params.requestId,
);
controller?.abort(notification.params.reason);
});
3. Alternative: Configurable Logging
Make logging configurable to avoid console spam in production:
constructor(private _options?: ProtocolOptions) {
// Add logging option
const enableCancellationLogging = this._options?.enableCancellationLogging ?? true;
this.setNotificationHandler(CancelledNotificationSchema, (notification) => {
if (enableCancellationLogging) {
console.log(`[${this.constructor.name}] Received cancellation for request ${notification.params.requestId}: ${notification.params.reason || 'No reason provided'}`);
}
// ... rest of handler
});
}
Testing
The fix should be tested with:
- Client cancelling requests (should log cancellation reason)
- Server cancelling requests (should log cancellation reason)
- Client receiving cancellation notifications (should log received cancellation)
- Server receiving cancellation notifications (should log received cancellation)
- Different cancellation reasons (timeout, manual abort, error conditions)
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't working