Fix exception handling during render in .NET Core (#793)

If an exception is thrown outside of the render call in
Razor views (async invocation), the app could crash. This does not appear
to be an issue in ASP.NET.

Unfortunately this means extra string allocations to hold the render
result, but the rest of the StringBuilder logic used internally is not
a problem.

Author: dustinsoftware

src/React.AspNet/ActionHtmlString.cs

@@ -7,12 +7,14 @@
 
 using System;
 using System.IO;
+using System.Text;
 
 #if LEGACYASPNET
 using System.Web;
 using IHtmlHelper = System.Web.Mvc.HtmlHelper;
 #else
 using Microsoft.AspNetCore.Mvc.Rendering;
+using Microsoft.AspNetCore.Html;
 using IHtmlString = Microsoft.AspNetCore.Html.IHtmlContent;
 #endif
 
@@ -27,6 +29,9 @@ namespace React.AspNet
 	/// </summary>
 	public static class HtmlHelperExtensions
 	{
+		[ThreadStatic]
+		private static StringWriter _sharedStringWriter;
+
 		/// <summary>
 		/// Gets the React environment
 		/// </summary>
@@ -66,28 +71,25 @@ public static IHtmlString React<T>(
 			IRenderFunctions renderFunctions = null
 		)
 		{
-			return new ActionHtmlString(writer =>
+			try
 			{
-				try
+				var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly, serverOnly);
+				if (!string.IsNullOrEmpty(htmlTag))
 				{
-					var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly, serverOnly);
-					if (!string.IsNullOrEmpty(htmlTag))
-					{
-						reactComponent.ContainerTag = htmlTag;
-					}
-
-					if (!string.IsNullOrEmpty(containerClass))
-					{
-						reactComponent.ContainerClass = containerClass;
-					}
-
-					reactComponent.RenderHtml(writer, clientOnly, serverOnly, exceptionHandler, renderFunctions);
+					reactComponent.ContainerTag = htmlTag;
 				}
-				finally
+
+				if (!string.IsNullOrEmpty(containerClass))
 				{
-					Environment.ReturnEngineToPool();
+					reactComponent.ContainerClass = containerClass;
 				}
-			});
+
+				return RenderToString(writer => reactComponent.RenderHtml(writer, clientOnly, serverOnly, exceptionHandler, renderFunctions));
+			}
+			finally
+			{
+				Environment.ReturnEngineToPool();
+			}
 		}
 
 		/// <summary>
@@ -116,31 +118,32 @@ public static IHtmlString ReactWithInit<T>(
 			Action<Exception, string, string> exceptionHandler = null
 		)
 		{
-			return new ActionHtmlString(writer =>
+			try
 			{
-				try
+				var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly);
+				if (!string.IsNullOrEmpty(htmlTag))
 				{
-					var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly);
-					if (!string.IsNullOrEmpty(htmlTag))
-					{
-						reactComponent.ContainerTag = htmlTag;
-					}
+					reactComponent.ContainerTag = htmlTag;
+				}
 
-					if (!string.IsNullOrEmpty(containerClass))
-					{
-						reactComponent.ContainerClass = containerClass;
-					}
+				if (!string.IsNullOrEmpty(containerClass))
+				{
+					reactComponent.ContainerClass = containerClass;
+				}
 
+				return RenderToString(writer =>
+				{
 					reactComponent.RenderHtml(writer, clientOnly, exceptionHandler: exceptionHandler);
 					writer.WriteLine();
 					WriteScriptTag(writer, bodyWriter => reactComponent.RenderJavaScript(bodyWriter));
-				}
-				finally
-				{
-					Environment.ReturnEngineToPool();
-				}
-			});
-		}
+				});
+					
+			}
+			finally
+			{
+				Environment.ReturnEngineToPool();
+			}
+	}
 
 		/// <summary>
 		/// Renders the JavaScript required to initialise all components client-side. This will
@@ -149,17 +152,33 @@ public static IHtmlString ReactWithInit<T>(
 		/// <returns>JavaScript for all components</returns>
 		public static IHtmlString ReactInitJavaScript(this IHtmlHelper htmlHelper, bool clientOnly = false)
 		{
-			return new ActionHtmlString(writer =>
+			try
 			{
-				try
+				return RenderToString(writer =>
 				{
 					WriteScriptTag(writer, bodyWriter => Environment.GetInitJavaScript(bodyWriter, clientOnly));
-				}
-				finally
-				{
-					Environment.ReturnEngineToPool();
-				}
-			});
+				});
+			}
+			finally
+			{
+				Environment.ReturnEngineToPool();
+			}
+		}
+
+		private static IHtmlString RenderToString(Action<StringWriter> withWriter)
+		{
+			var stringWriter = _sharedStringWriter;
+			if (stringWriter != null)
+			{
+				stringWriter.GetStringBuilder().Clear();
+			}
+			else
+			{
+				_sharedStringWriter = stringWriter = new StringWriter(new StringBuilder(128));
+			}
+
+			withWriter(stringWriter);
+			return new HtmlString(stringWriter.ToString());
 		}
 
 		private static void WriteScriptTag(TextWriter writer, Action<TextWriter> bodyWriter)

src/React.AspNet/HtmlHelperExtensions.cs

@@ -21,7 +21,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <Compile Include="..\SharedAssemblyInfo.cs;..\React.AspNet\HtmlHelperExtensions.cs;..\React.AspNet\ActionHtmlString.cs" />
+    <Compile Include="..\SharedAssemblyInfo.cs;..\React.AspNet\HtmlHelperExtensions.cs" />
     <Compile Include="..\SharedAssemblyVersionInfo.cs" />
     <Content Include="Content\**\*">
       <Pack>true</Pack>

src/React.Web.Mvc4/React.Web.Mvc4.csproj

@@ -407,7 +407,37 @@ public void ChainedRenderFunctionsCalled()
 			Assert.Equal("postrender-result", firstInstance.PostRenderResult);
 			Assert.Equal("postrender-result", secondInstance.PostRenderResult);
 		}
-		
+
+		[Fact]
+		public void RenderFunctionsCalledServerOnly()
+		{
+			var environment = new Mock<IReactEnvironment>();
+			environment.Setup(x => x.Execute<bool>("typeof Foo !== 'undefined'")).Returns(true);
+			environment.Setup(x => x.Execute<string>(@"outerWrap(ReactDOMServer.renderToStaticMarkup(wrap(React.createElement(Foo, {""hello"":""World""}))))"))
+				.Returns("[HTML]");
+
+			environment.Setup(x => x.Execute<string>(@"prerender();"))
+				.Returns("prerender-result");
+
+			environment.Setup(x => x.Execute<string>(@"postrender();"))
+				.Returns("postrender-result");
+
+			var config = CreateDefaultConfigMock();
+			config.Setup(x => x.UseServerSideRendering).Returns(true);
+			var reactIdGenerator = new Mock<IReactIdGenerator>();
+
+			var component = new ReactComponent(environment.Object, config.Object, reactIdGenerator.Object, "Foo", "container")
+			{
+				Props = new { hello = "World" }
+			};
+			var renderFunctions = new TestRenderFunctions();
+			var result = component.RenderHtml(renderFunctions: renderFunctions, renderServerOnly: true);
+
+			Assert.Equal(@"[HTML]", result);
+			Assert.Equal(@"prerender-result", renderFunctions.PreRenderResult);
+			Assert.Equal(@"postrender-result", renderFunctions.PostRenderResult);
+		}
+
 		private static Mock<IReactSiteConfiguration> CreateDefaultConfigMock()
 		{
 			var configMock = new Mock<IReactSiteConfiguration>();

tests/React.Tests/Core/ReactComponentTest.cs

@@ -206,6 +206,50 @@ public void ReactWithServerOnlyTrueShouldCallRenderHtmlWithTrue()
 
 			component.Verify(x => x.RenderHtml(It.IsAny<TextWriter>(), It.Is<bool>(y => y == false), It.Is<bool>(z => z == true), null, null), Times.Once);
 		}
+
+		[Fact]
+		public void RenderFunctionsCalledNonLazily()
+		{
+			var component = new Mock<IReactComponent>();
+			var fakeRenderFunctions = new Mock<IRenderFunctions>();
+			fakeRenderFunctions.Setup(x => x.PreRender(It.IsAny<Func<string, string>>())).Verifiable();
+			fakeRenderFunctions.Setup(x => x.PostRender(It.IsAny<Func<string, string>>())).Verifiable();
+			fakeRenderFunctions.Setup(x => x.TransformRenderedHtml(It.IsAny<string>())).Returns("HTML");
+
+			component.Setup(x => x.RenderHtml(It.IsAny<TextWriter>(), It.IsAny<bool>(), It.IsAny<bool>(), It.IsAny<Action<Exception, string, string>>(), It.IsAny<IRenderFunctions>()))
+				.Callback((TextWriter writer, bool renderContainerOnly, bool renderServerOnly, Action<Exception, string, string> exceptionHandler, IRenderFunctions renderFunctions) => 
+				{
+					renderFunctions.PreRender(_ => "one");
+					writer.Write(renderFunctions.TransformRenderedHtml("HTML"));
+					renderFunctions.PostRender(_ => "two");
+				}).Verifiable();
+
+			var environment = ConfigureMockEnvironment();
+			environment.Setup(x => x.CreateComponent(
+				"ComponentName",
+				new { },
+				null,
+				false,
+				true
+			)).Returns(component.Object);
+
+			var result = HtmlHelperExtensions.React(
+				htmlHelper: null,
+				componentName: "ComponentName",
+				props: new { },
+				htmlTag: "span",
+				clientOnly: false,
+				serverOnly: true,
+				renderFunctions: fakeRenderFunctions.Object
+			);
+
+			// JS calls must happen right away so thrown exceptions do not crash the app.
+			component.Verify(x => x.RenderHtml(It.IsAny<TextWriter>(), It.Is<bool>(y => y == false), It.Is<bool>(z => z == true), It.IsAny<Action<Exception, string, string>>(), It.IsAny<IRenderFunctions>()), Times.Once);
+			fakeRenderFunctions.Verify(x => x.PreRender(It.IsAny<Func<string, string>>()), Times.Once);
+			fakeRenderFunctions.Verify(x => x.PostRender(It.IsAny<Func<string, string>>()), Times.Once);
+
+			Assert.Equal("HTML", result.ToHtmlString());
+		}
 	}
 }
 #endif