Process crash from invalid JS

Reonekot commented 5 years ago

Thanks for filing a bug! To save time, if you're having trouble using the library, please check off the items you have tried. If you are just asking a question, skip right to the bottom.

Please verify these steps before filing an issue, and check them off as you go

[x] The relevant native JavascriptEngineSwitcher library packages are installed (such as JavaScriptEngineSwitcher.V8.Native.win-x64)
[x] The VC++ 2017 runtime is installed
[x] The value of SetUseReact and SetUseBabel is correct in ReactConfig.cs or Startup.cs
[x] I've looked at the sample projects in this repo to verify that my app is configured correctly

I'm using these library versions:

ReactJS.NET: React.AspNet 4.1.1
JavaScriptEngineSwitcher: JavaScriptEngineSwitcher.ChakraCore 3.0.7 JavaScriptEngineSwitcher.ChakraCore.Native.win-x64 3.0.7 JavaScriptEngineSwitcher.Extensions.MsDependencyInjection 3.0.0
react and react-dom: (N/A if using bundled react, or version number) react@16.8.6 + react-dom@16.8.6
webpack: (N/A if using bundled react) v4.29.6
node: (N/A if using bundled react) v8.12.0

Runtime environment:

OS: (Mac, Windows, Linux flavor. Include 32-bit/64-bit and version) Windows 10 64bit
.NET Framework or .NET Core Version: 4.7.2 (running asp.net core under full framework)

Steps to reproduce

Forget to rename .js to .jsx

I have been trying out ReactJS.NET to see if it is a solution for us in moving forward with serverside rendered React. Short story is that I got it working in the end, but on the way I ran into a problem with it crashing the process completely. That's worrisome to put into production, to say it mildly, as our website can't handle traffic without proper warm-up etc.

We are using WebPack - on the way getting it to work in the end, I forgot to rename my test component from .js -> .jsx, so webpack output invalid JS. This crashed the process completely with the following Windows error event:

Application: xxx.exe
Framework Version: v4.0.30319
Description: The process was terminated due to an unhandled exception.
Exception Info: exception code 8007000e, exception address 000001FBE121AC76
Stack:
   at JavaScriptEngineSwitcher.ChakraCore.JsRt.NativeMethods.JsGetAndClearExceptionWithMetadata(JavaScriptEngineSwitcher.ChakraCore.JsRt.JsValue ByRef)
   at JavaScriptEngineSwitcher.ChakraCore.JsRt.NativeMethods.JsGetAndClearExceptionWithMetadata(JavaScriptEngineSwitcher.ChakraCore.JsRt.JsValue ByRef)
   at JavaScriptEngineSwitcher.ChakraCore.JsRt.JsErrorHelpers.ThrowIfError(JavaScriptEngineSwitcher.ChakraCore.JsRt.JsErrorCode)
   at JavaScriptEngineSwitcher.ChakraCore.JsRt.JsContext.RunScript(System.String, JavaScriptEngineSwitcher.ChakraCore.JsRt.JsSourceContext, System.String, JavaScriptEngineSwitcher.ChakraCore.JsRt.JsParseScriptAttributes ByRef)
   at JavaScriptEngineSwitcher.ChakraCore.ChakraCoreJsEngine+<>c__DisplayClass25_0.<InnerEvaluate>b__0()
   at JavaScriptEngineSwitcher.ChakraCore.ScriptDispatcher+<>c__DisplayClass10_0`1[[System.__Canon, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]].<Invoke>b__0()
   at JavaScriptEngineSwitcher.ChakraCore.ScriptDispatcher.StartThread()
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Threading.ThreadHelper.ThreadStart()

I'm not a "frontender", so I'm not sure I can put together a complete example, though I guess it shouldn't be to difficult from the "React.Sample.Webpack.CoreMvc" example project, if needs be.

But basically I need to know if this is expected and "just name of the game" for this, or if is a bug that can be fixed or whether I screwed up in the setup of the component? (I tried setting SetExceptionHandler, but that is never hit.)

In advance, thanks for the help/advice.

Reonekot commented 5 years ago

Just tried V8 and that seems to "behave" better and my ExceptionHandler function is hit instead of a crash:

ReferenceError: props is not defined
   at render (reactComponents.js:24865:104) -> …s.length-1,n="https://reactjs.org/docs/error-decoder.html?invariant="+e,r=0;r<t;r++)n+="&args[]="+en…
   at i (React.Core.Resources.react.generated.min.js:39:11038)
   at he (React.Core.Resources.react.generated.min.js:39:11298)
   at e.render (React.Core.Resources.react.generated.min.js:39:13808)
   at e.read (React.Core.Resources.react.generated.min.js:39:13433)
   at renderToString (React.Core.Resources.react.generated.min.js:39:18190)
   at Script Document [3]:1:16

So I guess this is more likely an issue with JavaScriptEngineSwitcher.ChakraCore, than with ReactJS.NET?

The reason I preferred ChakraCore, was that V8 didn't have netstandard support and we plan to move away from full framework, so adding another full framework dependency seems moving in the wrong direction.

DaniilSokolyuk commented 5 years ago

Hi @Reonekot can you try specify DisableFatalOnOOM setting for ChakraCore .AddChakraCore(settings => { settings.DisableFatalOnOOM = true; });

dustinsoftware commented 5 years ago

cc @Taritsyn possible upstream issue with JS engine switcher?

Taritsyn commented 5 years ago

@dustinsoftware I will deal with this problem later, because now I am working on solving errror #72 “(chakra) recursive evaluation”.

Taritsyn commented 5 years ago

@Reonekot I would like to look at source code of the reactComponents.js file. In principle, a file with components that contains more than 24 865 lines of code could cause a out of memory, so still try to use the DisableFatalOnOOM option.

Taritsyn commented 5 years ago

@Reonekot You can see from the stack trace that the error occurred during error handling:

…
Stack:
   at JavaScriptEngineSwitcher.ChakraCore.JsRt.NativeMethods.JsGetAndClearExceptionWithMetadata(JavaScriptEngineSwitcher.ChakraCore.JsRt.JsValue ByRef)
   at JavaScriptEngineSwitcher.ChakraCore.JsRt.NativeMethods.JsGetAndClearExceptionWithMetadata(JavaScriptEngineSwitcher.ChakraCore.JsRt.JsValue ByRef)
…

Moreover, the error handler is triggered 2 times. Most likely, this is some error in the original ChakraCore library, because such calls are impossible in the JavaScriptEngineSwitcher.ChakraCore module.

To send information about this error to developers of original library, I must first reproduce it.

Reonekot commented 5 years ago

Sorry for the long absence - been on a long Easter vacation and totally forgot I had this open. Thanks for the feedback and looking into it!

I tried setting DisableFatalOnOOM = true and this "fixes" my original issue, so my exception handler on IReactSiteConfiguration is now hit, which matches what I see with V8. This is much more what I expected and need before putting it in production, since killing the process due to an error in a "view" is a no-go for us. (Hopefully our frontend build tools, or test systems, will catch such an error, but still...)

And it seems you were spot on with the line count of reactComponents.js - this file for this unoptimized build is 24.877 lines.

Excuse my ignorance here, but how do I handle this? My reactComponent.js looks like this before webpack when running "unoptimized":

import React from 'react';
import ReactDOM from 'react-dom';
import ReactTestComponent from './reactmodules/reactTestComponent';

global.React = React;
global.ReactDOM = ReactDOM;
global.ReactTestComponent = ReactTestComponent;

reactTestComponent is 10 lines of code all inclusive, as it's just a very basic "hello world" component. This then becomes too big for the JS engine when unoptimized - is the only option here to always do some form of minifacition, which I'd rather not do in dev environment?

Again, thanks for looking into this issue and the help!

Taritsyn commented 5 years ago

I tried setting DisableFatalOnOOM = true and this "fixes" my original issue, so my exception handler on IReactSiteConfiguration is now hit, which matches what I see with V8.

@Reonekot Looks like the problem is solved. In the next version I will make true as default value for the DisableFatalOnOOM option.

Reonekot commented 5 years ago

Yes, DisableFatalOnOOM solved the issue, so I'll close the issue.

reactjs / React.NET