Add timeout property to FTP Scope activity#518
Conversation
Add a configurable Timeout (milliseconds) property to the With FTP Session activity so users can control connection and operation timeouts for both FTP/FTPS (FluentFTP) and SFTP (SSH.NET) sessions, preventing indefinite hangs when remote servers are unresponsive. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
adrianignat13
requested review from
alexandru-petre,
Copilot and
viogroza
and removed request for
alexandru-petre,
Copilot and
viogroza
There was a problem hiding this comment.
Pull request overview
Adds a user-configurable timeout to the With FTP Session (FTP Scope) activity, wiring it through the activities layer into the core FTP/SFTP session implementations so hung connections/operations can be bounded.
Changes:
- Introduces
Timeout(ms) inFtpConfigurationand surfaces it as an activity argument onWithFtpSession. - Applies the configured timeout to FluentFTP client config (FTP/FTPS) and to SSH.NET connection/operation timeouts (SFTP).
- Adds unit tests and updates activity metadata + localized resources for the new property.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| Activities/FTP/UiPath.FTP/FtpConfiguration.cs | Adds nullable Timeout (ms) to session configuration. |
| Activities/FTP/UiPath.FTP/FtpSession.cs | Applies configured timeout to FluentFTP connect/read/data timeouts. |
| Activities/FTP/UiPath.FTP/SftpSession.cs | Applies configured timeout to SSH.NET ConnectionInfo.Timeout and SftpClient.OperationTimeout. |
| Activities/FTP/UiPath.FTP.Activities/WithFtpSession.cs | Adds Timeout activity argument and maps it into FtpConfiguration. |
| Activities/FTP/UiPath.FTP.Activities/NetCore/ViewModels/WithFtpSessionViewModel.cs | Adds the Timeout design-time argument and orders it after Port. |
| Activities/FTP/UiPath.FTP.Activities/Resources/ActivitiesMetadata.json | Registers Timeout in activity metadata for Studio/designer. |
| Activities/FTP/UiPath.FTP.Activities/Properties/UiPath.FTP.Activities.resx | Adds localized display name/description for Timeout. |
| Activities/FTP/UiPath.FTP.Activities/Properties/UiPath.FTP.Activities.Designer.cs | Generated resource accessors for the new strings. |
| Activities/FTP/UiPath.FTP.Tests/SessionsTests/FtpSessionTests.cs | Adds coverage for “timeout applied” and “defaults preserved” (FTP). |
| Activities/FTP/UiPath.FTP.Tests/SessionsTests/SftpSessionTests.cs | Adds coverage for “timeout applied” and “defaults preserved” (SFTP). |
Files not reviewed (1)
- Activities/FTP/UiPath.FTP.Activities/Properties/UiPath.FTP.Activities.Designer.cs: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <data name="Activity_WithFtpSession_Property_Timeout_Description" xml:space="preserve"> | ||
| <value>The timeout value (in milliseconds) for the FTP connection. If not set, the default timeout of the underlying library is used.</value> | ||
| <comment>Property description</comment> | ||
| </data> | ||
| <data name="Activity_WithFtpSession_Property_Timeout_Name" xml:space="preserve"> | ||
| <value>Timeout (milliseconds)</value> | ||
| <comment>Property name</comment> |
There was a problem hiding this comment.
The Timeout description says it applies to the “FTP connection”, but With FTP Session can also run in SFTP mode. Consider updating the wording to “FTP/SFTP session” (or similar) to avoid misleading users.
|
|
||
| // When no timeout is set, FluentFTP defaults should be preserved | ||
| var defaultClient = new FtpClient(); | ||
| Assert.Equal(defaultClient.Config.ConnectTimeout, ftpClient.Config.ConnectTimeout); |
There was a problem hiding this comment.
This default-preservation test only checks ConnectTimeout. Since the implementation sets 4 FluentFTP timeout properties when configured, consider asserting all 4 match a default FtpClient’s corresponding values when Timeout is not set to prevent partial regressions.
| Assert.Equal(defaultClient.Config.ConnectTimeout, ftpClient.Config.ConnectTimeout); | |
| Assert.Equal(defaultClient.Config.ConnectTimeout, ftpClient.Config.ConnectTimeout); | |
| Assert.Equal(defaultClient.Config.ReadTimeout, ftpClient.Config.ReadTimeout); | |
| Assert.Equal(defaultClient.Config.DataConnectionConnectTimeout, ftpClient.Config.DataConnectionConnectTimeout); | |
| Assert.Equal(defaultClient.Config.DataConnectionReadTimeout, ftpClient.Config.DataConnectionReadTimeout); |
- Fix timeout description to say "FTP/SFTP" instead of just "FTP" - Add validation rejecting negative timeout values - Compute TimeSpan once in SftpSession instead of duplicating conversion - Assert all 4 FluentFTP timeout defaults in FTP default test - Assert both ConnectionInfo.Timeout and OperationTimeout defaults in SFTP test using fresh client instances instead of hardcoded values - Rename test credentials to avoid SonarQube hard-coded credential warnings Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- Activities/FTP/UiPath.FTP.Activities/Properties/UiPath.FTP.Activities.Designer.cs: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Use localized resource string (InvalidTimeoutException) for timeout validation instead of hard-coded English message - Use ArgumentOutOfRangeException with parameter name for consistency - Add InternalsVisibleTo on UiPath.FTP for test assembly access - Add internal Client properties on FtpSession and SftpSession to eliminate brittle reflection-based test access - Dispose all IDisposable clients in tests (sessions, default clients) - Extract SFTP test config helper to centralize dummy credentials - Add NOSONAR comments on test-only credential assignments Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- Activities/FTP/UiPath.FTP.Activities/Properties/UiPath.FTP.Activities.Designer.cs: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <comment>Property description</comment> | ||
| </data> | ||
| <data name="Activity_WithFtpSession_Property_Timeout_Description" xml:space="preserve"> | ||
| <value>The timeout value (in milliseconds) for the FTP/SFTP connection. If not set, the default timeout of the underlying library is used.</value> |
There was a problem hiding this comment.
The Timeout description says it applies to the FTP/SFTP "connection", but the implementation also sets read/data-connection timeouts for FTP and OperationTimeout for SFTP (i.e., it impacts operations too). Update this localized string to reflect that the timeout applies to both connecting and subsequent read/data operations, so users understand what is affected.
| <value>The timeout value (in milliseconds) for the FTP/SFTP connection. If not set, the default timeout of the underlying library is used.</value> | |
| <value>The timeout value (in milliseconds) for establishing the FTP/SFTP connection and for subsequent read and data operations. If not set, the default timeouts of the underlying library are used.</value> |
There was a problem hiding this comment.
Actually, we should be very specific; I came up with this variant:
Per-operation timeout in milliseconds for FTP/SFTP connections and data transfers. The activity may involve multiple operations, each timed independently. Leave empty to use the library's default timeouts.
| public DesignInArgument<int> Port { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// The connection timeout in milliseconds. |
There was a problem hiding this comment.
This summary describes Timeout as a "connection timeout", but the core implementation also applies it to read/data operation timeouts (FTP read/data timeouts, SFTP OperationTimeout). Consider updating the comment to avoid implying it only affects initial connection.
| /// The connection timeout in milliseconds. | |
| /// The timeout in milliseconds for establishing the connection and for FTP/SFTP data or read operations. |
| <comment>Property description</comment> | ||
| </data> | ||
| <data name="Activity_WithFtpSession_Property_Timeout_Name" xml:space="preserve"> | ||
| <value>Timeout (milliseconds)</value> |
There was a problem hiding this comment.
We should adapt the name to something like:
Server Reply Timeout (ms)
or
Response Timeout (ms)
or
Server Response Timeout (ms)
or
Request Timeout (ms)
or
Individual Operation Timeout (ms)
Problem Statement
The
With FTP Session(FTP Scope) activity has no timeout property. FTP/SFTP connectionscan hang indefinitely when the remote server is unresponsive, with no way for users to
control connection or operation timeouts.
Analysis
Both underlying libraries already support timeouts natively:
Config.ConnectTimeout,Config.ReadTimeout,Config.DataConnectionConnectTimeout,Config.DataConnectionReadTimeout— allintin millisecondsConnectionInfo.TimeoutandSftpClient.OperationTimeout— bothTimeSpanThe existing codebase uses
InArgument<int>for thePortproperty with anull-expression check pattern (Port.Expression == null ? null : (int?)Port.Get(context)).The
Timeoutproperty follows this same pattern for consistency.Timeout is implemented as per-connection (sets connect/read/data timeouts on the
underlying clients), which is the standard approach matching how Python activities handle
timeout. A cumulative scope-level timeout could be added later if needed.
Considered Use Cases
ConnectionInfo.Timeout) and data operation timeout(
SftpClient.OperationTimeout) are set to prevent hangs during file transfers(
[DefaultValue(null)]on the new property)Implementation
Core layer (
UiPath.FTP):FtpConfiguration.cs— Addedint? TimeoutpropertyFtpSession.cs— Apply timeout to 4 FluentFTP config properties (connect, read,data connection connect, data connection read) when set
SftpSession.cs— Apply timeout toConnectionInfo.Timeout(connect) andSftpClient.OperationTimeout(data operations) when setActivity layer (
UiPath.FTP.Activities):WithFtpSession.cs— AddedInArgument<int> Timeoutwith localized attributes,[DefaultValue(null)], wired toFtpConfigurationbefore the firstawaitDesign layer:
WithFtpSessionViewModel.cs— AddedDesignInArgument<int> Timeoutwith OrderIndexafter Port
ActivitiesMetadata.json— Registered Timeout property metadata.resx/.Designer.cs— Added localized display name and description stringsTests:
FtpSessionTests.cs— 2 new tests: timeout applied correctly, defaults preservedSftpSessionTests.cs— 2 new tests: timeout applied to both ConnectionInfo.Timeoutand OperationTimeout, defaults preserved
How to Test
dotnet test Activities/Activities.FTP.sln— all 14 tests passWith FTP Session, set Timeout to 5000,connect to a responsive server — should work normally
jira: https://uipath.atlassian.net/browse/STUD-75804
🤖 Generated with Claude Code