When a SPA dev server (or prerendering build) takes too long to start up, only fail current request, not future requests. Fixes aspnet#1447

SteveSandersonMS · SteveSandersonMS · commit a98c1459b528 · 2018-01-02T14:15:20.000Z
diff --git a/src/Microsoft.AspNetCore.SpaServices.Extensions/AngularCli/AngularCliBuilder.cs b/src/Microsoft.AspNetCore.SpaServices.Extensions/AngularCli/AngularCliBuilder.cs
@@ -20,7 +20,6 @@ namespace Microsoft.AspNetCore.SpaServices.AngularCli
 public class AngularCliBuilder : ISpaPrerendererBuilder
{
 private static TimeSpan RegexMatchTimeout = TimeSpan.FromSeconds(5); // This is a development-time only feature, so a very long timeout is fine
- private static TimeSpan BuildTimeout = TimeSpan.FromSeconds(50); // Note that the HTTP request itself by default times out after 60s, so you only get useful error information if this is shorter
 
 private readonly string _npmScriptName;
 
@@ -63,8 +62,7 @@ public async Task Build(ISpaBuilder spaBuilder)
 try
{
 await npmScriptRunner.StdOut.WaitForMatch(
- new Regex("Date", RegexOptions.None, RegexMatchTimeout),
- BuildTimeout);
+ new Regex("Date", RegexOptions.None, RegexMatchTimeout));
 }
 catch (EndOfStreamException ex)
{
diff --git a/src/Microsoft.AspNetCore.SpaServices.Extensions/AngularCli/AngularCliMiddleware.cs b/src/Microsoft.AspNetCore.SpaServices.Extensions/AngularCli/AngularCliMiddleware.cs
@@ -12,6 +12,7 @@
 using System.Threading.Tasks;
 using System.Threading;
 using System.Net.Http;
+using Microsoft.AspNetCore.SpaServices.Extensions.Util;
 
 namespace Microsoft.AspNetCore.SpaServices.AngularCli
{
@@ -49,7 +50,15 @@ public static void Attach(
 var targetUriTask = angularCliServerInfoTask.ContinueWith(
 task => new UriBuilder("http", "localhost", task.Result.Port).Uri);
 
- SpaProxyingExtensions.UseProxyToSpaDevelopmentServer(spaBuilder, targetUriTask);
+ SpaProxyingExtensions.UseProxyToSpaDevelopmentServer(spaBuilder, () =>
+{
+ // On each request, we create a separate startup task with its own timeout. That way, even if
+ // the first request times out, subsequent requests could still work.
+ return targetUriTask.WithTimeout(StartupTimeout,
+ $"The Angular CLI process did not start listening for requests " +
+ $"within the timeout period of{StartupTimeout.Seconds} seconds. " +
+ $"Check the log output for error information.");
+ });
 }
 
 private static async Task<AngularCliServerInfo> StartAngularCliServerAsync(
@@ -68,8 +77,7 @@ private static async Task<AngularCliServerInfo> StartAngularCliServerAsync(
 try
{
 openBrowserLine = await npmScriptRunner.StdOut.WaitForMatch(
- new Regex("open your browser on (http\\S+)", RegexOptions.None, RegexMatchTimeout),
- StartupTimeout);
+ new Regex("open your browser on (http\\S+)", RegexOptions.None, RegexMatchTimeout));
 }
 catch (EndOfStreamException ex)
{
@@ -78,13 +86,6 @@ private static async Task<AngularCliServerInfo> StartAngularCliServerAsync(
 $"Angular CLI was listening for requests. The error output was: " +
 $"{stdErrReader.ReadAsString()}", ex);
 }
- catch (TaskCanceledException ex)
-{
- throw new InvalidOperationException(
- $"The Angular CLI process did not start listening for requests " +
- $"within the timeout period of{StartupTimeout.Seconds} seconds. " +
- $"Check the log output for error information.", ex);
- }
 }
 
 var uri = new Uri(openBrowserLine.Groups[1].Value);
diff --git a/src/Microsoft.AspNetCore.SpaServices.Extensions/Prerendering/SpaPrerenderingExtensions.cs b/src/Microsoft.AspNetCore.SpaServices.Extensions/Prerendering/SpaPrerenderingExtensions.cs
@@ -6,6 +6,7 @@
 using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.NodeServices;
 using Microsoft.AspNetCore.SpaServices;
+using Microsoft.AspNetCore.SpaServices.Extensions.Util;
 using Microsoft.AspNetCore.SpaServices.Prerendering;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Net.Http.Headers;
@@ -23,6 +24,8 @@ namespace Microsoft.AspNetCore.Builder
 /// </summary>
 public static class SpaPrerenderingExtensions
{
+ private static TimeSpan BuildTimeout = TimeSpan.FromSeconds(50); // Note that the HTTP request itself by default times out after 60s, so you only get useful error information if this is shorter
+
 /// <summary>
 /// Enables server-side prerendering middleware for a Single Page Application.
 /// </summary>
@@ -85,9 +88,15 @@ public static void UseSpaPrerendering(
 }
 
 // If we're building on demand, wait for that to finish, or raise any build errors
- if (buildOnDemandTask != null)
+ if (buildOnDemandTask != null && !buildOnDemandTask.IsCompleted)
{
- await buildOnDemandTask;
+ // For better debuggability, create a per-request timeout that makes it clear if the
+ // prerendering builder took too long for this request, but without aborting the
+ // underlying build task so that subsequent requests could still work.
+ await buildOnDemandTask.WithTimeout(BuildTimeout,
+ $"The prerendering build process did not complete within the " +
+ $"timeout period of{BuildTimeout.Seconds} seconds. " +
+ $"Check the log output for error information.");
 }
 
 // It's no good if we try to return a 304. We need to capture the actual
diff --git a/src/Microsoft.AspNetCore.SpaServices.Extensions/Proxying/SpaProxyingExtensions.cs b/src/Microsoft.AspNetCore.SpaServices.Extensions/Proxying/SpaProxyingExtensions.cs
@@ -45,7 +45,7 @@ public static void UseProxyToSpaDevelopmentServer(
{
 UseProxyToSpaDevelopmentServer(
 spaBuilder,
- Task.FromResult(baseUri));
+ () => Task.FromResult(baseUri));
 }
 
 /// <summary>
@@ -54,10 +54,10 @@ public static void UseProxyToSpaDevelopmentServer(
 /// development. Do not enable this middleware in production applications.
 /// </summary>
 /// <param name="spaBuilder">The <see cref="ISpaBuilder"/>.</param>
- /// <param name="baseUriTask">A <see cref="Task"/> that resolves with the target base URI to which requests should be proxied.</param>
+ /// <param name="baseUriTaskFactory">A callback that will be invoked on each request to supply a <see cref="Task"/> that resolves with the target base URI to which requests should be proxied.</param>
 public static void UseProxyToSpaDevelopmentServer(
 this ISpaBuilder spaBuilder,
- Task<Uri> baseUriTask)
+ Func<Task<Uri>> baseUriTaskFactory)
{
 var applicationBuilder = spaBuilder.ApplicationBuilder;
 var applicationStoppingToken = GetStoppingToken(applicationBuilder);
@@ -72,11 +72,11 @@ public static void UseProxyToSpaDevelopmentServer(
 var neverTimeOutHttpClient =
 SpaProxy.CreateHttpClientForProxy(Timeout.InfiniteTimeSpan);
 
- // Proxy all requests into the Angular CLI server
+ // Proxy all requests to the SPA development server
 applicationBuilder.Use(async (context, next) =>
{
 var didProxyRequest = await SpaProxy.PerformProxyRequest(
- context, neverTimeOutHttpClient, baseUriTask, applicationStoppingToken,
+ context, neverTimeOutHttpClient, baseUriTaskFactory(), applicationStoppingToken,
 proxy404s: true);
 });
 }
diff --git a/src/Microsoft.AspNetCore.SpaServices.Extensions/ReactDevelopmentServer/ReactDevelopmentServerMiddleware.cs b/src/Microsoft.AspNetCore.SpaServices.Extensions/ReactDevelopmentServer/ReactDevelopmentServerMiddleware.cs
@@ -11,6 +11,7 @@
 using System.Collections.Generic;
 using System.Text.RegularExpressions;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.SpaServices.Extensions.Util;
 
 namespace Microsoft.AspNetCore.SpaServices.ReactDevelopmentServer
{
@@ -48,7 +49,15 @@ public static void Attach(
 var targetUriTask = portTask.ContinueWith(
 task => new UriBuilder("http", "localhost", task.Result).Uri);
 
- SpaProxyingExtensions.UseProxyToSpaDevelopmentServer(spaBuilder, targetUriTask);
+ SpaProxyingExtensions.UseProxyToSpaDevelopmentServer(spaBuilder, () =>
+{
+ // On each request, we create a separate startup task with its own timeout. That way, even if
+ // the first request times out, subsequent requests could still work.
+ return targetUriTask.WithTimeout(StartupTimeout,
+ $"The create-react-app server did not start listening for requests " +
+ $"within the timeout period of{StartupTimeout.Seconds} seconds. " +
+ $"Check the log output for error information.");
+ });
 }
 
 private static async Task<int> StartCreateReactAppServerAsync(
@@ -75,8 +84,7 @@ private static async Task<int> StartCreateReactAppServerAsync(
 // no compiler warnings. So instead of waiting for that, consider it ready as soon
 // as it starts listening for requests.
 await npmScriptRunner.StdOut.WaitForMatch(
- new Regex("Starting the development server", RegexOptions.None, RegexMatchTimeout),
- StartupTimeout);
+ new Regex("Starting the development server", RegexOptions.None, RegexMatchTimeout));
 }
 catch (EndOfStreamException ex)
{
@@ -85,13 +93,6 @@ await npmScriptRunner.StdOut.WaitForMatch(
 $"create-react-app server was listening for requests. The error output was: " +
 $"{stdErrReader.ReadAsString()}", ex);
 }
- catch (TaskCanceledException ex)
-{
- throw new InvalidOperationException(
- $"The create-react-app server did not start listening for requests " +
- $"within the timeout period of{StartupTimeout.Seconds} seconds. " +
- $"Check the log output for error information.", ex);
- }
 }
 
 return portNumber;
diff --git a/src/Microsoft.AspNetCore.SpaServices.Extensions/Util/EventedStreamReader.cs b/src/Microsoft.AspNetCore.SpaServices.Extensions/Util/EventedStreamReader.cs
@@ -34,7 +34,7 @@ public EventedStreamReader(StreamReader streamReader)
 Task.Factory.StartNew(Run);
 }
 
- public Task<Match> WaitForMatch(Regex regex, TimeSpan timeout = default)
+ public Task<Match> WaitForMatch(Regex regex)
{
 var tcs = new TaskCompletionSource<Match>();
 var completionLock = new object();
@@ -72,15 +72,6 @@ void ResolveIfStillPending(Action applyResolution)
 OnReceivedLine += onReceivedLineHandler;
 OnStreamClosed += onStreamClosedHandler;
 
- if (timeout != default)
-{
- var timeoutToken = new CancellationTokenSource(timeout);
- timeoutToken.Token.Register(() =>
-{
- ResolveIfStillPending(() => tcs.SetCanceled());
- });
- }
-
 return tcs.Task;
 }
 
diff --git a/src/Microsoft.AspNetCore.SpaServices.Extensions/Util/TaskTimeoutExtensions.cs b/src/Microsoft.AspNetCore.SpaServices.Extensions/Util/TaskTimeoutExtensions.cs
@@ -0,0 +1,35 @@
+ // Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Threading.Tasks;
+
+namespace Microsoft.AspNetCore.SpaServices.Extensions.Util
+{
+ internal static class TaskTimeoutExtensions
+{
+ public static async Task WithTimeout(this Task task, TimeSpan timeoutDelay, string message)
+{
+ if (task == await Task.WhenAny(task, Task.Delay(timeoutDelay)))
+{
+ task.Wait(); // Allow any errors to propagate
+ }
+ else
+{
+ throw new TimeoutException(message);
+ }
+ }
+
+ public static async Task<T> WithTimeout<T>(this Task<T> task, TimeSpan timeoutDelay, string message)
+{
+ if (task == await Task.WhenAny(task, Task.Delay(timeoutDelay)))
+{
+ return task.Result;
+ }
+ else
+{
+ throw new TimeoutException(message);
+ }
+ }
+ }
+}

Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,6 @@ namespace Microsoft.AspNetCore.SpaServices.AngularCli`
`20`	`20`	`publicclassAngularCliBuilder:ISpaPrerendererBuilder`
`21`	`21`	`{`
`22`	`22`	`privatestaticTimeSpanRegexMatchTimeout=TimeSpan.FromSeconds(5);// This is a development-time only feature, so a very long timeout is fine`
`23`		`-privatestaticTimeSpanBuildTimeout=TimeSpan.FromSeconds(50);// Note that the HTTP request itself by default times out after 60s, so you only get useful error information if this is shorter`
`24`	`23`
`25`	`24`	`privatereadonlystring_npmScriptName;`
`26`	`25`
`@@ -63,8 +62,7 @@ public async Task Build(ISpaBuilder spaBuilder)`
`63`	`62`	`try`
`64`	`63`	`{`
`65`	`64`	`awaitnpmScriptRunner.StdOut.WaitForMatch(`
`66`		`-newRegex("Date",RegexOptions.None,RegexMatchTimeout),`
`67`		`-BuildTimeout);`
	`65`	`+newRegex("Date",RegexOptions.None,RegexMatchTimeout));`
`68`	`66`	`}`
`69`	`67`	`catch(EndOfStreamExceptionex)`
`70`	`68`	`{`
-Original file line number
+Diff line change
 usingSystem.Threading.Tasks;
 usingSystem.Threading;
 usingSystem.Net.Http;
 +usingMicrosoft.AspNetCore.SpaServices.Extensions.Util;
 namespaceMicrosoft.AspNetCore.SpaServices.AngularCli
+{
 vartargetUriTask=angularCliServerInfoTask.ContinueWith(
  task =>newUriBuilder("http","localhost",task.Result.Port).Uri);
 -SpaProxyingExtensions.UseProxyToSpaDevelopmentServer(spaBuilder,targetUriTask);
 +SpaProxyingExtensions.UseProxyToSpaDevelopmentServer(spaBuilder,()=>
 +{
 +// On each request, we create a separate startup task with its own timeout. That way, even if
 +// the first request times out, subsequent requests could still work.
 +returntargetUriTask.WithTimeout(StartupTimeout,
 +$"The Angular CLI process did not start listening for requests "+
 +$"within the timeout period of {StartupTimeout.Seconds} seconds. "+
 +$"Check the log output for error information.");
 +});
+}
 privatestaticasyncTask<AngularCliServerInfo>StartAngularCliServerAsync(
 try
+{
 openBrowserLine=awaitnpmScriptRunner.StdOut.WaitForMatch(
 -newRegex("open your browser on (http\\S+)",RegexOptions.None,RegexMatchTimeout),
 -StartupTimeout);
 +newRegex("open your browser on (http\\S+)",RegexOptions.None,RegexMatchTimeout));
+}
 catch(EndOfStreamExceptionex)
+{
 $"Angular CLI was listening for requests. The error output was: "+
 $"{stdErrReader.ReadAsString()}",ex);
+}
 -catch(TaskCanceledExceptionex)
 -{
 -thrownewInvalidOperationException(
 -$"The Angular CLI process did not start listening for requests "+
 -$"within the timeout period of {StartupTimeout.Seconds} seconds. "+
 -$"Check the log output for error information.",ex);
 -}
+}
 varuri=newUri(openBrowserLine.Groups[1].Value);
-Original file line number
+Diff line change
 usingMicrosoft.AspNetCore.Http.Features;
 usingMicrosoft.AspNetCore.NodeServices;
 usingMicrosoft.AspNetCore.SpaServices;
 +usingMicrosoft.AspNetCore.SpaServices.Extensions.Util;
 usingMicrosoft.AspNetCore.SpaServices.Prerendering;
 usingMicrosoft.Extensions.DependencyInjection;
 usingMicrosoft.Net.Http.Headers;
 /// </summary>
 publicstaticclassSpaPrerenderingExtensions
+{
 +privatestaticTimeSpanBuildTimeout=TimeSpan.FromSeconds(50);// Note that the HTTP request itself by default times out after 60s, so you only get useful error information if this is shorter
++
 /// <summary>
 /// Enables server-side prerendering middleware for a Single Page Application.
 /// </summary>
+}
 // If we're building on demand, wait for that to finish, or raise any build errors
 -if(buildOnDemandTask!=null)
 +if(buildOnDemandTask!=null&&!buildOnDemandTask.IsCompleted)
+{
 -awaitbuildOnDemandTask;
 +// For better debuggability, create a per-request timeout that makes it clear if the
 +// prerendering builder took too long for this request, but without aborting the
 +// underlying build task so that subsequent requests could still work.
 +awaitbuildOnDemandTask.WithTimeout(BuildTimeout,
 +$"The prerendering build process did not complete within the "+
 +$"timeout period of {BuildTimeout.Seconds} seconds. "+
 +$"Check the log output for error information.");
+}
 // It's no good if we try to return a 304. We need to capture the actual
-Original file line number
+Diff line change
 Task.Factory.StartNew(Run);
+}
 -publicTask<Match>WaitForMatch(Regexregex,TimeSpantimeout=default)
 +publicTask<Match>WaitForMatch(Regexregex)
+{
 vartcs=newTaskCompletionSource<Match>();
 varcompletionLock=newobject();
 OnReceivedLine+=onReceivedLineHandler;
 OnStreamClosed+=onStreamClosedHandler;
 -if(timeout!=default)
 -{
 -vartimeoutToken=newCancellationTokenSource(timeout);
 -timeoutToken.Token.Register(()=>
 -{
 -ResolveIfStillPending(()=>tcs.SetCanceled());
 -});
 -}
+-
 returntcs.Task;
+}
-Original file line number
+Diff line change
@@ @@ -0,0 +1,35 @@ @@
 +// Copyright (c) .NET Foundation. All rights reserved.
 +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
++
 +usingSystem;
 +usingSystem.Threading.Tasks;
++
 +namespaceMicrosoft.AspNetCore.SpaServices.Extensions.Util
 +{
 +internalstaticclassTaskTimeoutExtensions
 +{
 +publicstaticasyncTaskWithTimeout(thisTasktask,TimeSpantimeoutDelay,stringmessage)
 +{
 +if(task==awaitTask.WhenAny(task,Task.Delay(timeoutDelay)))
 +{
 +task.Wait();// Allow any errors to propagate
 +}
 +else
 +{
 +thrownewTimeoutException(message);
 +}
 +}
++
 +publicstaticasyncTask<T>WithTimeout<T>(thisTask<T>task,TimeSpantimeoutDelay,stringmessage)
 +{
 +if(task==awaitTask.WhenAny(task,Task.Delay(timeoutDelay)))
 +{
 +returntask.Result;
 +}
 +else
 +{
 +thrownewTimeoutException(message);
 +}
 +}
 +}
 +}