Better error from scripts. (#408)

- Will track the identifier that cause an undefined / null access
- Will report the call stack of the error to the user

Jint.Tests/Jint.Tests.csproj

@@ -1,5 +1,4 @@
 <Project Sdk="Microsoft.NET.Sdk">
-
   <PropertyGroup>
     <TargetFrameworks>netcoreapp1.0;net451</TargetFrameworks>
     <AssemblyName>Jint.Tests</AssemblyName>
@@ -15,25 +14,20 @@
     <GenerateAssemblyVersionAttribute>false</GenerateAssemblyVersionAttribute>
     <GenerateAssemblyFileVersionAttribute>false</GenerateAssemblyFileVersionAttribute>
   </PropertyGroup>
-
   <ItemGroup>
     <EmbeddedResource Include="Runtime\Scripts\*.*;Parser\Scripts\*.*" Exclude="bin\**;obj\**;**\*.xproj;packages\**;@(EmbeddedResource)" />
   </ItemGroup>
-
   <ItemGroup>
     <ProjectReference Include="..\Jint\Jint.csproj" />
   </ItemGroup>
-
   <ItemGroup>
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.0.0" />
     <PackageReference Include="xunit" Version="2.2.0" />
     <PackageReference Include="xunit.runner.visualstudio" Version="2.2.0" />
     <PackageReference Include="xunit.analyzers" Version="0.3.0" />
   </ItemGroup>
-
   <ItemGroup Condition=" '$(TargetFramework)' == 'net451' ">
     <Reference Include="System" />
     <Reference Include="Microsoft.CSharp" />
   </ItemGroup>
-
-</Project>
+</Project>

Jint.Tests/Runtime/ErrorTests.cs

@@ -0,0 +1,68 @@
+using System;
+using Jint.Parser;
+using Jint.Runtime;
+using Xunit;
+
+namespace Jint.Tests.Runtime
+{
+    public class ErrorTests
+    {
+        [Fact]
+        public void CanReturnCorrectErrorMessageAndLocation1()
+        {
+            var script = @"
+var a = {};
+
+var b = a.user.name;
+";
+
+            var engine = new Engine();
+            var e = Assert.Throws<JavaScriptException>(() => engine.Execute(script));
+            Assert.Equal("user is undefined", e.Message);
+            Assert.Equal(4, e.Location.Start.Line);
+            Assert.Equal(8, e.Location.Start.Column);
+        }
+
+        [Fact]
+        public void CanReturnCorrectErrorMessageAndLocation2()
+        {
+            var script = @"
+ test();
+";
+
+            var engine = new Engine();
+            var e = Assert.Throws<JavaScriptException>(() => engine.Execute(script));
+            Assert.Equal("test is not defined", e.Message);
+            Assert.Equal(2, e.Location.Start.Line);
+            Assert.Equal(1, e.Location.Start.Column);
+        }
+
+        [Fact]
+        public void CanProduceCorrectStackTrace()
+        {
+            var engine = new Engine(options => options.LimitRecursion(1000));
+
+            engine.Execute(@"var a = function(v) {
+	return v.xxx.yyy;
+}
+
+var b = function(v) {
+	return a(v);
+}", new ParserOptions
+            {
+                Source = "custom.js"
+            });
+
+            var e = Assert.Throws<JavaScriptException>(() => engine.Execute("var x = b(7);", new ParserOptions { Source = "main.js"}));
+            Assert.Equal("xxx is undefined", e.Message);
+            Assert.Equal(2, e.Location.Start.Line);
+            Assert.Equal(8, e.Location.Start.Column);
+            Assert.Equal("custom.js", e.Location.Source);
+
+            var stack = e.CallStack;
+            Assert.Equal(@" at a(v) @ custom.js 8:6
+ at b(7) @ main.js 8:1
+".Replace("\r\n", "\n"), stack.Replace("\r\n", "\n"));
+        }
+    }
+}

Jint/Engine.cs

@@ -317,9 +317,7 @@ public Engine Execute(Program program)
                 if (result.Type == Completion.Throw)
                 {
                     throw new JavaScriptException(result.GetValueOrDefault())
-                    {
-                        Location = result.Location
-                    };
+                        .SetCallstack(this, result.Location);
                 }
 
                 _completionValue = result.GetValueOrDefault();

Jint/Native/Function/EvalFunctionInstance.cs

@@ -58,7 +58,8 @@ public JsValue Call(JsValue thisObject, JsValue[] arguments, bool directCall)
 
                             if (result.Type == Completion.Throw)
                             {
-                                throw new JavaScriptException(result.GetValueOrDefault());
+                                throw new JavaScriptException(result.GetValueOrDefault())
+                                    .SetCallstack(_engine, result.Location);
                             }
                             else
                             {

Jint/Native/Function/ScriptFunctionInstance.cs

@@ -94,8 +94,8 @@ public override JsValue Call(JsValue thisArg, JsValue[] arguments)
 
                     if (result.Type == Completion.Throw)
                     {
-                        JavaScriptException ex = new JavaScriptException(result.GetValueOrDefault());
-                        ex.Location = result.Location;
+                        JavaScriptException ex = new JavaScriptException(result.GetValueOrDefault())
+                            .SetCallstack(Engine, result.Location);
                         throw ex;
                     }
 

Jint/Runtime/CallStack/JintCallStack.cs

@@ -1,9 +1,11 @@
-namespace Jint.Runtime.CallStack
+using System.Collections;
+
+namespace Jint.Runtime.CallStack
 {
     using System.Collections.Generic;
     using System.Linq;
 
-    public class JintCallStack
+    public class JintCallStack : IEnumerable<CallStackElement>
     {
         private Stack<CallStackElement> _stack = new Stack<CallStackElement>();
 
@@ -45,9 +47,19 @@ public void Clear()
             _statistics.Clear();
         }
 
+        public IEnumerator<CallStackElement> GetEnumerator()
+        {
+            return _stack.GetEnumerator();
+        }
+
         public override string ToString()
         {
             return string.Join("->", _stack.Select(cse => cse.ToString()).Reverse());
         }
+
+        IEnumerator IEnumerable.GetEnumerator()
+        {
+            return GetEnumerator();
+        }
     }
 }

Jint/Runtime/ExpressionIntepreter.cs

@@ -757,7 +757,7 @@ public Reference EvaluateMemberExpression(MemberExpression memberExpression)
 
             var propertyNameReference = EvaluateExpression(expression);
             var propertyNameValue = _engine.GetValue(propertyNameReference);
-            TypeConverter.CheckObjectCoercible(_engine, baseValue);
+            TypeConverter.CheckObjectCoercible(_engine, baseValue, memberExpression, baseReference);
             var propertyNameString = TypeConverter.ToString(propertyNameValue);
 
             return new Reference(baseValue, propertyNameString, StrictModeScope.IsStrictModeCode);

Jint/Runtime/JavaScriptException.cs

@@ -1,12 +1,20 @@
 using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
 using Jint.Native;
 using Jint.Native.Error;
+using Jint.Parser;
+using Jint.Parser.Ast;
+using Jint.Runtime.CallStack;
+using Jint.Runtime.Descriptors;
 
 namespace Jint.Runtime
 {
     public class JavaScriptException : Exception
     {
         private readonly JsValue _errorObject;
+        private string _callStack;
 
         public JavaScriptException(ErrorConstructor errorConstructor) : base("")
         {
@@ -25,6 +33,40 @@ public JavaScriptException(JsValue error)
             _errorObject = error;
         }
 
+        public JavaScriptException SetCallstack(Engine engine, Location location = null)
+        {
+            Location = location;
+            var sb = new StringBuilder();
+            foreach (var cse in engine.CallStack)
+            {
+                sb.Append(" at ")
+                    .Append(cse)
+                    .Append("(");
+
+                for (var index = 0; index < cse.CallExpression.Arguments.Count; index++)
+                {
+                    if (index != 0)
+                        sb.Append(", ");
+                    var arg = cse.CallExpression.Arguments[index];
+                    if (arg is IPropertyKeyExpression pke)
+                        sb.Append(pke.GetKey());
+                    else
+                        sb.Append(arg);
+                }
+
+
+                sb.Append(") @ ")
+                    .Append(cse.CallExpression.Location.Source)
+                    .Append(" ")
+                    .Append(cse.CallExpression.Location.Start.Column)
+                    .Append(":")
+                    .Append(cse.CallExpression.Location.Start.Line)
+                    .AppendLine();
+            }
+            CallStack = sb.ToString();
+            return this;
+        }
+
         private static string GetErrorMessage(JsValue error) 
         {
             if (error.IsObject())
@@ -33,8 +75,10 @@ private static string GetErrorMessage(JsValue error)
                 var message = oi.Get("message").AsString();
                 return message;
             }
-            else
-                return string.Empty;            
+            if (error.IsString())
+                return error.AsString();
+
+            return error.ToString();
         }
 
         public JsValue Error { get { return _errorObject; } }
@@ -44,6 +88,32 @@ public override string ToString()
             return _errorObject.ToString();
         }
 
+        public string CallStack
+        {
+            get
+            {
+                if (_callStack != null)
+                    return _callStack;
+                if (_errorObject == null)
+                    return null;
+                if (_errorObject.IsObject() == false)
+                    return null;
+                var callstack = _errorObject.AsObject().Get("callstack");
+                if (callstack == JsValue.Undefined)
+                    return null;
+                return callstack.AsString();
+            }
+            set
+            {
+                _callStack = value;
+                if (value != null && _errorObject.IsObject())
+                {
+                    _errorObject.AsObject()
+                        .FastAddProperty("callstack", new JsValue(value), false, false, false);
+                }
+            }
+        }
+
         public Jint.Parser.Location Location { get; set; }
 
         public int LineNumber { get { return null == Location ? 0 : Location.Start.Line; } }

Jint/Runtime/TypeConverter.cs

@@ -7,6 +7,8 @@
 using Jint.Native.Number;
 using Jint.Native.Object;
 using Jint.Native.String;
+using Jint.Parser.Ast;
+using Jint.Runtime.References;
 
 namespace Jint.Runtime
 {
@@ -339,6 +341,21 @@ public static Types GetPrimitiveType(JsValue value)
             return value.Type;
         }
 
+        public static void CheckObjectCoercible(Engine engine, JsValue o, MemberExpression expression,
+            object baseReference)
+        {
+            if (o != Undefined.Instance && o != Null.Instance)
+                return;
+
+            var message = string.Empty;
+            var reference = baseReference as Reference;
+            if (reference != null)
+                message = $"{reference.GetReferencedName()} is {o}";
+
+            throw new JavaScriptException(engine.TypeError, message)
+                .SetCallstack(engine, expression.Location);
+        }
+
         public static void CheckObjectCoercible(Engine engine, JsValue o)
         {
             if (o == Undefined.Instance || o == Null.Instance)