Revamp CoreIpc [ROBO-3791] #106

eduard-dumitru · 2024-10-09T10:22:06Z

Changes:

Pipelines
- (TBR) disable the constraint of pushing NuGet packages to the feed
  only when the source branch is master
- (TBD) restore, build, and pack the main solution explicitly
  before running unit tests
- Use .NET Runtime 8.0.8 and UseDotNet@2 for both 6.0.317 and 8.0.400
General
- change the UiPath.CoreIpc* namespace to UiPath.Ipc*
- decommission the usage of Nito
- start optimizing logging via LoggerMessageAttribute
API and behavior
- commission the new IPC API that doesn't rely on the fluent builder pattern
- commission the feature that allows the optional BeforeCall
  of an IpcServer Endpoint to share its AsyncLocal context
  with the target method execution, if the BeforeCall executes
  synchronously.
  Check the ServerBeforeCall_WhenSync_ShouldShareAsyncLocalContextWithTheTargetMethodCall test for further details.
- extensibility support for potentially any transport, including JS-DotNetWasm interop
Internals
- decommission the generic variant of ServiceClient, thus greatly simplifying the flow
- commission the ServiceClientProper and ServiceClientForCallback subclasses

-------- 1. Pipelines - (TBR) disable the constraint of pushing NuGet packages to the feed only when source branch is master - (TBD) restore, build and pack the main solution explicitly before running unit tests - Use .NET Runtime 8.0.8 and UseDotNet@2 for both 6.0.317 and 8.0.400 2. General - change the UiPath.CoreIpc* namespace to UiPath.Ipc* - decomission the usage of Nito - start optimizing logging via LoggerMessageAttribute 3. API and behavior - commission the new Ipc API that doesn't rely on the fluent builder pattern - commission the feature that allows the optional BeforeCall of an IpcServer Endpoint to share its AsyncLocal context with the target method execution, if the BeforeCall executes synchronously. Check the ServerBeforeCall_WhenSync_ShouldShareAsyncLocalContextWithTheTargetMethodCall test for further details. - extensibility support for potentially any transport, including JS-DotNetWasm interop 4. Internals - decommission the generic variant of ServiceClient, thus greatly simplifying the flow - commission the ServiceClientProper and ServiceClientForCallback subclasses

- simplifications

- decommission WaitForStop

- decommission the duplication of IClient methods in Message

bogdanStan92 · 2024-12-10T10:17:03Z

src/UiPath.CoreIpc/UiPath.CoreIpc.csproj

@@ -1,8 +1,10 @@
 <Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>net6.0;net461;net6.0-windows</TargetFrameworks>


We should discuss this, not necessarily as part of this PR, but maybe we shouldn't target net6 anymore as it's out of support

bogdanStan92 · 2024-12-10T10:20:12Z

src/IpcSample.ConsoleClient/Client.cs

-using Microsoft.Extensions.DependencyInjection;
+using System.Text;
+using UiPath.Ipc;
+using UiPath.Ipc.Transport.NamedPipe;

 namespace UiPath.CoreIpc.Tests;


UiPath.Ipc.Tests*

src/CoreIpc.sln

bogdanStan92 · 2025-06-16T08:26:39Z

NuGet.Config

@@ -2,5 +2,6 @@
 <configuration>
  <packageSources>
    <add key="Nuget" value="https://api.nuget.org/v3/index.json" />
+    <add key="UiPath-Internal" value="https://uipath.pkgs.visualstudio.com/Public.Feeds/_packaging/UiPath-Internal/nuget/v3/index.json" />


This doesn't seem necessary

bogdanStan92 · 2025-06-16T08:39:00Z

src/CI/azp-dotnet-dist.yaml

@@ -16,15 +16,15 @@ steps:

  - task: DotNetCoreCLI@2
    displayName: 'dotnet push to UiPath-Internal'
-    condition: and(succeeded(), eq(variables['Build.SourceBranch'], 'refs/heads/master'))
+    condition: succeeded()


We should discuss the release strategy

src/IpcSample.ConsoleClient/Client.cs

src/UiPath.CoreIpc.Extensions.Abstractions/UiPath.CoreIpc.Extensions.Abstractions.csproj

src/UiPath.CoreIpc/Server/Server.cs

bogdanStan92 · 2025-06-24T12:07:11Z

src/UiPath.CoreIpc/Connection.cs

        await _sendLock.WaitAsync(cancellationToken);
        try
        {
+            Logger?.LogInformation($"Connection.SendMessage: sendLock was successfully aquired. Pushing the bytes onto the network. ByteCount: {data.Length}");


Do we want these traces inside the lock?

src/UiPath.CoreIpc/Config/IpcClient.cs

eduard-dumitru · 2025-07-14T17:13:44Z

/azp run

azure-pipelines · 2025-07-14T17:13:52Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Pull Request Overview

This PR implements a major revamp of the CoreIpc library, focusing on API redesign, namespace consolidation, and improved extensibility. The changes include a shift from fluent builder patterns to direct configuration, enhanced transport abstractions, and optimized logging through attributes.

Namespace migration from UiPath.CoreIpc* to UiPath.Ipc*
New non-fluent IPC API replacing builder patterns
Enhanced transport extensibility with improved abstractions
Introduction of logging optimization via LoggerMessageAttribute

Reviewed Changes

Copilot reviewed 170 out of 173 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`src/UiPath.CoreIpc/UiPath.CoreIpc.csproj`	Updated project configuration with new assembly name, namespace, and dependency versions
`src/UiPath.CoreIpc/Config/IpcServer.cs`	New IPC server implementation with required properties and async disposal
`src/UiPath.CoreIpc/Config/IpcClient.cs`	New IPC client implementation replacing builder pattern
`src/UiPath.CoreIpc/Client/ServiceClient.cs`	Major refactoring of service client architecture with new inheritance hierarchy
`src/UiPath.CoreIpc/Transport/`	New transport abstraction layer for NamedPipe, TCP, and WebSocket implementations
`src/UiPath.CoreIpc/Logging/LoggingExtensions.cs`	New structured logging implementation using `LoggerMessageAttribute`
`src/UiPath.CoreIpc/Wire/Dtos.cs`	Updated DTOs with nullable reference types and simplified serialization

Comments suppressed due to low confidence (1)

src/UiPath.CoreIpc/Transport/NamedPipe/NamedPipeServerTransport.cs:12

[nitpick] The property name 'AccessControl' is ambiguous. Consider renaming to 'AccessControlConfig' or 'AccessControlSetup' to better indicate it's a delegate for configuring access control.

    public AccessControlDelegate? AccessControl { get; init; }

Copilot · 2025-07-16T12:57:54Z

src/UiPath.CoreIpc/report/generate-report.bat

+REM generate-public-api --target-frameworks "net6.0"         --assembly UiPath.Ipc.dll --project-path "D:\Alt\coreipc\src\UiPath.CoreIpc\UiPath.CoreIpc.csproj" --output-directory "D:\Alt\coreipc\src\UiPath.CoreIpc\report" --verbose
+REM generate-public-api --target-frameworks "net461"         --assembly UiPath.Ipc.dll --project-path "D:\Alt\coreipc\src\UiPath.CoreIpc\UiPath.CoreIpc.csproj" --output-directory "D:\Alt\coreipc\src\UiPath.CoreIpc\report" --verbose


The batch file contains hardcoded absolute paths in the commented lines (lines 40-41). These should be removed or replaced with relative paths to improve maintainability and avoid exposing local development paths.

Suggested change

REM generate-public-api --target-frameworks "net6.0" --assembly UiPath.Ipc.dll --project-path "D:\Alt\coreipc\src\UiPath.CoreIpc\UiPath.CoreIpc.csproj" --output-directory "D:\Alt\coreipc\src\UiPath.CoreIpc\report" --verbose

REM generate-public-api --target-frameworks "net461" --assembly UiPath.Ipc.dll --project-path "D:\Alt\coreipc\src\UiPath.CoreIpc\UiPath.CoreIpc.csproj" --output-directory "D:\Alt\coreipc\src\UiPath.CoreIpc\report" --verbose

REM generate-public-api --target-frameworks "net6.0" --assembly UiPath.Ipc.dll --project-path "%projectPath%" --output-directory "%outputPath%" --verbose

REM generate-public-api --target-frameworks "net461" --assembly UiPath.Ipc.dll --project-path "%projectPath%" --output-directory "%outputPath%" --verbose

Copilot · 2025-07-16T12:57:54Z

src/UiPath.CoreIpc/Wire/IpcJsonSerializer.cs

+    public static readonly IpcJsonSerializer Instance = new();
+
+    static readonly JsonSerializer StringArgsSerializer = new() { CheckAdditionalContent = true };
+


The conditional compilation directive for AsyncMethodBuilder should include a comment explaining why it's excluded for NET461, as this affects performance optimization behavior.

Suggested change

// The AsyncMethodBuilder attribute is excluded for NET461 because the .NET Framework 4.6.1

// does not support PoolingAsyncValueTaskMethodBuilder, which is used for performance optimization.

Copilot · 2025-07-16T12:57:54Z

src/UiPath.CoreIpc/Config/IpcServer.cs

+        => Task.WhenAll(Enumerable.Range(start: 0, parallelCount).Select(_ => Task.Run(() => action(ct))));
+    }
+
+    [MemberNotNullWhen(returnValue: true, member: nameof(Transport))]


The MemberNotNullWhen attribute references Transport, but the Transport property is marked as required and should never be null when the object is properly constructed. This attribute may be unnecessary or incorrectly applied.

Suggested change

[MemberNotNullWhen(returnValue: true, member: nameof(Transport))]

Copilot · 2025-07-16T12:57:55Z

src/UiPath.CoreIpc/Client/ServiceClient.cs

    {
-        Dispose(true);
-        GC.SuppressFinalize(this);
+        CloseConnection().AsTask().TraceError();


Converting ValueTask to Task using AsTask() in the Dispose method may cause unnecessary allocations. Consider using ConfigureAwait(false) and handling the ValueTask directly, or ensure this is intentionally fire-and-forget.

eduard-dumitru · 2025-07-17T14:09:58Z

/azp run CI

azure-pipelines · 2025-07-17T14:10:07Z

Azure Pipelines successfully started running 1 pipeline(s).

eduard-dumitru · 2025-07-17T14:23:50Z

/azp run CI

azure-pipelines · 2025-07-17T14:23:59Z

Azure Pipelines successfully started running 1 pipeline(s).

eduard-dumitru force-pushed the feat/revamp_lean branch 2 times, most recently from 84bb59a to ad64ac2 Compare October 9, 2024 13:27

eduard-dumitru force-pushed the feat/revamp_lean branch from ad64ac2 to 6b263e6 Compare October 9, 2024 13:54

eduard-dumitru marked this pull request as ready for review October 17, 2024 11:14

eduard-dumitru requested review from bogdanStan92 and danutboanta October 17, 2024 11:15

eduard-dumitru changed the title ~~Revamp CoreIpc (lean)~~ Revamp CoreIpc [ROBO-3791] Oct 22, 2024

eduard-dumitru added 16 commits December 9, 2024 10:41

taking shape

6e8f258

taking shape

9d86227

taking shape

6b724fc

taking shape

2799b98

taking shape

8d6b388

taking shape

67f1445

taking shape

cd06d85

taking shape

ce1a19b

taking shape

1e3211a

rename Endpoint.. to Contract..

0d7f45d

- simplifications

update NodeInterop to the new API

89bb598

simplifications after review

1edfc9b

- decommission WaitForStop

reduce surface area

d17ec52

- decommission the duplication of IClient methods in Message

polyfill cleanup for net461

fdb5cd2

cleanup and revival

72c8a71

warning fixes, namespace and polyfill cleanup

f4bffbb

bogdanStan92 reviewed Jul 11, 2025

View reviewed changes

fixes after review

5a52bb1

eduard-dumitru force-pushed the feat/revamp_lean branch 2 times, most recently from c8a0895 to 078cfce Compare July 14, 2025 13:06

README.md

a68ff89

eduard-dumitru force-pushed the feat/revamp_lean branch from 078cfce to a68ff89 Compare July 14, 2025 13:07

eduard-dumitru requested a review from Copilot July 14, 2025 15:35