diff --git a/README.md b/README.md index 4f07df864..8e2920567 100644 --- a/README.md +++ b/README.md @@ -415,6 +415,15 @@ npx @modelcontextprotocol/inspector --cli node build/index.js # With config file npx @modelcontextprotocol/inspector --cli --config path/to/config.json --server myserver +# With request timeout configs (in milliseconds) - request timeout +npx modelcontextprotocol/inspector --cli node build/index.js --method tools/call --tool-name longRunningOperation --tool-arg duration=5 steps=5 --request-timeout 15000 + +# With request timeout configs (in milliseconds) - reset timeout on progress +npx modelcontextprotocol/inspector --cli node build/index.js --method tools/call --tool-name longRunningOperation --tool-arg duration=15 steps=5 --reset-timeout-on-progress true + +# With request timeout configs (in milliseconds) - max total timeout (used with request timeout and reset timeout on progress) +npx modelcontextprotocol/inspector --cli node build/index.js --method tools/call --tool-name longRunningOperation --tool-arg duration=35 steps=5 --reset-timeout-on-progress true --request-timeout 7000 --max-total-timeout 35000 + # List available tools npx @modelcontextprotocol/inspector --cli node build/index.js --method tools/list diff --git a/cli/scripts/cli-tests.js b/cli/scripts/cli-tests.js index 554a5262e..fa6a7439f 100755 --- a/cli/scripts/cli-tests.js +++ b/cli/scripts/cli-tests.js @@ -48,6 +48,9 @@ console.log( console.log( `${colors.BLUE}- Transport inference from URL suffixes (/mcp, /sse)${colors.NC}`, ); +console.log( + `${colors.BLUE}- Request timeout configuration options${colors.NC}`, +); console.log(`\n`); // Get directory paths @@ -117,6 +120,83 @@ try { const invalidConfigPath = path.join(TEMP_DIR, "invalid-config.json"); fs.writeFileSync(invalidConfigPath, '{\n "mcpServers": {\n "invalid": {'); +// Helper function to analyze output for timeout behavior verification +function analyzeTimeoutBehavior(output, testName, expectedBehavior) { + const lines = output.split("\n"); + + console.log( + `${colors.BLUE}Analyzing timeout behavior for ${testName}...${colors.NC}`, + ); + + // Extract key timing information + const progressUpdates = lines.filter((line) => + line.includes("Progress update received"), + ); + const timeoutMessages = lines.filter( + (line) => line.includes("timeout") || line.includes("timed out"), + ); + const errorMessages = lines.filter( + (line) => line.includes("Error") || line.includes("Failed"), + ); + + console.log( + `${colors.BLUE}Found ${progressUpdates.length} progress updates${colors.NC}`, + ); + + if (progressUpdates.length > 0) { + console.log( + `${colors.BLUE}First progress update: ${progressUpdates[0]}${colors.NC}`, + ); + if (progressUpdates.length > 1) { + console.log( + `${colors.BLUE}Last progress update: ${progressUpdates[progressUpdates.length - 1]}${colors.NC}`, + ); + } + } + + if (timeoutMessages.length > 0) { + console.log( + `${colors.BLUE}Timeout messages: ${timeoutMessages.join("\n")}${colors.NC}`, + ); + } + + if (errorMessages.length > 0) { + console.log( + `${colors.BLUE}Error messages: ${errorMessages.join("\n")}${colors.NC}`, + ); + } + + switch (expectedBehavior) { + case "should_reset_timeout": + // For tests where resetTimeoutOnProgress is true, we expect: + // 1. Multiple progress updates (more than 2) + // 2. No timeout errors before completion + // 3. Successful completion + return ( + progressUpdates.length >= 3 && + !output.includes("Request timed out") && + !output.includes("Maximum total timeout exceeded") + ); + + case "should_timeout_quickly": + // For tests where resetTimeoutOnProgress is false, we expect: + // 1. Few progress updates (less than 3) + // 2. A timeout error + return ( + progressUpdates.length < 3 && + (output.includes("Request timed out") || output.includes("timed out")) + ); + + case "should_hit_max_timeout": + // For tests where maxTotalTimeout should be hit, we expect: + // 1. Error mentioning maximum total timeout + return output.includes("Maximum total timeout exceeded"); + + default: + return null; // No specific expectation + } +} + // Create config files with different transport types for testing const sseConfigPath = path.join(TEMP_DIR, "sse-config.json"); fs.writeFileSync( @@ -197,7 +277,7 @@ fs.writeFileSync( ); // Function to run a basic test -async function runBasicTest(testName, ...args) { +async function runBasicTest(testName, expectedBehavior, ...args) { const outputFile = path.join( OUTPUT_DIR, `${testName.replace(/\//g, "_")}.log`, @@ -243,6 +323,36 @@ async function runBasicTest(testName, ...args) { clearTimeout(timeout); outputStream.end(); + // For specific timeout behavior tests, validate the behavior + if (expectedBehavior) { + const behaviorCorrect = analyzeTimeoutBehavior( + output, + testName, + expectedBehavior, + ); + + if (behaviorCorrect === true) { + console.log( + `${colors.GREEN}✓ Timeout behavior test passed: ${testName}${colors.NC}`, + ); + PASSED_TESTS++; + resolve(true); + return; + } else if (behaviorCorrect === false) { + console.log( + `${colors.RED}✗ Timeout behavior test failed: ${testName}${colors.NC}`, + ); + console.log( + `${colors.RED}Expected: ${expectedBehavior}${colors.NC}`, + ); + console.log( + `${colors.RED}Output did not match expected behavior${colors.NC}`, + ); + FAILED_TESTS++; + process.exit(1); + } + } + if (code === 0) { console.log(`${colors.GREEN}✓ Test passed: ${testName}${colors.NC}`); console.log(`${colors.BLUE}First few lines of output:${colors.NC}`); @@ -283,7 +393,7 @@ async function runBasicTest(testName, ...args) { } // Function to run an error test (expected to fail) -async function runErrorTest(testName, ...args) { +async function runErrorTest(testName, expectedBehavior, ...args) { const outputFile = path.join( OUTPUT_DIR, `${testName.replace(/\//g, "_")}.log`, @@ -331,11 +441,52 @@ async function runErrorTest(testName, ...args) { clearTimeout(timeout); outputStream.end(); + // For specific timeout behavior tests, validate the behavior + if (expectedBehavior) { + const behaviorCorrect = analyzeTimeoutBehavior( + output, + testName, + expectedBehavior, + ); + + if (behaviorCorrect === true) { + console.log( + `${colors.GREEN}✓ Timeout behavior test passed: ${testName}${colors.NC}`, + ); + PASSED_TESTS++; + resolve(true); + return; + } else if (behaviorCorrect === false) { + console.log( + `${colors.RED}✗ Timeout behavior test failed: ${testName}${colors.NC}`, + ); + console.log( + `${colors.RED}Expected: ${expectedBehavior}${colors.NC}`, + ); + console.log( + `${colors.RED}Output did not match expected behavior${colors.NC}`, + ); + FAILED_TESTS++; + process.exit(1); + } + } + // For error tests, we expect a non-zero exit code if (code !== 0) { - console.log( - `${colors.GREEN}✓ Error test passed: ${testName}${colors.NC}`, - ); + // Look for specific timeout errors + if ( + testName.includes("timeout") && + (output.includes("timeout") || output.includes("timed out")) + ) { + console.log( + `${colors.GREEN}✓ Timeout error test passed: ${testName}${colors.NC}`, + ); + } else { + console.log( + `${colors.GREEN}✓ Error test passed: ${testName}${colors.NC}`, + ); + } + console.log(`${colors.BLUE}Error output (expected):${colors.NC}`); const firstFewLines = output .split("\n") @@ -384,6 +535,7 @@ async function runTests() { // Test 1: Basic CLI mode with method await runBasicTest( "basic_cli_mode", + null, TEST_CMD, ...TEST_ARGS, "--cli", @@ -394,6 +546,7 @@ async function runTests() { // Test 2: CLI mode with non-existent method (should fail) await runErrorTest( "nonexistent_method", + null, TEST_CMD, ...TEST_ARGS, "--cli", @@ -402,7 +555,7 @@ async function runTests() { ); // Test 3: CLI mode without method (should fail) - await runErrorTest("missing_method", TEST_CMD, ...TEST_ARGS, "--cli"); + await runErrorTest("missing_method", null, TEST_CMD, ...TEST_ARGS, "--cli"); console.log( `\n${colors.YELLOW}=== Running Environment Variable Tests ===${colors.NC}`, @@ -411,6 +564,7 @@ async function runTests() { // Test 4: CLI mode with environment variables await runBasicTest( "env_variables", + null, TEST_CMD, ...TEST_ARGS, "-e", @@ -425,6 +579,7 @@ async function runTests() { // Test 5: CLI mode with invalid environment variable format (should fail) await runErrorTest( "invalid_env_format", + null, TEST_CMD, ...TEST_ARGS, "-e", @@ -437,6 +592,7 @@ async function runTests() { // Test 5b: CLI mode with environment variable containing equals sign in value await runBasicTest( "env_variable_with_equals", + null, TEST_CMD, ...TEST_ARGS, "-e", @@ -449,6 +605,7 @@ async function runTests() { // Test 5c: CLI mode with environment variable containing base64-encoded value await runBasicTest( "env_variable_with_base64", + null, TEST_CMD, ...TEST_ARGS, "-e", @@ -465,6 +622,7 @@ async function runTests() { // Test 6: Using config file with CLI mode await runBasicTest( "config_file", + null, "--config", path.join(PROJECT_ROOT, "sample-config.json"), "--server", @@ -477,6 +635,7 @@ async function runTests() { // Test 7: Using config file without server name (should fail) await runErrorTest( "config_without_server", + null, "--config", path.join(PROJECT_ROOT, "sample-config.json"), "--cli", @@ -487,6 +646,7 @@ async function runTests() { // Test 8: Using server name without config file (should fail) await runErrorTest( "server_without_config", + null, "--server", "everything", "--cli", @@ -497,6 +657,7 @@ async function runTests() { // Test 9: Using non-existent config file (should fail) await runErrorTest( "nonexistent_config", + null, "--config", "./nonexistent-config.json", "--server", @@ -509,6 +670,7 @@ async function runTests() { // Test 10: Using invalid config file format (should fail) await runErrorTest( "invalid_config", + null, "--config", invalidConfigPath, "--server", @@ -521,6 +683,7 @@ async function runTests() { // Test 11: Using config file with non-existent server (should fail) await runErrorTest( "nonexistent_server", + null, "--config", path.join(PROJECT_ROOT, "sample-config.json"), "--server", @@ -530,6 +693,69 @@ async function runTests() { "tools/list", ); + console.log( + `\n${colors.YELLOW}=== Running Tool-Related Tests ===${colors.NC}`, + ); + + // Test 12: CLI mode with tool call + await runBasicTest( + "tool_call", + null, + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + "message=Hello", + ); + + // Test 13: CLI mode with tool call but missing tool name (should fail) + await runErrorTest( + "missing_tool_name", + null, + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-arg", + "message=Hello", + ); + + // Test 14: CLI mode with tool call but invalid tool args format (should fail) + await runErrorTest( + "invalid_tool_args", + null, + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + "invalid_format", + ); + + // Test 15: CLI mode with multiple tool args + await runBasicTest( + "multiple_tool_args", + null, + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "add", + "--tool-arg", + "a=1", + "b=2", + ); + console.log( `\n${colors.YELLOW}=== Running Resource-Related Tests ===${colors.NC}`, ); @@ -537,6 +763,7 @@ async function runTests() { // Test 16: CLI mode with resource read await runBasicTest( "resource_read", + null, TEST_CMD, ...TEST_ARGS, "--cli", @@ -549,6 +776,7 @@ async function runTests() { // Test 17: CLI mode with resource read but missing URI (should fail) await runErrorTest( "missing_uri", + null, TEST_CMD, ...TEST_ARGS, "--cli", @@ -563,6 +791,7 @@ async function runTests() { // Test 18: CLI mode with prompt get await runBasicTest( "prompt_get", + null, TEST_CMD, ...TEST_ARGS, "--cli", @@ -575,6 +804,7 @@ async function runTests() { // Test 19: CLI mode with prompt get and args await runBasicTest( "prompt_get_with_args", + null, TEST_CMD, ...TEST_ARGS, "--cli", @@ -590,6 +820,7 @@ async function runTests() { // Test 20: CLI mode with prompt get but missing prompt name (should fail) await runErrorTest( "missing_prompt_name", + null, TEST_CMD, ...TEST_ARGS, "--cli", @@ -602,6 +833,7 @@ async function runTests() { // Test 21: CLI mode with log level await runBasicTest( "log_level", + null, TEST_CMD, ...TEST_ARGS, "--cli", @@ -614,6 +846,7 @@ async function runTests() { // Test 22: CLI mode with invalid log level (should fail) await runErrorTest( "invalid_log_level", + null, TEST_CMD, ...TEST_ARGS, "--cli", @@ -635,6 +868,7 @@ async function runTests() { // Test 23: CLI mode with config file, environment variables, and tool call await runBasicTest( "combined_options", + null, "--config", path.join(PROJECT_ROOT, "sample-config.json"), "--server", @@ -649,6 +883,7 @@ async function runTests() { // Test 24: CLI mode with all possible options (that make sense together) await runBasicTest( "all_options", + null, "--config", path.join(PROJECT_ROOT, "sample-config.json"), "--server", @@ -673,6 +908,7 @@ async function runTests() { // Test 25: Config with stdio transport type await runBasicTest( "config_stdio_type", + null, "--config", stdioConfigPath, "--server", @@ -685,6 +921,7 @@ async function runTests() { // Test 26: Config with SSE transport type (CLI mode) - expects connection error await runErrorTest( "config_sse_type_cli", + null, "--config", sseConfigPath, "--server", @@ -697,6 +934,7 @@ async function runTests() { // Test 27: Config with streamable-http transport type (CLI mode) - expects connection error await runErrorTest( "config_http_type_cli", + null, "--config", httpConfigPath, "--server", @@ -709,6 +947,7 @@ async function runTests() { // Test 28: Legacy config without type field (backward compatibility) await runBasicTest( "config_legacy_no_type", + null, "--config", legacyConfigPath, "--server", @@ -793,6 +1032,7 @@ async function runTests() { // Test 29: Config with single server auto-selection await runBasicTest( "single_server_auto_select", + null, "--config", singleServerConfigPath, "--cli", @@ -803,6 +1043,7 @@ async function runTests() { // Test 30: Config with default-server should now require explicit selection (multiple servers) await runErrorTest( "default_server_requires_explicit_selection", + null, "--config", defaultServerConfigPath, "--cli", @@ -813,6 +1054,7 @@ async function runTests() { // Test 31: Config with multiple servers and no default (should fail) await runErrorTest( "multi_server_no_default", + null, "--config", multiServerConfigPath, "--cli", @@ -833,6 +1075,7 @@ async function runTests() { { detached: true, stdio: "ignore", + shell: true, }, ); runningServers.push(httpServer); @@ -842,6 +1085,7 @@ async function runTests() { // Test 32: HTTP transport inferred from URL ending with /mcp await runBasicTest( "http_transport_inferred", + null, "http://127.0.0.1:3001/mcp", "--cli", "--method", @@ -851,6 +1095,7 @@ async function runTests() { // Test 33: HTTP transport with explicit --transport http flag await runBasicTest( "http_transport_with_explicit_flag", + null, "http://127.0.0.1:3001/mcp", "--transport", "http", @@ -862,6 +1107,7 @@ async function runTests() { // Test 34: HTTP transport with suffix and --transport http flag await runBasicTest( "http_transport_with_explicit_flag_and_suffix", + null, "http://127.0.0.1:3001/mcp", "--transport", "http", @@ -873,6 +1119,7 @@ async function runTests() { // Test 35: SSE transport given to HTTP server (should fail) await runErrorTest( "sse_transport_given_to_http_server", + null, "http://127.0.0.1:3001", "--transport", "sse", @@ -913,6 +1160,166 @@ async function runTests() { ); } + console.log( + `\n${colors.YELLOW}=== MCP Inspector CLI Timeout Configuration Tests ===${colors.NC}`, + ); + console.log( + `${colors.BLUE}This script tests the MCP Inspector CLI's timeout configuration options:${colors.NC}`, + ); + console.log( + `${colors.BLUE}- Request timeout (--request-timeout)${colors.NC}`, + ); + console.log( + `${colors.BLUE}- Reset timeout on progress (--reset-timeout-on-progress)${colors.NC}`, + ); + console.log( + `${colors.BLUE}- Maximum total timeout (--max-total-timeout)${colors.NC}\n`, + ); + + // Test 29: Default timeout values + await runBasicTest( + "default_timeouts", + null, + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/list", + ); + + // Test 30: Custom request timeout + await runBasicTest( + "custom_request_timeout", + null, + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "longRunningOperation", + "--tool-arg", + "duration=5", + "steps=5", + "--request-timeout", + "15000", + ); + + // Test 31: Request timeout too short (should fail) + await runErrorTest( + "short_request_timeout", + "should_timeout_quickly", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "longRunningOperation", + "--tool-arg", + "duration=5", + "steps=5", + "--request-timeout", + "100", + ); + + // Test 32: Combined timeout options for long-running operations + // This tests request-timeout + reset-timeout-on-progress + max-total-timeout working together + // Note: We cannot fully validate progress-based timeout resets in CLI mode since progress + // notifications are not yet captured/handled, so the timeout won't actually reset on progress. + // We use a request-timeout long enough for the operation to complete without relying on resets. + await runBasicTest( + "combined_timeout_options", + null, + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "longRunningOperation", + "--tool-arg", + "duration=2", + "steps=2", + "--request-timeout", + "5000", + "--reset-timeout-on-progress", + "true", + "--max-total-timeout", + "10000", + ); + + // console.log( + // `\n${colors.YELLOW}=== Running Progress-Related Timeout Tests ===${colors.NC}`, + // ); + + // // Test 33: Reset timeout on progress disabled - should fail with timeout + // // This test is commented out because we cannot yet capture progress notifications + // // in CLI mode to validate that timeouts are NOT being reset on progress + // await runErrorTest( + // "reset_timeout_disabled", + // "should_timeout_quickly", + // TEST_CMD, + // ...TEST_ARGS, + // "--cli", + // "--method", + // "tools/call", + // "--tool-name", + // "longRunningOperation", + // "--tool-arg", + // "duration=15", + // "steps=5", + // "--request-timeout", + // "2000", + // "--reset-timeout-on-progress", + // "false", + // "--max-total-timeout", + // "30000", + // ); + + console.log( + `\n${colors.YELLOW}=== Running Input Validation Tests ===${colors.NC}`, + ); + + // Test 34: Invalid request timeout value + await runErrorTest( + "invalid_request_timeout", + null, + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/list", + "--request-timeout", + "invalid", + ); + + // Test 35: Invalid reset-timeout-on-progress value + await runErrorTest( + "invalid_reset_timeout", + null, + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/list", + "--reset-timeout-on-progress", + "not-a-boolean", + ); + + // Test 36: Invalid max total timeout value + await runErrorTest( + "invalid_max_timeout", + null, + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/list", + "--max-total-timeout", + "invalid", + ); + // Print test summary console.log(`\n${colors.YELLOW}=== Test Summary ===${colors.NC}`); console.log(`${colors.GREEN}Passed: ${PASSED_TESTS}${colors.NC}`); diff --git a/cli/src/client/connection.ts b/cli/src/client/connection.ts index dcbe8e518..c82749fad 100644 --- a/cli/src/client/connection.ts +++ b/cli/src/client/connection.ts @@ -1,6 +1,7 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import type { Transport } from "@modelcontextprotocol/sdk/shared/transport.js"; import { McpResponse } from "./types.js"; +import { RequestOptions } from "@modelcontextprotocol/sdk/shared/protocol.js"; export const validLogLevels = [ "trace", @@ -15,9 +16,10 @@ export type LogLevel = (typeof validLogLevels)[number]; export async function connect( client: Client, transport: Transport, + options: RequestOptions, ): Promise { try { - await client.connect(transport); + await client.connect(transport, options); if (client.getServerCapabilities()?.logging) { // default logging level is undefined in the spec, but the user of the diff --git a/cli/src/index.ts b/cli/src/index.ts index 90c9f5e6e..f5878b8fe 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -20,6 +20,7 @@ import { } from "./client/index.js"; import { handleError } from "./error-handler.js"; import { createTransport, TransportOptions } from "./transport.js"; +import { RequestOptions } from "@modelcontextprotocol/sdk/shared/protocol.js"; import { awaitableLog } from "./utils/awaitable-log.js"; // JSON value type for CLI arguments @@ -41,6 +42,9 @@ type Args = { logLevel?: LogLevel; toolName?: string; toolArg?: Record; + requestTimeout?: number; + resetTimeoutOnProgress?: boolean; + maxTotalTimeout?: number; transport?: "sse" | "stdio" | "http"; headers?: Record; }; @@ -123,7 +127,13 @@ async function callMethod(args: Args): Promise { const client = new Client(clientIdentity); try { - await connect(client, transport); + const mcpRequestOptions: RequestOptions = { + timeout: args?.requestTimeout, + resetTimeoutOnProgress: args?.resetTimeoutOnProgress, + maxTotalTimeout: args?.maxTotalTimeout, + }; + + await connect(client, transport, mcpRequestOptions); let result: McpResponse; @@ -240,6 +250,24 @@ function parseHeaderPair( return { ...previous, [key]: val }; } +function parseStringToNumber(value: string): number { + const parsedValue = parseInt(value, 10); + if (isNaN(parsedValue)) { + throw new Error(`Invalid parameter format: ${value}. Use a number format.`); + } + return parsedValue; +} + +function parseStringToBoolean(value: string): boolean { + if (value === "true") { + return true; + } else if (value === "false") { + return false; + } else { + throw new Error(`Invalid parameter format: ${value}. Use true or false.`); + } +} + function parseArgs(): Args { const program = new Command(); @@ -304,9 +332,24 @@ function parseArgs(): Args { return value as LogLevel; }, ) - // - // Transport options - // + .option( + "--request-timeout ", + "Timeout for requests to the MCP server (ms)", + parseStringToNumber, + 10000, + ) + .option( + "--reset-timeout-on-progress ", + "Reset timeout on progress notifications", + parseStringToBoolean, + true, + ) + .option( + "--max-total-timeout ", + "Maximum total timeout for requests sent to the MCP server (ms) (Use with progress notifications)", + parseStringToNumber, + 60000, + ) .option( "--transport ", "Transport type (sse, http, or stdio). Auto-detected from URL: /mcp → http, /sse → sse, commands → stdio",