Skip to content

Commit a98c145

Browse files
When a SPA dev server (or prerendering build) takes too long to start up, only fail current request, not future requests. Fixes #1447
1 parent 975d537 commit a98c145

File tree

7 files changed

+75
-40
lines changed

7 files changed

+75
-40
lines changed

src/Microsoft.AspNetCore.SpaServices.Extensions/AngularCli/AngularCliBuilder.cs

+1-3
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ namespace Microsoft.AspNetCore.SpaServices.AngularCli
2020
public class AngularCliBuilder : ISpaPrerendererBuilder
2121
{
2222
private static TimeSpan RegexMatchTimeout = TimeSpan.FromSeconds(5); // This is a development-time only feature, so a very long timeout is fine
23-
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
2423

2524
private readonly string _npmScriptName;
2625

@@ -63,8 +62,7 @@ public async Task Build(ISpaBuilder spaBuilder)
6362
try
6463
{
6564
await npmScriptRunner.StdOut.WaitForMatch(
66-
new Regex("Date", RegexOptions.None, RegexMatchTimeout),
67-
BuildTimeout);
65+
new Regex("Date", RegexOptions.None, RegexMatchTimeout));
6866
}
6967
catch (EndOfStreamException ex)
7068
{

src/Microsoft.AspNetCore.SpaServices.Extensions/AngularCli/AngularCliMiddleware.cs

+11-10
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using System.Threading.Tasks;
1313
using System.Threading;
1414
using System.Net.Http;
15+
using Microsoft.AspNetCore.SpaServices.Extensions.Util;
1516

1617
namespace Microsoft.AspNetCore.SpaServices.AngularCli
1718
{
@@ -49,7 +50,15 @@ public static void Attach(
4950
var targetUriTask = angularCliServerInfoTask.ContinueWith(
5051
task => new UriBuilder("http", "localhost", task.Result.Port).Uri);
5152

52-
SpaProxyingExtensions.UseProxyToSpaDevelopmentServer(spaBuilder, targetUriTask);
53+
SpaProxyingExtensions.UseProxyToSpaDevelopmentServer(spaBuilder, () =>
54+
{
55+
// On each request, we create a separate startup task with its own timeout. That way, even if
56+
// the first request times out, subsequent requests could still work.
57+
return targetUriTask.WithTimeout(StartupTimeout,
58+
$"The Angular CLI process did not start listening for requests " +
59+
$"within the timeout period of {StartupTimeout.Seconds} seconds. " +
60+
$"Check the log output for error information.");
61+
});
5362
}
5463

5564
private static async Task<AngularCliServerInfo> StartAngularCliServerAsync(
@@ -68,8 +77,7 @@ private static async Task<AngularCliServerInfo> StartAngularCliServerAsync(
6877
try
6978
{
7079
openBrowserLine = await npmScriptRunner.StdOut.WaitForMatch(
71-
new Regex("open your browser on (http\\S+)", RegexOptions.None, RegexMatchTimeout),
72-
StartupTimeout);
80+
new Regex("open your browser on (http\\S+)", RegexOptions.None, RegexMatchTimeout));
7381
}
7482
catch (EndOfStreamException ex)
7583
{
@@ -78,13 +86,6 @@ private static async Task<AngularCliServerInfo> StartAngularCliServerAsync(
7886
$"Angular CLI was listening for requests. The error output was: " +
7987
$"{stdErrReader.ReadAsString()}", ex);
8088
}
81-
catch (TaskCanceledException ex)
82-
{
83-
throw new InvalidOperationException(
84-
$"The Angular CLI process did not start listening for requests " +
85-
$"within the timeout period of {StartupTimeout.Seconds} seconds. " +
86-
$"Check the log output for error information.", ex);
87-
}
8889
}
8990

9091
var uri = new Uri(openBrowserLine.Groups[1].Value);

src/Microsoft.AspNetCore.SpaServices.Extensions/Prerendering/SpaPrerenderingExtensions.cs

+11-2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using Microsoft.AspNetCore.Http.Features;
77
using Microsoft.AspNetCore.NodeServices;
88
using Microsoft.AspNetCore.SpaServices;
9+
using Microsoft.AspNetCore.SpaServices.Extensions.Util;
910
using Microsoft.AspNetCore.SpaServices.Prerendering;
1011
using Microsoft.Extensions.DependencyInjection;
1112
using Microsoft.Net.Http.Headers;
@@ -23,6 +24,8 @@ namespace Microsoft.AspNetCore.Builder
2324
/// </summary>
2425
public static class SpaPrerenderingExtensions
2526
{
27+
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
28+
2629
/// <summary>
2730
/// Enables server-side prerendering middleware for a Single Page Application.
2831
/// </summary>
@@ -85,9 +88,15 @@ public static void UseSpaPrerendering(
8588
}
8689

8790
// If we're building on demand, wait for that to finish, or raise any build errors
88-
if (buildOnDemandTask != null)
91+
if (buildOnDemandTask != null && !buildOnDemandTask.IsCompleted)
8992
{
90-
await buildOnDemandTask;
93+
// For better debuggability, create a per-request timeout that makes it clear if the
94+
// prerendering builder took too long for this request, but without aborting the
95+
// underlying build task so that subsequent requests could still work.
96+
await buildOnDemandTask.WithTimeout(BuildTimeout,
97+
$"The prerendering build process did not complete within the " +
98+
$"timeout period of {BuildTimeout.Seconds} seconds. " +
99+
$"Check the log output for error information.");
91100
}
92101

93102
// It's no good if we try to return a 304. We need to capture the actual

src/Microsoft.AspNetCore.SpaServices.Extensions/Proxying/SpaProxyingExtensions.cs

+5-5
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public static void UseProxyToSpaDevelopmentServer(
4545
{
4646
UseProxyToSpaDevelopmentServer(
4747
spaBuilder,
48-
Task.FromResult(baseUri));
48+
() => Task.FromResult(baseUri));
4949
}
5050

5151
/// <summary>
@@ -54,10 +54,10 @@ public static void UseProxyToSpaDevelopmentServer(
5454
/// development. Do not enable this middleware in production applications.
5555
/// </summary>
5656
/// <param name="spaBuilder">The <see cref="ISpaBuilder"/>.</param>
57-
/// <param name="baseUriTask">A <see cref="Task"/> that resolves with the target base URI to which requests should be proxied.</param>
57+
/// <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>
5858
public static void UseProxyToSpaDevelopmentServer(
5959
this ISpaBuilder spaBuilder,
60-
Task<Uri> baseUriTask)
60+
Func<Task<Uri>> baseUriTaskFactory)
6161
{
6262
var applicationBuilder = spaBuilder.ApplicationBuilder;
6363
var applicationStoppingToken = GetStoppingToken(applicationBuilder);
@@ -72,11 +72,11 @@ public static void UseProxyToSpaDevelopmentServer(
7272
var neverTimeOutHttpClient =
7373
SpaProxy.CreateHttpClientForProxy(Timeout.InfiniteTimeSpan);
7474

75-
// Proxy all requests into the Angular CLI server
75+
// Proxy all requests to the SPA development server
7676
applicationBuilder.Use(async (context, next) =>
7777
{
7878
var didProxyRequest = await SpaProxy.PerformProxyRequest(
79-
context, neverTimeOutHttpClient, baseUriTask, applicationStoppingToken,
79+
context, neverTimeOutHttpClient, baseUriTaskFactory(), applicationStoppingToken,
8080
proxy404s: true);
8181
});
8282
}

src/Microsoft.AspNetCore.SpaServices.Extensions/ReactDevelopmentServer/ReactDevelopmentServerMiddleware.cs

+11-10
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using System.Collections.Generic;
1212
using System.Text.RegularExpressions;
1313
using System.Threading.Tasks;
14+
using Microsoft.AspNetCore.SpaServices.Extensions.Util;
1415

1516
namespace Microsoft.AspNetCore.SpaServices.ReactDevelopmentServer
1617
{
@@ -48,7 +49,15 @@ public static void Attach(
4849
var targetUriTask = portTask.ContinueWith(
4950
task => new UriBuilder("http", "localhost", task.Result).Uri);
5051

51-
SpaProxyingExtensions.UseProxyToSpaDevelopmentServer(spaBuilder, targetUriTask);
52+
SpaProxyingExtensions.UseProxyToSpaDevelopmentServer(spaBuilder, () =>
53+
{
54+
// On each request, we create a separate startup task with its own timeout. That way, even if
55+
// the first request times out, subsequent requests could still work.
56+
return targetUriTask.WithTimeout(StartupTimeout,
57+
$"The create-react-app server did not start listening for requests " +
58+
$"within the timeout period of {StartupTimeout.Seconds} seconds. " +
59+
$"Check the log output for error information.");
60+
});
5261
}
5362

5463
private static async Task<int> StartCreateReactAppServerAsync(
@@ -75,8 +84,7 @@ private static async Task<int> StartCreateReactAppServerAsync(
7584
// no compiler warnings. So instead of waiting for that, consider it ready as soon
7685
// as it starts listening for requests.
7786
await npmScriptRunner.StdOut.WaitForMatch(
78-
new Regex("Starting the development server", RegexOptions.None, RegexMatchTimeout),
79-
StartupTimeout);
87+
new Regex("Starting the development server", RegexOptions.None, RegexMatchTimeout));
8088
}
8189
catch (EndOfStreamException ex)
8290
{
@@ -85,13 +93,6 @@ await npmScriptRunner.StdOut.WaitForMatch(
8593
$"create-react-app server was listening for requests. The error output was: " +
8694
$"{stdErrReader.ReadAsString()}", ex);
8795
}
88-
catch (TaskCanceledException ex)
89-
{
90-
throw new InvalidOperationException(
91-
$"The create-react-app server did not start listening for requests " +
92-
$"within the timeout period of {StartupTimeout.Seconds} seconds. " +
93-
$"Check the log output for error information.", ex);
94-
}
9596
}
9697

9798
return portNumber;

src/Microsoft.AspNetCore.SpaServices.Extensions/Util/EventedStreamReader.cs

+1-10
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public EventedStreamReader(StreamReader streamReader)
3434
Task.Factory.StartNew(Run);
3535
}
3636

37-
public Task<Match> WaitForMatch(Regex regex, TimeSpan timeout = default)
37+
public Task<Match> WaitForMatch(Regex regex)
3838
{
3939
var tcs = new TaskCompletionSource<Match>();
4040
var completionLock = new object();
@@ -72,15 +72,6 @@ void ResolveIfStillPending(Action applyResolution)
7272
OnReceivedLine += onReceivedLineHandler;
7373
OnStreamClosed += onStreamClosedHandler;
7474

75-
if (timeout != default)
76-
{
77-
var timeoutToken = new CancellationTokenSource(timeout);
78-
timeoutToken.Token.Register(() =>
79-
{
80-
ResolveIfStillPending(() => tcs.SetCanceled());
81-
});
82-
}
83-
8475
return tcs.Task;
8576
}
8677

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Threading.Tasks;
6+
7+
namespace Microsoft.AspNetCore.SpaServices.Extensions.Util
8+
{
9+
internal static class TaskTimeoutExtensions
10+
{
11+
public static async Task WithTimeout(this Task task, TimeSpan timeoutDelay, string message)
12+
{
13+
if (task == await Task.WhenAny(task, Task.Delay(timeoutDelay)))
14+
{
15+
task.Wait(); // Allow any errors to propagate
16+
}
17+
else
18+
{
19+
throw new TimeoutException(message);
20+
}
21+
}
22+
23+
public static async Task<T> WithTimeout<T>(this Task<T> task, TimeSpan timeoutDelay, string message)
24+
{
25+
if (task == await Task.WhenAny(task, Task.Delay(timeoutDelay)))
26+
{
27+
return task.Result;
28+
}
29+
else
30+
{
31+
throw new TimeoutException(message);
32+
}
33+
}
34+
}
35+
}

0 commit comments

Comments
 (0)