Merge pull request #101 from JeringTech/improve-robustness

Improve robustness

Author: JeremyTCD

Changelog.md

@@ -3,7 +3,15 @@ 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.0.0-beta.1...HEAD)
+## [Unreleased](https://github.com/JeringTech/Javascript.NodeJS/compare/6.0.0-beta.2...HEAD)
+
+## [6.0.0-beta.2](https://github.com/JeringTech/Javascript.NodeJS/compare/6.0.0-beta.1...6.0.0-beta.2) - Feb 24, 2021
+### Additions
+- Added `OutOfProcessNodeJSServiceOptions.NumProcessRetries` option. ([#101](https://github.com/JeringTech/Javascript.NodeJS/pull/101).
+### Changes
+- `HttpNodeJSService` now logs endpoint on connect. Logged at `information` level. ([#101](https://github.com/JeringTech/Javascript.NodeJS/pull/101)).
+- **Breaking**: Renamed `OutOfProcessNodeJSServiceOptions.WatchGracefulShutdown` to `GracefulProcessShutdown`. Option now affects
+process shutdowns on-file-change *and* when retrying invocations. ([#101](https://github.com/JeringTech/Javascript.NodeJS/pull/101)).
 
 ## [6.0.0-beta.1](https://github.com/JeringTech/Javascript.NodeJS/compare/6.0.0-beta.0...6.0.0-beta.1) - Feb 22, 2021
 ### Fixes

perf/NodeJS/ConcurrencyBenchmarks.cs

@@ -13,7 +13,7 @@ public class ConcurrencyBenchmarks
     {
         private const string DUMMY_WARMUP_MODULE = "module.exports = (callback) => callback()";
         private const string DUMMY_CONCURRENCY_MODULE_FILE = "dummyConcurrencyModule.js";
-        private static readonly string PROJECT_PATH = Path.Combine(Directory.GetCurrentDirectory(), "../../../../../../../Javascript");  // BenchmarkDotNet creates a project nested deep in bin
+        private static readonly string _projectPath = Path.Combine(Directory.GetCurrentDirectory(), "../../../../../../../Javascript");  // BenchmarkDotNet creates a project nested deep in bin
 
         private ServiceProvider _serviceProvider;
         private INodeJSService _nodeJSService;
@@ -25,7 +25,7 @@ public void INodeJSService_Concurrency_MultiProcess_Setup()
         {
             var services = new ServiceCollection();
             services.AddNodeJS();
-            services.Configure<NodeJSProcessOptions>(options => options.ProjectPath = PROJECT_PATH);
+            services.Configure<NodeJSProcessOptions>(options => options.ProjectPath = _projectPath);
             services.Configure<OutOfProcessNodeJSServiceOptions>(options => options.Concurrency = Concurrency.MultiProcess);
             _serviceProvider = services.BuildServiceProvider();
             _nodeJSService = _serviceProvider.GetRequiredService<INodeJSService>();
@@ -57,7 +57,7 @@ public void INodeJSService_Concurrency_None_Setup()
         {
             var services = new ServiceCollection();
             services.AddNodeJS();
-            services.Configure<NodeJSProcessOptions>(options => options.ProjectPath = PROJECT_PATH);
+            services.Configure<NodeJSProcessOptions>(options => options.ProjectPath = _projectPath);
             _serviceProvider = services.BuildServiceProvider();
             _nodeJSService = _serviceProvider.GetRequiredService<INodeJSService>();
 
@@ -86,7 +86,7 @@ public void INodeServices_Concurrency_Setup()
             var services = new ServiceCollection();
             services.AddNodeServices(options =>
             {
-                options.ProjectPath = PROJECT_PATH;
+                options.ProjectPath = _projectPath;
                 options.WatchFileExtensions = null;
             });
             _serviceProvider = services.BuildServiceProvider();

perf/NodeJS/FileWatchingBenchmarks.cs

@@ -40,7 +40,7 @@ public void HttpNodeJSService_FileWatching_GracefulShutdownDisabled_MoveToNewPro
             services.Configure<OutOfProcessNodeJSServiceOptions>(options =>
             {
                 options.EnableFileWatching = true;
-                options.WatchGracefulShutdown = false;
+                options.GracefulProcessShutdown = false;
             });
             _serviceProvider = services.BuildServiceProvider();
             _httpNodeJSService = _serviceProvider.GetRequiredService<INodeJSService>() as HttpNodeJSService;

perf/NodeJS/LatencyBenchmarks.cs

@@ -14,7 +14,7 @@ public class LatencyBenchmarks
         private const string DUMMY_WARMUP_MODULE = "module.exports = (callback) => callback()";
         private const string DUMMY_LATENCY_MODULE_FILE = "dummyLatencyModule.js";
         private const string DUMMY_MODULE_IDENTIFIER = "dummyLatencyModuleIdentifier";
-        private static readonly string PROJECT_PATH = Path.Combine(Directory.GetCurrentDirectory(), "../../../../../../../Javascript"); // BenchmarkDotNet creates a project nested deep in bin
+        private static readonly string _projectPath = Path.Combine(Directory.GetCurrentDirectory(), "../../../../../../../Javascript"); // BenchmarkDotNet creates a project nested deep in bin
 
         private ServiceProvider _serviceProvider;
         private int _counter;
@@ -27,7 +27,7 @@ public void INodeJSService_Latency_InvokeFromFile_Setup()
         {
             var services = new ServiceCollection();
             services.AddNodeJS();
-            services.Configure<NodeJSProcessOptions>(options => options.ProjectPath = PROJECT_PATH);
+            services.Configure<NodeJSProcessOptions>(options => options.ProjectPath = _projectPath);
             _serviceProvider = services.BuildServiceProvider();
             _nodeJSService = _serviceProvider.GetRequiredService<INodeJSService>();
             _counter = 0;
@@ -50,7 +50,7 @@ public void INodeJSService_Latency_InvokeFromFile_GracefulShutdownEnabled_Setup(
         {
             var services = new ServiceCollection();
             services.AddNodeJS();
-            services.Configure<NodeJSProcessOptions>(options => options.ProjectPath = PROJECT_PATH);
+            services.Configure<NodeJSProcessOptions>(options => options.ProjectPath = _projectPath);
             services.Configure<OutOfProcessNodeJSServiceOptions>(options => options.EnableFileWatching = true);
             _serviceProvider = services.BuildServiceProvider();
             _nodeJSService = _serviceProvider.GetRequiredService<INodeJSService>();
@@ -87,7 +87,7 @@ public async Task<DummyResult> INodeJSService_Latency_InvokeFromCache()
 
         private string DummyModuleFactory()
         {
-            return File.ReadAllText(Path.Combine(PROJECT_PATH, DUMMY_LATENCY_MODULE_FILE));
+            return File.ReadAllText(Path.Combine(_projectPath, DUMMY_LATENCY_MODULE_FILE));
         }
 
         [Obsolete]
@@ -97,7 +97,7 @@ public void INodeServices_Latency_Setup()
             var services = new ServiceCollection();
             services.AddNodeServices(options =>
             {
-                options.ProjectPath = PROJECT_PATH;
+                options.ProjectPath = _projectPath;
                 options.WatchFileExtensions = null;
             });
             _serviceProvider = services.BuildServiceProvider();

perf/NodeJS/RealWorkloadBenchmarks.cs

@@ -22,7 +22,7 @@ public static void Main(string[] args)
         Console.WriteLine(""Hello world {0}!"");
     }}
 }}";
-        private static readonly string PROJECT_PATH = Path.Combine(Directory.GetCurrentDirectory(), "../../../../../../../Javascript"); // BenchmarkDotNet creates a project nested deep in bin
+        private static readonly string _projectPath = Path.Combine(Directory.GetCurrentDirectory(), "../../../../../../../Javascript"); // BenchmarkDotNet creates a project nested deep in bin
 
         private int _counter;
         private ServiceProvider _serviceProvider;
@@ -35,7 +35,7 @@ public void INodeJSService_RealWorkload_Setup()
         {
             var services = new ServiceCollection();
             services.AddNodeJS();
-            services.Configure<NodeJSProcessOptions>(options => options.ProjectPath = PROJECT_PATH); // Module loads prismjs from node_modules
+            services.Configure<NodeJSProcessOptions>(options => options.ProjectPath = _projectPath); // Module loads prismjs from node_modules
             services.Configure<OutOfProcessNodeJSServiceOptions>(options => options.Concurrency = Concurrency.MultiProcess);
             _serviceProvider = services.BuildServiceProvider();
             _nodeJSService = _serviceProvider.GetRequiredService<INodeJSService>();
@@ -66,7 +66,7 @@ public async Task<string[]> INodeJSService_RealWorkload()
 
         private string DummyModuleFactory()
         {
-            return File.ReadAllText(Path.Combine(PROJECT_PATH, DUMMY_REAL_WORKLOAD_MODULE_FILE));
+            return File.ReadAllText(Path.Combine(_projectPath, DUMMY_REAL_WORKLOAD_MODULE_FILE));
         }
 
         [Obsolete]
@@ -76,7 +76,7 @@ public void INodeServices_RealWorkload_Setup()
             var services = new ServiceCollection();
             services.AddNodeServices(options =>
             {
-                options.ProjectPath = PROJECT_PATH;
+                options.ProjectPath = _projectPath;
                 options.WatchFileExtensions = null;
             });
             _serviceProvider = services.BuildServiceProvider();

src/NodeJS/Jering.Javascript.NodeJS.csproj

@@ -6,7 +6,7 @@
         <PackageId>Jering.Javascript.NodeJS</PackageId>
         <Authors>JeremyTCD</Authors>
         <Title>Invoke Javascript in NodeJS, from C#</Title>
-        <Description>Jering.Javascript.NodeJS enables you to invoke javascript in NodeJS, from C#. With this ability, you can use javascript libraries and scripts from your C# projects.</Description>
+        <Description>Jering.Javascript.NodeJS enables you to invoke javascript in NodeJS, from C#. With this ability, you can use javascript libraries and scripts from C# projects.</Description>
         <Copyright>© 2018-2021 Jering. All rights reserved.</Copyright>
         <PackageProjectUrl>https://www.jering.tech/utilities/jering.javascript.nodejs/index</PackageProjectUrl>
         <RepositoryUrl>https://github.com/JeringTech/Javascript.NodeJS</RepositoryUrl>

src/NodeJS/NodeJSServiceCollectionExtensions.cs

@@ -2,7 +2,6 @@
 using Microsoft.Extensions.Options;
 using System;
 using System.Collections.ObjectModel;
-using System.Net.Http;
 using System.Threading;
 
 namespace Jering.Javascript.NodeJS
@@ -22,10 +21,9 @@ public static IServiceCollection AddNodeJS(this IServiceCollection services)
             services.
                 AddLogging().
                 AddOptions();
-            services.AddHttpClient();
 
             // Services defined in this project
-            return services.
+            services.
                 AddSingleton<IConfigureOptions<NodeJSProcessOptions>, ConfigureNodeJSProcessOptions>().
                 AddSingleton<IHttpContentFactory, InvocationContentFactory>().
                 AddSingleton<IEmbeddedResourcesService, EmbeddedResourcesService>().
@@ -35,26 +33,16 @@ public static IServiceCollection AddNodeJS(this IServiceCollection services)
                 AddSingleton<IEnvironmentService, EnvironmentService>().
                 AddSingleton<IFileWatcherFactory, FileWatcherFactory>().
                 AddSingleton<IMonitorService, MonitorService>().
-                AddSingleton<ITaskService, TaskService>().
-                AddSingleton(IHttpClientServiceFactory);
-        }
-
-        internal static IHttpClientService IHttpClientServiceFactory(IServiceProvider serviceProvider)
-        {
+                AddSingleton<ITaskService, TaskService>();
 #if NETCOREAPP3_1
             // If not called, framework forces HTTP/1.1 so long as origin isn't https
             AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", true);
 #endif
+            services.
+                AddHttpClient<IHttpClientService, HttpClientService>().
+                SetHandlerLifetime(Timeout.InfiniteTimeSpan); // No DNS changes, handler lifetime can be infinite
 
-            // Create client
-            OutOfProcessNodeJSServiceOptions outOfProcessNodeJSServiceOptions = serviceProvider.GetRequiredService<IOptions<OutOfProcessNodeJSServiceOptions>>().Value;
-            IHttpClientFactory httpClientFactory = serviceProvider.GetRequiredService<IHttpClientFactory>();
-            HttpClient httpClient = httpClientFactory.CreateClient();
-
-            // Configure
-            httpClient.Timeout = outOfProcessNodeJSServiceOptions.TimeoutMS == -1 ? Timeout.InfiniteTimeSpan : TimeSpan.FromMilliseconds(outOfProcessNodeJSServiceOptions.TimeoutMS + 1000);
-
-            return new HttpClientService(httpClient);
+            return services;
         }
 
         internal static INodeJSService INodeJSServiceFactory(IServiceProvider serviceProvider)

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

@@ -1,4 +1,5 @@
-using System;
+using Microsoft.Extensions.Options;
+using System;
 using System.Net.Http;
 using System.Threading;
 using System.Threading.Tasks;
@@ -19,9 +20,16 @@ public class HttpClientService : IHttpClientService
         /// Creates a <see cref="HttpClientService"/>.
         /// </summary>
         /// <param name="httpClient">The <see cref="HttpClient"/> to send HTTP requests with.</param>
-        public HttpClientService(HttpClient httpClient)
+        /// <param name="outOfProcessNodeJSServiceOptionsAccessor"></param>
+        public HttpClientService(HttpClient httpClient,
+            IOptions<OutOfProcessNodeJSServiceOptions> outOfProcessNodeJSServiceOptionsAccessor)
         {
             _httpClient = httpClient;
+
+            // Configure
+            OutOfProcessNodeJSServiceOptions outOfProcessNodeJSServiceOptions = outOfProcessNodeJSServiceOptionsAccessor.Value;
+            httpClient.Timeout = outOfProcessNodeJSServiceOptions.TimeoutMS == -1 ? System.Threading.Timeout.InfiniteTimeSpan :
+                TimeSpan.FromMilliseconds(outOfProcessNodeJSServiceOptions.TimeoutMS + 1000);
         }
 
         /// <inheritdoc />

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

@@ -22,6 +22,7 @@ public class HttpNodeJSService : OutOfProcessNodeJSService
 
         private readonly IHttpContentFactory _httpContentFactory;
         private readonly IJsonService _jsonService;
+        private readonly ILogger<HttpNodeJSService> _logger;
         private readonly IHttpClientService _httpClientService;
 
         private bool _disposed;
@@ -41,7 +42,7 @@ public class HttpNodeJSService : OutOfProcessNodeJSService
         /// <param name="httpClientService"></param>
         /// <param name="jsonService"></param>
         /// <param name="nodeJSProcessFactory"></param>
-        /// <param name="loggerFactory"></param>
+        /// <param name="logger"></param>
         public HttpNodeJSService(IOptions<OutOfProcessNodeJSServiceOptions> outOfProcessNodeJSServiceOptionsAccessor,
             IHttpContentFactory httpContentFactory,
             IEmbeddedResourcesService embeddedResourcesService,
@@ -51,9 +52,9 @@ public HttpNodeJSService(IOptions<OutOfProcessNodeJSServiceOptions> outOfProcess
             IHttpClientService httpClientService,
             IJsonService jsonService,
             INodeJSProcessFactory nodeJSProcessFactory,
-            ILoggerFactory loggerFactory) :
+            ILogger<HttpNodeJSService> logger) :
             base(nodeJSProcessFactory,
-                loggerFactory.CreateLogger(typeof(HttpNodeJSService)),
+                logger,
                 outOfProcessNodeJSServiceOptionsAccessor,
                 embeddedResourcesService,
                 fileWatcherFactory,
@@ -64,41 +65,10 @@ public HttpNodeJSService(IOptions<OutOfProcessNodeJSServiceOptions> outOfProcess
         {
             _httpClientService = httpClientService;
             _jsonService = jsonService;
+            _logger = logger;
             _httpContentFactory = httpContentFactory;
         }
 
-        // DO NOT DELETE - keep for backward compatibility.
-        /// <summary>
-        /// <para>Creates a <see cref="HttpNodeJSService"/>.</para> 
-        /// <para>If this constructor is used, file watching is disabled.</para>
-        /// </summary>
-        /// <param name="outOfProcessNodeJSServiceOptionsAccessor"></param>
-        /// <param name="httpContentFactory"></param>
-        /// <param name="embeddedResourcesService"></param>
-        /// <param name="httpClientService"></param>
-        /// <param name="jsonService"></param>
-        /// <param name="nodeJSProcessFactory"></param>
-        /// <param name="loggerFactory"></param>
-        public HttpNodeJSService(IOptions<OutOfProcessNodeJSServiceOptions> outOfProcessNodeJSServiceOptionsAccessor,
-            IHttpContentFactory httpContentFactory,
-            IEmbeddedResourcesService embeddedResourcesService,
-            IHttpClientService httpClientService,
-            IJsonService jsonService,
-            INodeJSProcessFactory nodeJSProcessFactory,
-            ILoggerFactory loggerFactory) :
-            this(outOfProcessNodeJSServiceOptionsAccessor,
-                httpContentFactory,
-                embeddedResourcesService,
-                null,
-                null,
-                null,
-                httpClientService,
-                jsonService,
-                nodeJSProcessFactory,
-                loggerFactory)
-        {
-        }
-
         /// <inheritdoc />
         protected override async Task<(bool, T)> TryInvokeAsync<T>(InvocationRequest invocationRequest, CancellationToken cancellationToken)
         {
@@ -201,6 +171,7 @@ protected override void OnConnectionEstablishedMessageReceived(string connection
                 else if (currentChar == ']')
                 {
                     _endpoint = new Uri(stringBuilder.ToString());
+                    _logger.LogInformation(string.Format(Strings.LogInformation_HttpEndpoint, _endpoint));
                     return;
                 }
                 else

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

@@ -13,7 +13,7 @@ public interface IHttpClientService
         /// <summary>
         /// Gets or sets the timespan to wait before the request times out.
         /// </summary>
-        TimeSpan Timeout {get; set;}
+        TimeSpan Timeout { get; set; }
 
         /// <summary>
         /// Send an HTTP request as an asynchronous operation.

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

@@ -16,9 +16,9 @@ namespace Jering.Javascript.NodeJS
     public class InvocationContent : HttpContent
     {
         // Arbitrary boundary
-        internal static readonly byte[] BOUNDARY_BYTES = Encoding.UTF8.GetBytes("--Uiw6+hXl3k+5ia0cUYGhjA==");
+        internal static readonly byte[] _boundaryBytes = Encoding.UTF8.GetBytes("--Uiw6+hXl3k+5ia0cUYGhjA==");
 
-        private static readonly MediaTypeHeaderValue MULTIPART_CONTENT_TYPE = new MediaTypeHeaderValue("multipart/mixed");
+        private static readonly MediaTypeHeaderValue _multipartContentType = new MediaTypeHeaderValue("multipart/mixed");
         private readonly IJsonService _jsonService;
         private readonly InvocationRequest _invocationRequest;
 
@@ -34,7 +34,7 @@ public InvocationContent(IJsonService jsonService, InvocationRequest invocationR
 
             if (invocationRequest.ModuleSourceType == ModuleSourceType.Stream)
             {
-                Headers.ContentType = MULTIPART_CONTENT_TYPE;
+                Headers.ContentType = _multipartContentType;
             }
         }
 
@@ -50,7 +50,7 @@ protected override async Task SerializeToStreamAsync(Stream stream, TransportCon
 
             if (_invocationRequest.ModuleSourceType == ModuleSourceType.Stream)
             {
-                await stream.WriteAsync(BOUNDARY_BYTES, 0, BOUNDARY_BYTES.Length).ConfigureAwait(false);
+                await stream.WriteAsync(_boundaryBytes, 0, _boundaryBytes.Length).ConfigureAwait(false);
                 await _invocationRequest.ModuleStreamSource.CopyToAsync(stream).ConfigureAwait(false);
             }
         }