Fix handshake with Node.js not completing when external systems interfere with Node.js's stdout stream

Changelog.md

@@ -3,7 +3,13 @@ This project uses [semantic versioning](http://semver.org/spec/v2.0.0.html). Ref
 *[Semantic Versioning in Practice](https://www.jering.tech/articles/semantic-versioning-in-practice)*
 for an overview of semantic versioning.
 
-## [Unreleased](https://github.com/JeringTech/Javascript.NodeJS/compare/6.3.1...HEAD)
+## [Unreleased](https://github.com/JeringTech/Javascript.NodeJS/compare/7.0.0-beta.0...HEAD)
+
+## [7.0.0-beta.0](https://github.com/JeringTech/Javascript.NodeJS/compare/6.3.1...7.0.0-beta.0) - Aug 25, 2022
+### Changes
+- **Breaking**: `OutOfProcessNodeJSService.OnConnectionEstablishedMessageReceived` now takes a `System.Text.RegularExpressions.Match` argument instead of a `string`. ([#146](https://github.com/JeringTech/Javascript.NodeJS/pull/146))
+### Fixes
+- Fixed handshake with Node.js not completing when external systems interfere with Node.js's stdout stream. ([#146](https://github.com/JeringTech/Javascript.NodeJS/pull/146))
 
 ## [6.3.1](https://github.com/JeringTech/Javascript.NodeJS/compare/6.3.0...6.3.1) - May 10, 2022
 ### Fixes

ReadMe.md

@@ -1769,6 +1769,7 @@ Contributions are welcome!
 - [flcdrg](https://github.com/flcdrg)
 - [samcic](https://github.com/samcic)
 - [johnrom](https://github.com/johnrom)
+- [aKzenT](https://github.com/aKzenT)
 
 ## About
 Follow [@JeringTech](https://twitter.com/JeringTech) for updates and more.

azure-pipelines.yml

@@ -14,6 +14,3 @@ jobs:
     outOfProcessBuildDependencies: ["nodejs"]
     codecovKey: "e5de9f48-fb06-43c6-8368-44de5cf7e5d4"
     cacheYarnPackages: true
-- template: templates/docs/main.yml@templates
-  parameters:
-    nugetRestorePats: "$(nugetRestorePats)"

src/NodeJS/Jering.Javascript.NodeJS.csproj

@@ -48,7 +48,7 @@
             <PrivateAssets>all</PrivateAssets>
             <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
         </PackageReference>
-        <PackageReference Include="Nullable" Version="1.3.0">
+        <PackageReference Include="Nullable" Version="1.3.1">
             <PrivateAssets>all</PrivateAssets>
             <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
         </PackageReference>

src/NodeJS/NodeJSServiceImplementations/OutOfProcess/Http/HttpNodeJSService.cs

@@ -6,6 +6,7 @@
 using System.Net.Http;
 using System.Reflection;
 using System.Text;
+using System.Text.RegularExpressions;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -17,6 +18,11 @@ namespace Jering.Javascript.NodeJS
     /// </summary>
     public class HttpNodeJSService : OutOfProcessNodeJSService
     {
+        /// <summary>
+        /// Regex to match message used to perform a handshake with the NodeJS process.
+        /// </summary>
+        private static readonly Regex _sharedConnectionEstablishedMessageRegex = new(@"\[Jering\.Javascript\.NodeJS: HttpVersion - (?<protocol>HTTP\/\d\.\d) Listening on IP - (?<ip>[^ ]+) Port - (?<port>\d+)\]", RegexOptions.Compiled, TimeSpan.FromSeconds(1));
+
         internal const string HTTP11_SERVER_SCRIPT_NAME = "Http11Server.js";
         internal const string HTTP20_SERVER_SCRIPT_NAME = "Http20Server.js";
 
@@ -33,6 +39,9 @@ public class HttpNodeJSService : OutOfProcessNodeJSService
         // want to use the most recent instance
         internal volatile Uri? _endpoint;
 
+        /// <inheritdoc />
+        protected override Regex ConnectionEstablishedMessageRegex => _sharedConnectionEstablishedMessageRegex;
+
         /// <summary>
         /// Creates an <see cref="HttpNodeJSService"/>.
         /// </summary>
@@ -172,42 +181,18 @@ public HttpNodeJSService(IOptions<OutOfProcessNodeJSServiceOptions> outOfProcess
         }
 
         /// <inheritdoc />
-        protected override void OnConnectionEstablishedMessageReceived(string connectionEstablishedMessage)
+        protected override void OnConnectionEstablishedMessageReceived(Match connectionMessageMatch)
         {
-            // Start after message start and "HttpVersion - HTTP/X.X Listening on IP - "
-            int startIndex = CONNECTION_ESTABLISHED_MESSAGE_START.Length + 41;
-            var stringBuilder = new StringBuilder("http://");
-
-            for (int i = startIndex; i < connectionEstablishedMessage.Length; i++)
-            {
-                char currentChar = connectionEstablishedMessage[i];
-
-                if (currentChar == ':')
+            _endpoint = new UriBuilder
                 {
-                    // ::1
-                    stringBuilder.Append("[::1]");
-                    i += 2;
-                }
-                else if (currentChar == ' ')
-                {
-                    stringBuilder.Append(':');
-
-                    // Skip over "Port - "
-                    i += 7;
-                }
-                else if (currentChar == ']')
-                {
-                    _endpoint = new Uri(stringBuilder.ToString());
-                    _logger.LogInformation(string.Format(Strings.LogInformation_HttpEndpoint,
-                        connectionEstablishedMessage.Substring(41, 8), // Pluck out HTTP version
-                        _endpoint));
-                    return;
-                }
-                else
-                {
-                    stringBuilder.Append(currentChar);
-                }
-            }
+                    Scheme = "http",
+                    Host = connectionMessageMatch.Groups["ip"].Value,
+                    Port = int.Parse(connectionMessageMatch.Groups["port"].Value),
+                }.Uri;
+
+            _logger.LogInformation(string.Format(Strings.LogInformation_HttpEndpoint,
+                connectionMessageMatch.Groups["protocol"].Value, // Pluck out HTTP version
+                _endpoint));
         }
 
         /// <inheritdoc />

src/NodeJS/NodeJSServiceImplementations/OutOfProcess/OutOfProcessNodeJSService.cs

@@ -6,6 +6,7 @@
 using System.IO;
 using System.Linq;
 using System.Reflection;
+using System.Text.RegularExpressions;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -21,11 +22,6 @@ namespace Jering.Javascript.NodeJS
     /// <seealso cref="INodeJSService" />
     public abstract class OutOfProcessNodeJSService : INodeJSService
     {
-        /// <summary>
-        /// Start of the message used to perform a handshake with the NodeJS process.
-        /// </summary>
-        protected internal const string CONNECTION_ESTABLISHED_MESSAGE_START = "[Jering.Javascript.NodeJS: ";
-
         /// <summary>
         /// The logger for the NodeJS process's stdout and stderr streams as well as messages from <see cref="OutOfProcessNodeJSService"/> and its implementations.
         /// </summary>
@@ -57,6 +53,12 @@ public abstract class OutOfProcessNodeJSService : INodeJSService
         private volatile INodeJSProcess? _nodeJSProcess; // Volatile since it's used in a double checked lock (we check whether it's null)
         private IFileWatcher? _fileWatcher;
 
+        /// <summary>
+        /// <para>This regex is used to determine successful initialization of the process.</para>
+        /// <para>All match groups contained in the regex are passed as arguments to the <see cref="OnConnectionEstablishedMessageReceived"/> method.</para>
+        /// </summary>
+        protected abstract Regex ConnectionEstablishedMessageRegex { get; }
+
         /// <summary>
         /// Creates an <see cref="OutOfProcessNodeJSService"/> instance.
         /// </summary>
@@ -117,8 +119,8 @@ protected OutOfProcessNodeJSService(INodeJSProcessFactory nodeProcessFactory,
         /// <para>The message can be used to complete the handshake with the
         /// NodeJS process, for example by delivering a port and an IP address to use in further communications.</para>
         /// </summary>
-        /// <param name="connectionEstablishedMessage">The connection established message.</param>
-        protected abstract void OnConnectionEstablishedMessageReceived(string connectionEstablishedMessage);
+        /// <param name="connectionMessageMatch">The regex match that can be used to extract additional arguments to complete the handshake.</param>
+        protected abstract void OnConnectionEstablishedMessageReceived(Match connectionMessageMatch);
 
         /// <inheritdoc />
         public virtual async Task<T?> InvokeFromFileAsync<T>(string modulePath, string? exportName = null, object?[]? args = null, CancellationToken cancellationToken = default)
@@ -671,9 +673,9 @@ internal void OutputReceivedHandler(string message, EventWaitHandle waitHandle)
             //
             // Note that we should not get a connection message for any process other than the current _nodeJSProcess
             // because ConnectIfNotConnected is synchronous.
-            if (_nodeJSProcess?.Connected == false && message.StartsWith(CONNECTION_ESTABLISHED_MESSAGE_START))
+            if (_nodeJSProcess?.Connected == false && ConnectionEstablishedMessageRegex.Match(message) is { Success: true } match)
             {
-                OnConnectionEstablishedMessageReceived(message);
+                OnConnectionEstablishedMessageReceived(match);
 
                 if (_infoLoggingEnabled)
                 {

src/NodeJS/packages.lock.json

@@ -69,9 +69,9 @@
       },
       "Nullable": {
         "type": "Direct",
-        "requested": "[1.3.0, )",
-        "resolved": "1.3.0",
-        "contentHash": "xHAviTdTY3n+t1nEPN4JPRQR5lI124qRKVw+U9H7dO5sDNPpzoWeo/MQy7dSUmv9eD3k/CJVKokz1tFK+JOzRw=="
+        "requested": "[1.3.1, )",
+        "resolved": "1.3.1",
+        "contentHash": "Mk4ZVDfAORTjvckQprCSehi1XgOAAlk5ez06Va/acRYEloN9t6d6zpzJRn5MEq7+RnagyFIq9r+kbWzLGd+6QA=="
       },
       "System.Text.Encodings.Web": {
         "type": "Direct",
@@ -255,9 +255,9 @@
       },
       "Nullable": {
         "type": "Direct",
-        "requested": "[1.3.0, )",
-        "resolved": "1.3.0",
-        "contentHash": "xHAviTdTY3n+t1nEPN4JPRQR5lI124qRKVw+U9H7dO5sDNPpzoWeo/MQy7dSUmv9eD3k/CJVKokz1tFK+JOzRw=="
+        "requested": "[1.3.1, )",
+        "resolved": "1.3.1",
+        "contentHash": "Mk4ZVDfAORTjvckQprCSehi1XgOAAlk5ez06Va/acRYEloN9t6d6zpzJRn5MEq7+RnagyFIq9r+kbWzLGd+6QA=="
       },
       "System.Text.Encodings.Web": {
         "type": "Direct",
@@ -509,9 +509,9 @@
       },
       "Nullable": {
         "type": "Direct",
-        "requested": "[1.3.0, )",
-        "resolved": "1.3.0",
-        "contentHash": "xHAviTdTY3n+t1nEPN4JPRQR5lI124qRKVw+U9H7dO5sDNPpzoWeo/MQy7dSUmv9eD3k/CJVKokz1tFK+JOzRw=="
+        "requested": "[1.3.1, )",
+        "resolved": "1.3.1",
+        "contentHash": "Mk4ZVDfAORTjvckQprCSehi1XgOAAlk5ez06Va/acRYEloN9t6d6zpzJRn5MEq7+RnagyFIq9r+kbWzLGd+6QA=="
       },
       "System.Text.Encodings.Web": {
         "type": "Direct",
@@ -750,9 +750,9 @@
       },
       "Nullable": {
         "type": "Direct",
-        "requested": "[1.3.0, )",
-        "resolved": "1.3.0",
-        "contentHash": "xHAviTdTY3n+t1nEPN4JPRQR5lI124qRKVw+U9H7dO5sDNPpzoWeo/MQy7dSUmv9eD3k/CJVKokz1tFK+JOzRw=="
+        "requested": "[1.3.1, )",
+        "resolved": "1.3.1",
+        "contentHash": "Mk4ZVDfAORTjvckQprCSehi1XgOAAlk5ez06Va/acRYEloN9t6d6zpzJRn5MEq7+RnagyFIq9r+kbWzLGd+6QA=="
       },
       "System.Text.Encodings.Web": {
         "type": "Direct",
@@ -918,9 +918,9 @@
       },
       "Nullable": {
         "type": "Direct",
-        "requested": "[1.3.0, )",
-        "resolved": "1.3.0",
-        "contentHash": "xHAviTdTY3n+t1nEPN4JPRQR5lI124qRKVw+U9H7dO5sDNPpzoWeo/MQy7dSUmv9eD3k/CJVKokz1tFK+JOzRw=="
+        "requested": "[1.3.1, )",
+        "resolved": "1.3.1",
+        "contentHash": "Mk4ZVDfAORTjvckQprCSehi1XgOAAlk5ez06Va/acRYEloN9t6d6zpzJRn5MEq7+RnagyFIq9r+kbWzLGd+6QA=="
       },
       "System.Text.Encodings.Web": {
         "type": "Direct",

test/NodeJS/HttpNodeJSServiceUnitTests.cs

@@ -8,6 +8,7 @@
 using System.Net;
 using System.Net.Http;
 using System.Text;
+using System.Text.RegularExpressions;
 using System.Threading;
 using System.Threading.Tasks;
 using Xunit;
@@ -207,27 +208,28 @@ public async Task TryInvokeAsync_ThrowsInvocationExceptionIfHttpResponseHasAnUne
 
         [Theory]
         [MemberData(nameof(OnConnectionEstablishedMessageReceived_ExtractsEndPoint_Data))]
-        public void OnConnectionEstablishedMessageReceived_ExtractsEndPoint(string dummyIP, string dummyPort, string expectedResult)
+        public void OnConnectionEstablishedMessageReceived_ExtractsEndPoint(string dummyHttpVersion, string dummyIP, string dummyPort, string expectedResult)
         {
             // Arrange
             var loggerStringBuilder = new StringBuilder();
-            string dummyConnectionEstablishedMessage = $"[Jering.Javascript.NodeJS: HttpVersion - HTTP/1.1 Listening on IP - {dummyIP} Port - {dummyPort}]";
+            string dummyConnectionEstablishedMessage = $"[Jering.Javascript.NodeJS: HttpVersion - {dummyHttpVersion} Listening on IP - {dummyIP} Port - {dummyPort}]";
             ExposedHttpNodeJSService testSubject = CreateHttpNodeJSService(loggerStringBuilder: loggerStringBuilder);
 
             // Act
-            testSubject.ExposedOnConnectionEstablishedMessageReceived(dummyConnectionEstablishedMessage);
+            testSubject.ExposedOnConnectionEstablishedMessageReceived(testSubject.ExposedConnectionEstablishedMessageRegex.Match(dummyConnectionEstablishedMessage));
 
             // Assert
             Assert.Equal(expectedResult, testSubject._endpoint?.AbsoluteUri);
-            Assert.Contains(string.Format(Strings.LogInformation_HttpEndpoint, "HTTP/1.1", expectedResult), loggerStringBuilder.ToString());
+            Assert.Contains(string.Format(Strings.LogInformation_HttpEndpoint, dummyHttpVersion, expectedResult), loggerStringBuilder.ToString());
         }
 
         public static IEnumerable<object[]> OnConnectionEstablishedMessageReceived_ExtractsEndPoint_Data()
         {
             return new object[][]
             {
-                new object[]{"127.0.0.1", "12345", "http://127.0.0.1:12345/"}, // IPv4, arbitrary port
-                new object[]{"::1", "543", "http://[::1]:543/"} // IPv6, arbitrary port
+                new object[]{ "HTTP/1.1", "127.0.0.1", "12345", "http://127.0.0.1:12345/"}, // Http 1.1, IPv4, arbitrary port
+                new object[]{ "HTTP/1.1", "::1", "543", "http://[::1]:543/"}, // Http 1.1, IPv6, arbitrary port
+                new object[]{ "HTTP/2.0", "127.0.0.1", "12345", "http://127.0.0.1:12345/"} // Http 2.0, IPv4, arbitrary port
             };
         }
 
@@ -326,14 +328,16 @@ public ExposedHttpNodeJSService(IOptions<OutOfProcessNodeJSServiceOptions> outOf
             {
             }
 
+            public Regex ExposedConnectionEstablishedMessageRegex => ConnectionEstablishedMessageRegex;
+
             public Task<(bool, T?)> ExposedTryInvokeAsync<T>(InvocationRequest invocationRequest, CancellationToken cancellationToken)
             {
                 return TryInvokeAsync<T>(invocationRequest, cancellationToken);
             }
 
-            public void ExposedOnConnectionEstablishedMessageReceived(string connectionEstablishedMessage)
+            public void ExposedOnConnectionEstablishedMessageReceived(System.Text.RegularExpressions.Match connectionEstablishedMessageMatch)
             {
-                OnConnectionEstablishedMessageReceived(connectionEstablishedMessage);
+                OnConnectionEstablishedMessageReceived(connectionEstablishedMessageMatch);
             }
         }
     }