If errors occur while loading a JS file, don't throw an exception until GetEngine is called.

This ensures that errors are not thrown on a background thread if the engines are being initialised on a background thread (ie. when the engines are being recycled due to files being modified).

References #189

Author: Daniel15

src/React.AspNet/project.json

@@ -15,7 +15,7 @@
         }
     },
     "dependencies": {
-        "JsPool": "0.3.0",
+        "JsPool": "0.3.1",
         "Microsoft.Framework.DependencyInjection": "1.0.0-beta8",
         "Microsoft.AspNet.Mvc.Core": "6.0.0-beta8",
         "Microsoft.AspNet.StaticFiles": "1.0.0-beta8",

src/React.Core/JavaScriptEngineFactory.cs

@@ -42,6 +42,10 @@ protected readonly ConcurrentDictionary<int, IJsEngine> _engines
 		/// Whether this class has been disposed.
 		/// </summary>
 		protected bool _disposed;
+		/// <summary>
+		/// The exception that was thrown during the most recent recycle of the pool.
+		/// </summary>
+		protected Exception _scriptLoadException;
 
 		/// <summary>
 		/// Initializes a new instance of the <see cref="JavaScriptEngineFactory"/> class.
@@ -86,7 +90,11 @@ protected virtual IJsPool CreatePool()
 				poolConfig.StartEngines = _config.StartEngines.Value;
 			}
 
-			return new JsPool(poolConfig);
+			var pool = new JsPool(poolConfig);
+			// Reset the recycle exception on recycle. If there *are* errors loading the scripts
+			// during recycle, the errors will be caught in the initializer.
+			pool.Recycled += (sender, args) => _scriptLoadException = null;
+			return pool;
 		}
 
 		/// <summary>
@@ -105,7 +113,7 @@ protected virtual void InitialiseEngine(IJsEngine engine)
 			LoadUserScripts(engine);
 			if (!_config.LoadReact)
 			{
-				// We expect to user to have loaded their own versino of React in the scripts that
+				// We expect to user to have loaded their own version of React in the scripts that
 				// were loaded above, let's ensure that's the case. 
 				EnsureReactLoaded(engine);
 			}
@@ -127,7 +135,11 @@ private void LoadUserScripts(IJsEngine engine)
 				}
 				catch (JsRuntimeException ex)
 				{
-					throw new ReactScriptLoadException(string.Format(
+					// We can't simply rethrow the exception here, as it's possible this is running
+					// on a background thread (ie. as a response to a file changing). If we did
+					// throw the exception here, it would terminate the entire process. Instead,
+					// save the exception, and then just rethrow it later when getting the engine.
+					_scriptLoadException = new ReactScriptLoadException(string.Format(
 						"Error while loading \"{0}\": {1}\r\nLine: {2}\r\nColumn: {3}",
 						file,
 						ex.Message,
@@ -162,11 +174,12 @@ private static void EnsureReactLoaded(IJsEngine engine)
 		/// <returns>The JavaScript engine</returns>
 		public virtual IJsEngine GetEngineForCurrentThread()
 		{
-			EnsureNotDisposed();
+			EnsureValidState();
 			return _engines.GetOrAdd(Thread.CurrentThread.ManagedThreadId, id =>
 			{
 				var engine = _factory();
 				InitialiseEngine(engine);
+				EnsureValidState();
 				return engine;
 			});
 		}
@@ -192,7 +205,7 @@ public virtual void DisposeEngineForCurrentThread()
 		/// <returns>The JavaScript engine</returns>
 		public virtual IJsEngine GetEngine()
 		{
-			EnsureNotDisposed();
+			EnsureValidState();
 			return _pool.GetEngine();
 		}
 
@@ -298,14 +311,20 @@ public virtual void Dispose()
 		}
 
 		/// <summary>
-		/// Ensures that this object has not been disposed.
+		/// Ensures that this object has not been disposed, and that no error was thrown while
+		/// loading the scripts.
 		/// </summary>
-		public void EnsureNotDisposed()
+		public void EnsureValidState()
 		{
 			if (_disposed)
 			{
 				throw new ObjectDisposedException(GetType().Name);
 			}
+			if (_scriptLoadException != null)
+			{
+				// This means an exception occurred while loading the script (eg. syntax error in the file)
+				throw _scriptLoadException;
+			}
 		}
 
 		/// <summary>

src/React.Core/React.Core.csproj

@@ -61,8 +61,8 @@
       <HintPath>..\packages\JavaScriptEngineSwitcher.V8.1.2.4\lib\net40\JavaScriptEngineSwitcher.V8.dll</HintPath>
     </Reference>
     <Reference Include="JSPool, Version=0.3.0.0, Culture=neutral, PublicKeyToken=2fc7775f73072640, processorArchitecture=MSIL">
-      <SpecificVersion>False</SpecificVersion>
-      <HintPath>..\packages\JSPool.0.3.0\lib\net40-Client\JSPool.dll</HintPath>
+      <HintPath>..\packages\JSPool.0.3.1\lib\net40-Client\JSPool.dll</HintPath>
+      <Private>True</Private>
     </Reference>
     <Reference Include="MsieJavaScriptEngine, Version=1.5.1.0, Culture=neutral, PublicKeyToken=a3a2846a37ac0d3e, processorArchitecture=MSIL">
       <SpecificVersion>False</SpecificVersion>

src/React.Core/packages.config

@@ -1,9 +1,9 @@
 <?xml version="1.0" encoding="utf-8"?>
 <packages>
-  <package id="JavaScriptEngineSwitcher.Core" version="1.2.4" targetFramework="net40" />
-  <package id="JavaScriptEngineSwitcher.Msie" version="1.2.4" targetFramework="net40" />
-  <package id="JavaScriptEngineSwitcher.V8" version="1.2.4" targetFramework="net40" />
-  <package id="JSPool" version="0.3.0" targetFramework="net40" />
-  <package id="MsieJavaScriptEngine" version="1.5.1" targetFramework="net40" />
-  <package id="Newtonsoft.Json" version="6.0.8" targetFramework="net40" />
+  <package id="JavaScriptEngineSwitcher.Core" version="1.2.4" targetFramework="net4" />
+  <package id="JavaScriptEngineSwitcher.Msie" version="1.2.4" targetFramework="net4" />
+  <package id="JavaScriptEngineSwitcher.V8" version="1.2.4" targetFramework="net4" />
+  <package id="JSPool" version="0.3.1" targetFramework="net4" />
+  <package id="MsieJavaScriptEngine" version="1.5.1" targetFramework="net4" />
+  <package id="Newtonsoft.Json" version="6.0.8" targetFramework="net4" />
 </packages>

src/React.Tests/Core/JavaScriptEngineFactoryTest.cs

@@ -156,5 +156,32 @@ public void ShouldThrowIfReactVersionNotLoaded()
 				factory.GetEngineForCurrentThread();
 			});
 		}
+
+		[Test]
+		public void ShouldCatchErrorsWhileLoadingScripts()
+		{
+			var config = new Mock<IReactSiteConfiguration>();
+			config.Setup(x => x.ScriptsWithoutTransform).Returns(new List<string> {"foo.js"});
+			config.Setup(x => x.LoadReact).Returns(true);
+			var fileSystem = new Mock<IFileSystem>();
+			fileSystem.Setup(x => x.ReadAsString("foo.js")).Returns("FAIL PLZ");
+
+			var jsEngine = new Mock<IJsEngine>();
+			jsEngine.Setup(x => x.Evaluate<int>("1 + 1")).Returns(2);
+			jsEngine.Setup(x => x.Execute("FAIL PLZ")).Throws(new JsRuntimeException("Fail")
+			{
+				LineNumber = 42,
+				ColumnNumber = 911,
+			});
+			var registration = new JavaScriptEngineFactory.Registration
+			{
+				Factory = () => jsEngine.Object,
+				Priority = 1
+			};
+			var factory = new JavaScriptEngineFactory(new[] { registration }, config.Object, fileSystem.Object);
+
+			var ex = Assert.Throws<ReactScriptLoadException>(() => factory.GetEngineForCurrentThread());
+			Assert.AreEqual("Error while loading \"foo.js\": Fail\r\nLine: 42\r\nColumn: 911", ex.Message);
+		}
 	}
 }

src/wrap/React.Core/project.json

@@ -11,10 +11,10 @@
         "JavaScriptEngineSwitcher.Core": "1.2.4",
         "JavaScriptEngineSwitcher.Msie": "1.2.4",
         "JavaScriptEngineSwitcher.V8": "1.2.4",
-        "JSPool": "0.3.0",
+        "JSPool": "0.3.1",
         "MsieJavaScriptEngine": "1.5.1",
         "Newtonsoft.Json": "6.0.8",
-        "VroomJs": "2.0.0-*"
+        "VroomJs": "2.0.2-*"
       }
     }
   }