Skip to content

Commit 33997d5

Browse files
AndriySvyrydAndriySvyryd
committed
Add a DiagnosticSource event for the npm process
1 parent b63df18 commit 33997d5

File tree

7 files changed

+105
-54
lines changed

7 files changed

+105
-54
lines changed

‎src/Middleware/SpaServices.Extensions/src/AngularCli/AngularCliBuilder.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4+
using System;
5+
using System.Diagnostics;
6+
using System.IO;
7+
using System.Text.RegularExpressions;
8+
using System.Threading.Tasks;
49
using Microsoft.AspNetCore.Builder;
510
using Microsoft.AspNetCore.NodeServices.Npm;
611
using Microsoft.AspNetCore.NodeServices.Util;
712
using Microsoft.AspNetCore.SpaServices.Prerendering;
813
using Microsoft.AspNetCore.SpaServices.Util;
914
using Microsoft.Extensions.DependencyInjection;
1015
using Microsoft.Extensions.Hosting;
11-
using System;
12-
using System.IO;
13-
using System.Text.RegularExpressions;
14-
using System.Threading.Tasks;
1516

1617
namespace Microsoft.AspNetCore.SpaServices.AngularCli
1718
{
@@ -55,12 +56,14 @@ public async Task Build(ISpaBuilder spaBuilder)
5556
var logger = LoggerFinder.GetOrCreateLogger(
5657
appBuilder,
5758
nameof(AngularCliBuilder));
59+
var diagnosticSource = appBuilder.ApplicationServices.GetRequiredService<DiagnosticSource>();
5860
var scriptRunner = new NodeScriptRunner(
5961
sourcePath,
6062
_scriptName,
6163
"--watch",
6264
null,
6365
pkgManagerCommand,
66+
diagnosticSource,
6467
applicationStoppingToken);
6568
scriptRunner.AttachToLogger(logger);
6669

‎src/Middleware/SpaServices.Extensions/src/AngularCli/AngularCliMiddleware.cs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,21 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using Microsoft.AspNetCore.Builder;
5-
using Microsoft.Extensions.Logging;
6-
using Microsoft.AspNetCore.NodeServices.Npm;
7-
using Microsoft.AspNetCore.NodeServices.Util;
8-
using Microsoft.AspNetCore.SpaServices.Util;
94
using System;
5+
using System.Diagnostics;
106
using System.IO;
7+
using System.Net.Http;
118
using System.Text.RegularExpressions;
12-
using System.Threading.Tasks;
139
using System.Threading;
14-
using System.Net.Http;
10+
using System.Threading.Tasks;
11+
using Microsoft.AspNetCore.Builder;
12+
using Microsoft.AspNetCore.NodeServices.Npm;
13+
using Microsoft.AspNetCore.NodeServices.Util;
1514
using Microsoft.AspNetCore.SpaServices.Extensions.Util;
16-
using Microsoft.Extensions.Hosting;
15+
using Microsoft.AspNetCore.SpaServices.Util;
1716
using Microsoft.Extensions.DependencyInjection;
17+
using Microsoft.Extensions.Hosting;
18+
using Microsoft.Extensions.Logging;
1819

1920
namespace Microsoft.AspNetCore.SpaServices.AngularCli
2021
{
@@ -44,7 +45,8 @@ public static void Attach(
4445
var appBuilder = spaBuilder.ApplicationBuilder;
4546
var applicationStoppingToken = appBuilder.ApplicationServices.GetRequiredService<IHostApplicationLifetime>().ApplicationStopping;
4647
var logger = LoggerFinder.GetOrCreateLogger(appBuilder, LogCategoryName);
47-
var angularCliServerInfoTask = StartAngularCliServerAsync(sourcePath, scriptName, pkgManagerCommand, devServerPort, logger, applicationStoppingToken);
48+
var diagnosticSource = appBuilder.ApplicationServices.GetRequiredService<DiagnosticSource>();
49+
var angularCliServerInfoTask = StartAngularCliServerAsync(sourcePath, scriptName, pkgManagerCommand, devServerPort, logger, diagnosticSource, applicationStoppingToken);
4850

4951
// Everything we proxy is hardcoded to target http://localhost because:
5052
// - the requests are always from the local machine (we're not accepting remote
@@ -67,7 +69,7 @@ public static void Attach(
6769
}
6870

6971
private static async Task<AngularCliServerInfo> StartAngularCliServerAsync(
70-
string sourcePath, string scriptName, string pkgManagerCommand, int portNumber, ILogger logger, CancellationToken applicationStoppingToken)
72+
string sourcePath, string scriptName, string pkgManagerCommand, int portNumber, ILogger logger, DiagnosticSource diagnosticSource, CancellationToken applicationStoppingToken)
7173
{
7274
if (portNumber == default(int))
7375
{
@@ -76,7 +78,7 @@ private static async Task<AngularCliServerInfo> StartAngularCliServerAsync(
7678
logger.LogInformation($"Starting @angular/cli on port {portNumber}...");
7779

7880
var scriptRunner = new NodeScriptRunner(
79-
sourcePath, scriptName, $"--port {portNumber}", null, pkgManagerCommand, applicationStoppingToken);
81+
sourcePath, scriptName, $"--port {portNumber}", null, pkgManagerCommand, diagnosticSource, applicationStoppingToken);
8082
scriptRunner.AttachToLogger(logger);
8183

8284
Match openBrowserLine;

‎src/Middleware/SpaServices.Extensions/src/Npm/NodeScriptRunner.cs

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using Microsoft.Extensions.Logging;
5-
using Microsoft.AspNetCore.NodeServices.Util;
64
using System;
5+
using System.Collections.Generic;
76
using System.Diagnostics;
87
using System.Runtime.InteropServices;
98
using System.Text.RegularExpressions;
10-
using System.Collections.Generic;
119
using System.Threading;
10+
using Microsoft.AspNetCore.NodeServices.Util;
11+
using Microsoft.Extensions.Logging;
1212

1313
// This is under the NodeServices namespace because post 2.1 it will be moved to that package
1414
namespace Microsoft.AspNetCore.NodeServices.Npm
@@ -17,15 +17,15 @@ namespace Microsoft.AspNetCore.NodeServices.Npm
1717
/// Executes the <c>script</c> entries defined in a <c>package.json</c> file,
1818
/// capturing any output written to stdio.
1919
/// </summary>
20-
internal class NodeScriptRunner
20+
internal class NodeScriptRunner : IDisposable
2121
{
2222
private Process _npmProcess;
2323
public EventedStreamReader StdOut { get; }
2424
public EventedStreamReader StdErr { get; }
2525

2626
private static Regex AnsiColorRegex = new Regex("\x001b\\[[0-9;]*m", RegexOptions.None, TimeSpan.FromSeconds(1));
2727

28-
public NodeScriptRunner(string workingDirectory, string scriptName, string arguments, IDictionary<string, string> envVars, string pkgManagerCommand, CancellationToken applicationStoppingToken)
28+
public NodeScriptRunner(string workingDirectory, string scriptName, string arguments, IDictionary<string, string> envVars, string pkgManagerCommand, DiagnosticSource diagnosticSource, CancellationToken applicationStoppingToken)
2929
{
3030
if (string.IsNullOrEmpty(workingDirectory))
3131
{
@@ -75,7 +75,18 @@ public NodeScriptRunner(string workingDirectory, string scriptName, string argum
7575
StdOut = new EventedStreamReader(_npmProcess.StandardOutput);
7676
StdErr = new EventedStreamReader(_npmProcess.StandardError);
7777

78-
applicationStoppingToken.Register(EnsureNpmIsDead);
78+
applicationStoppingToken.Register(((IDisposable)this).Dispose);
79+
80+
if (diagnosticSource.IsEnabled("Microsoft.AspNetCore.NodeServices.Npm.NpmStarted"))
81+
{
82+
diagnosticSource.Write(
83+
"Microsoft.AspNetCore.NodeServices.Npm.NpmStarted",
84+
new
85+
{
86+
processStartInfo = processStartInfo,
87+
process = _npmProcess
88+
});
89+
}
7990
}
8091

8192
public void AttachToLogger(ILogger logger)
@@ -137,7 +148,7 @@ private static Process LaunchNodeProcess(ProcessStartInfo startInfo, string comm
137148
}
138149
}
139150

140-
private void EnsureNpmIsDead()
151+
void IDisposable.Dispose()
141152
{
142153
if (_npmProcess != null && !_npmProcess.HasExited)
143154
{

‎src/Middleware/SpaServices.Extensions/src/ReactDevelopmentServer/ReactDevelopmentServerMiddleware.cs

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,21 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using Microsoft.AspNetCore.Builder;
5-
using Microsoft.Extensions.Logging;
6-
using Microsoft.AspNetCore.NodeServices.Npm;
7-
using Microsoft.AspNetCore.NodeServices.Util;
8-
using Microsoft.AspNetCore.SpaServices.Util;
94
using System;
10-
using System.IO;
115
using System.Collections.Generic;
6+
using System.Diagnostics;
7+
using System.IO;
128
using System.Text.RegularExpressions;
9+
using System.Threading;
1310
using System.Threading.Tasks;
11+
using Microsoft.AspNetCore.Builder;
12+
using Microsoft.AspNetCore.NodeServices.Npm;
13+
using Microsoft.AspNetCore.NodeServices.Util;
1414
using Microsoft.AspNetCore.SpaServices.Extensions.Util;
15+
using Microsoft.AspNetCore.SpaServices.Util;
1516
using Microsoft.Extensions.DependencyInjection;
1617
using Microsoft.Extensions.Hosting;
17-
using System.Threading;
18+
using Microsoft.Extensions.Logging;
1819

1920
namespace Microsoft.AspNetCore.SpaServices.ReactDevelopmentServer
2021
{
@@ -44,7 +45,8 @@ public static void Attach(
4445
var appBuilder = spaBuilder.ApplicationBuilder;
4546
var applicationStoppingToken = appBuilder.ApplicationServices.GetRequiredService<IHostApplicationLifetime>().ApplicationStopping;
4647
var logger = LoggerFinder.GetOrCreateLogger(appBuilder, LogCategoryName);
47-
var portTask = StartCreateReactAppServerAsync(sourcePath, scriptName, pkgManagerCommand, devServerPort, logger, applicationStoppingToken);
48+
var diagnosticSource = appBuilder.ApplicationServices.GetRequiredService<DiagnosticSource>();
49+
var portTask = StartCreateReactAppServerAsync(sourcePath, scriptName, pkgManagerCommand, devServerPort, logger, diagnosticSource, applicationStoppingToken);
4850

4951
// Everything we proxy is hardcoded to target http://localhost because:
5052
// - the requests are always from the local machine (we're not accepting remote
@@ -67,7 +69,7 @@ public static void Attach(
6769
}
6870

6971
private static async Task<int> StartCreateReactAppServerAsync(
70-
string sourcePath, string scriptName, string pkgManagerCommand, int portNumber, ILogger logger, CancellationToken applicationStoppingToken)
72+
string sourcePath, string scriptName, string pkgManagerCommand, int portNumber, ILogger logger, DiagnosticSource diagnosticSource, CancellationToken applicationStoppingToken)
7173
{
7274
if (portNumber == default(int))
7375
{
@@ -81,7 +83,7 @@ private static async Task<int> StartCreateReactAppServerAsync(
8183
{ "BROWSER", "none" }, // We don't want create-react-app to open its own extra browser window pointing to the internal dev server port
8284
};
8385
var scriptRunner = new NodeScriptRunner(
84-
sourcePath, scriptName, null, envVars, pkgManagerCommand, applicationStoppingToken);
86+
sourcePath, scriptName, null, envVars, pkgManagerCommand, diagnosticSource, applicationStoppingToken);
8587
scriptRunner.AttachToLogger(logger);
8688

8789
using (var stdErrReader = new EventedStreamStringReader(scriptRunner.StdErr))

‎src/Middleware/SpaServices.Extensions/src/ReactDevelopmentServer/ReactDevelopmentServerMiddlewareExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using Microsoft.AspNetCore.Builder;
54
using System;
5+
using Microsoft.AspNetCore.Builder;
66

77
namespace Microsoft.AspNetCore.SpaServices.ReactDevelopmentServer
88
{

‎src/Middleware/SpaServices.Extensions/test/Microsoft.AspNetCore.SpaServices.Extensions.Tests.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@
33
<PropertyGroup>
44
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
55
<TestDependsOnNode>true</TestDependsOnNode>
6+
<BuildHelixPayload>false</BuildHelixPayload>
67
</PropertyGroup>
78

89
<ItemGroup>
910
<Reference Include="Microsoft.AspNetCore.SpaServices.Extensions" />
1011
<Reference Include="Microsoft.AspNetCore.Hosting" />
1112
<Reference Include="Microsoft.AspNetCore.TestHost" />
13+
<Reference Include="Microsoft.Extensions.DiagnosticAdapter" />
1214
<Reference Include="Microsoft.Extensions.Hosting" />
1315
<Reference Include="Microsoft.Extensions.Logging.Testing" />
1416
<Content Include="js\**\*" />

‎src/Middleware/SpaServices.Extensions/test/SpaServicesExtensionsTests.cs

Lines changed: 52 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,13 @@
88
using System.Text;
99
using System.Threading;
1010
using System.Threading.Tasks;
11-
using Castle.DynamicProxy.Generators.Emitters;
1211
using Microsoft.AspNetCore.Builder;
1312
using Microsoft.AspNetCore.Hosting;
1413
using Microsoft.AspNetCore.SpaServices.AngularCli;
1514
using Microsoft.AspNetCore.SpaServices.ReactDevelopmentServer;
1615
using Microsoft.AspNetCore.SpaServices.StaticFiles;
17-
using Microsoft.AspNetCore.Testing;
1816
using Microsoft.Extensions.DependencyInjection;
17+
using Microsoft.Extensions.DiagnosticAdapter;
1918
using Microsoft.Extensions.FileProviders;
2019
using Microsoft.Extensions.Hosting;
2120
using Microsoft.Extensions.Logging;
@@ -26,7 +25,7 @@ namespace Microsoft.AspNetCore.SpaServices.Extensions.Tests
2625
{
2726
public class SpaServicesExtensionsTests
2827
{
29-
[ConditionalFact]
28+
[Fact]
3029
public void UseSpa_ThrowsInvalidOperationException_IfRootpathNotSet()
3130
{
3231
// Arrange
@@ -39,57 +38,85 @@ public void UseSpa_ThrowsInvalidOperationException_IfRootpathNotSet()
3938
Assert.Equal("No RootPath was set on the SpaStaticFilesOptions.", exception.Message);
4039
}
4140

42-
[ConditionalFact]
41+
[Fact]
4342
public async Task UseSpa_KillsRds_WhenAppIsStopped()
4443
{
4544
var serviceProvider = GetServiceProvider(s => s.RootPath = "/");
4645
var applicationbuilder = new ApplicationBuilder(serviceProvider);
4746
var applicationLifetime = serviceProvider.GetRequiredService<IHostApplicationLifetime>();
47+
var diagnosticListener = serviceProvider.GetRequiredService<DiagnosticListener>();
48+
var listener = new NpmStartedDiagnosticListener();
49+
diagnosticListener.SubscribeWithAdapter(listener);
4850

4951
applicationbuilder.UseSpa(b =>
5052
{
5153
b.Options.SourcePath = Directory.GetCurrentDirectory();
52-
b.UseReactDevelopmentServer(GetCommand());
54+
b.UseReactDevelopmentServer(GetPlatformSpecificWaitCommand());
5355
});
5456

55-
// Give node a moment to start up
56-
await Task.Delay(TimeSpan.FromSeconds(3));
57-
var processCount = Process.GetProcesses().Length;
58-
59-
// Act
60-
applicationLifetime.StopApplication();
61-
62-
// Assert
63-
AssertNoErrors();
64-
Assert.True(Process.GetProcesses().Length < processCount, $"Expected less than {processCount} processes");
57+
await Assert_NpmKilled_WhenAppIsStopped(applicationLifetime, listener);
6558
}
6659

67-
[ConditionalFact]
60+
[Fact]
6861
public async Task UseSpa_KillsAngularCli_WhenAppIsStopped()
6962
{
7063
var serviceProvider = GetServiceProvider(s => s.RootPath = "/");
7164
var applicationbuilder = new ApplicationBuilder(serviceProvider);
7265
var applicationLifetime = serviceProvider.GetRequiredService<IHostApplicationLifetime>();
66+
var diagnosticListener = serviceProvider.GetRequiredService<DiagnosticListener>();
67+
var listener = new NpmStartedDiagnosticListener();
68+
diagnosticListener.SubscribeWithAdapter(listener);
7369

7470
applicationbuilder.UseSpa(b =>
7571
{
7672
b.Options.SourcePath = Directory.GetCurrentDirectory();
77-
b.UseAngularCliServer(GetCommand());
73+
b.UseAngularCliServer(GetPlatformSpecificWaitCommand());
7874
});
7975

76+
await Assert_NpmKilled_WhenAppIsStopped(applicationLifetime, listener);
77+
}
78+
79+
private async Task Assert_NpmKilled_WhenAppIsStopped(IHostApplicationLifetime applicationLifetime, NpmStartedDiagnosticListener listener)
80+
{
8081
// Give node a moment to start up
81-
await Task.Delay(TimeSpan.FromSeconds(3));
82-
var processCount = Process.GetProcesses().Length;
82+
await Task.WhenAny(listener.NpmStarted, Task.Delay(TimeSpan.FromSeconds(30)));
83+
84+
Process npmProcess = null;
85+
var npmExitEvent = new ManualResetEventSlim();
86+
if (listener.NpmStarted.IsCompleted)
87+
{
88+
npmProcess = listener.NpmStarted.Result.Process;
89+
Assert.False(npmProcess.HasExited);
90+
npmProcess.Exited += (_, __) => npmExitEvent.Set();
91+
}
8392

8493
// Act
8594
applicationLifetime.StopApplication();
8695

8796
// Assert
8897
AssertNoErrors();
89-
Assert.True(Process.GetProcesses().Length < processCount, $"Expected less than {processCount} processes");
98+
Assert.True(listener.NpmStarted.IsCompleted, "npm wasn't launched");
99+
100+
npmExitEvent.Wait(TimeSpan.FromSeconds(30));
101+
Assert.True(npmProcess.HasExited, "npm wasn't killed");
90102
}
91103

92-
private string GetCommand()
104+
private class NpmStartedDiagnosticListener
105+
{
106+
private readonly TaskCompletionSource<(ProcessStartInfo ProcessStartInfo, Process Process)> _npmStartedTaskCompletionSource
107+
= new TaskCompletionSource<(ProcessStartInfo ProcessStartInfo, Process Process)>();
108+
109+
public Task<(ProcessStartInfo ProcessStartInfo, Process Process)> NpmStarted
110+
=> _npmStartedTaskCompletionSource.Task;
111+
112+
[DiagnosticName("Microsoft.AspNetCore.NodeServices.Npm.NpmStarted")]
113+
public virtual void OnNpmStarted(ProcessStartInfo processStartInfo, Process process)
114+
{
115+
_npmStartedTaskCompletionSource.TrySetResult((processStartInfo, process));
116+
}
117+
}
118+
119+
private string GetPlatformSpecificWaitCommand()
93120
=> RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "waitWindows" : "wait";
94121

95122
private IApplicationBuilder GetApplicationBuilder(IServiceProvider serviceProvider = null)
@@ -116,6 +143,10 @@ private IServiceProvider GetServiceProvider(Action<SpaStaticFilesOptions> config
116143
services.AddSingleton(typeof(IHostApplicationLifetime), new TestHostApplicationLifetime());
117144
services.AddSingleton(typeof(IWebHostEnvironment), new TestWebHostEnvironment());
118145

146+
var listener = new DiagnosticListener("Microsoft.AspNetCore");
147+
services.AddSingleton(listener);
148+
services.AddSingleton<DiagnosticSource>(listener);
149+
119150
return services.BuildServiceProvider();
120151
}
121152

0 commit comments

Comments
 (0)