-
Notifications
You must be signed in to change notification settings - Fork 29.4k
Add Timeout config for the MCP Client tool #15886
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
base: master
Are you sure you want to change the base?
Conversation
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.
cubic found 2 issues across 3 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai
to give specific feedback.
displayName: 'Timeout', | ||
name: 'timeout', | ||
type: 'number', | ||
typeOptions: { |
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.
Consider adding a maxValue to prevent users from setting excessively long timeouts that could lead to resource exhaustion.
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.
Should this be done? I lifted this straight from the HTTP Request node, where it doesn't have a max either. And I don't feel quite comfortable setting a hard cap on behalf of users, they'll know better than me what they need
typeOptions: { | ||
minValue: 1, | ||
}, | ||
default: 60000, |
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 default timeout value (60000ms) in the UI doesn't match the value mentioned in the PR description (default is described as being set to 60 seconds in the MCP SDK).
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.
Am I crazy, or is it the bot? I swear 60000ms===60 seconds :)
Hey @jreyesr, Thanks for the PR, We have created "GHC-2279" as the internal reference to get this reviewed. One of us will be in touch if there are any changes needed, in most cases this is normally within a couple of weeks but it depends on the current workload of the team. |
Summary
Hello! This PR adds a setting for the MCP Client tool that allows users to set a timeout on tool calls (similar to the Timeout setting on the HTTP Request node)
Currently, this timeout is set to 60 seconds from the default value provided by the MCP SDK, and it can't be configured by the N8N user.
Allowing users to control this would be useful both to:
You can use the following workflow to test the changes. No credentials are required. The tool can be run in isolation, without running the entire agent. Choose the
get_supported_languages
tool because it doesn't require any more parameters. The tool takes ~550 milliseconds to run, so the currently configured timeout of 100 millis should fail with "MCP error -32001: Request timed out". Bump it to 1000 millis to check that it now succeeds.Sample workflow
Related Linear tickets, Github issues, and Community forum posts
The post above actually asks for a more general solution, namely exposing all the tool call request options. There aren't that many that look interesting to me, but I'm open to changing this PR to expose more options, if you think it'd be better to expose all of them in one go, instead of just adding the single Timeout
Review / Merge checklist