PropertyInfoDescriptor exception handling (#612)

Jint.Tests/Runtime/InteropTests.cs

@@ -1363,7 +1363,6 @@ namespace Jint.Tests.Runtime
             ");
         }
 
-
         [Fact]
         public void ShouldUseExplicitMethod()
         {
@@ -1681,6 +1680,109 @@ namespace Jint.Tests.Runtime
             Assert.Equal(engine.Invoke("throwException2").AsString(), exceptionMessage);
         }
 
+        class MemberExceptionTest
+        {
+            public MemberExceptionTest(bool throwOnCreate)
+            {
+                if (throwOnCreate)
+                    throw new InvalidOperationException();
+            }
+
+            public JsValue ThrowingProperty1
+            {
+                get { throw new InvalidOperationException(); }
+                set { throw new InvalidOperationException(); }
+            }
+
+            public object ThrowingProperty2
+            {
+                get { throw new InvalidOperationException(); }
+                set { throw new InvalidOperationException(); }
+            }
+
+            public void ThrowingFunction()
+            {
+                throw new InvalidOperationException();
+            }
+        }
+
+        [Fact]
+        public void ShouldCatchClrMemberExceptions()
+        {
+            var engine = new Engine(cfg =>
+            {
+                cfg.AllowClr();
+                cfg.CatchClrExceptions();
+            });
+
+            engine.SetValue("assert", new Action<bool>(Assert.True));
+            engine.SetValue("log", new Action<object>(Console.WriteLine));
+            engine.SetValue("create", typeof(MemberExceptionTest));
+            engine.SetValue("instance", new MemberExceptionTest(throwOnCreate: false));
+
+            // Test calling a constructor that throws an exception
+            engine.Execute(@"
+                try
+                {
+                    create(true);
+                    assert(false);
+                }
+                catch (e)
+                {
+                    assert(true);
+                }
+            ");
+
+            // Test calling a member function that throws an exception
+            engine.Execute(@"
+                try
+                {
+                    instance.ThrowingFunction();
+                    assert(false);
+                }
+                catch (e)
+                {
+                    assert(true);
+                }
+            ");
+
+            // Test using a property getter that throws an exception
+            engine.Execute(@"
+                try
+                {
+                    log(o.ThrowingProperty);
+                    assert(false);
+                }
+                catch (e)
+                {
+                    assert(true);
+                }
+            ");
+
+            // Test using a property setter that throws an exception
+            engine.Execute(@"
+                try
+                {
+                    instance.ThrowingProperty1 = 123;
+                    assert(false);
+                }
+                catch (e)
+                {
+                    assert(true);
+                }
+
+                try
+                {
+                    instance.ThrowingProperty2 = 456;
+                    assert(false);
+                }
+                catch (e)
+                {
+                    assert(true);
+                }
+            ");
+        }
+
         [Fact]
         public void ShouldCatchSomeExceptions()
         {

Jint/Engine.cs

@@ -33,7 +33,7 @@ using Jint.Runtime.References;
 
 namespace Jint
 {
-    public sealed class Engine
+    public class Engine
     {
         private static readonly ParserOptions DefaultParserOptions = new ParserOptions
         {

Jint/Runtime/Descriptors/Specialized/PropertyInfoDescriptor.cs

@@ -21,27 +21,47 @@ namespace Jint.Runtime.Descriptors.Specialized
 
         protected internal override JsValue CustomValue
         {
-            get => JsValue.FromObject(_engine, _propertyInfo.GetValue(_item, null));
+            get
+            {
+                object v;
+                try
+                {
+                    v = _propertyInfo.GetValue(_item, null);
+                }
+                catch (TargetInvocationException exception)
+                {
+                    ExceptionHelper.ThrowMeaningfulException(_engine, exception);
+                    throw;
+                }
+
+                return JsValue.FromObject(_engine, v);
+            }
             set
             {
-                var currentValue = value;
                 object obj;
-                if (_propertyInfo.PropertyType == typeof (JsValue))
+                if (_propertyInfo.PropertyType == typeof(JsValue))
                 {
-                    obj = currentValue;
+                    obj = value;
                 }
                 else
                 {
                     // attempt to convert the JsValue to the target type
-                    obj = currentValue.ToObject();
+                    obj = value.ToObject();
                     if (obj != null && obj.GetType() != _propertyInfo.PropertyType)
                     {
                         obj = _engine.ClrTypeConverter.Convert(obj, _propertyInfo.PropertyType, CultureInfo.InvariantCulture);
                     }
                 }
 
-                _propertyInfo.SetValue(_item, obj, null);
+                try
+                {
+                    _propertyInfo.SetValue(_item, obj, null);
+                }
+                catch (TargetInvocationException exception)
+                {
+                    ExceptionHelper.ThrowMeaningfulException(_engine, exception);
+                }
             }
         }
     }
-}
+}

Jint/Runtime/ExceptionHelper.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Reflection;
 using Jint.Native;
 using Jint.Runtime.CallStack;
 using Jint.Runtime.References;
@@ -156,6 +157,17 @@ namespace Jint.Runtime
             throw new ArgumentNullException(paramName);
         }
 
+        public static void ThrowMeaningfulException(Engine engine, TargetInvocationException exception)
+        {
+            var meaningfulException = exception.InnerException ?? exception;
+
+            var handler = engine.Options._ClrExceptionsHandler;
+            if (handler != null && handler(meaningfulException))
+                ThrowError(engine, meaningfulException.Message);
+
+            throw meaningfulException;
+        }
+
         public static void ThrowError(Engine engine, string message)
         {
             throw new JavaScriptException(engine.Error, message);

Jint/Runtime/Interop/DelegateWrapper.cs

@@ -37,7 +37,7 @@ namespace Jint.Runtime.Interop
         public override JsValue Call(JsValue thisObject, JsValue[] jsArguments)
         {
             var parameterInfos = _d.Method.GetParameters();
-            
+
 #if NETFRAMEWORK
             if (parameterInfos.Length > 0 && parameterInfos[0].ParameterType == typeof(System.Runtime.CompilerServices.Closure))
             {
@@ -61,7 +61,7 @@ namespace Jint.Runtime.Interop
             {
                 var parameterType = parameterInfos[i].ParameterType;
 
-                if (parameterType == typeof (JsValue))
+                if (parameterType == typeof(JsValue))
                 {
                     parameters[i] = jsArguments[i];
                 }
@@ -88,7 +88,7 @@ namespace Jint.Runtime.Interop
             }
 
             // assign params to array and converts each objet to expected type
-            if(_delegateContainsParamsArgument)
+            if (_delegateContainsParamsArgument)
             {
                 int paramsArgumentIndex = delegateArgumentsCount - 1;
                 int paramsCount = Math.Max(0, jsArgumentsCount - delegateNonParamsArgumentsCount);
@@ -120,15 +120,8 @@ namespace Jint.Runtime.Interop
             }
             catch (TargetInvocationException exception)
             {
-                var meaningfulException = exception.InnerException ?? exception;
-                var handler = Engine.Options._ClrExceptionsHandler;
-
-                if (handler != null && handler(meaningfulException))
-                {
-                    ExceptionHelper.ThrowError(_engine, meaningfulException.Message);
-                }
-
-                throw meaningfulException;
+                ExceptionHelper.ThrowMeaningfulException(_engine, exception);
+                throw;
             }
         }
     }

Jint/Runtime/Interop/MethodInfoFunctionInstance.cs

@@ -88,15 +88,7 @@ namespace Jint.Runtime.Interop
                 }
                 catch (TargetInvocationException exception)
                 {
-                    var meaningfulException = exception.InnerException ?? exception;
-                    var handler = Engine.Options._ClrExceptionsHandler;
-
-                    if (handler != null && handler(meaningfulException))
-                    {
-                        ExceptionHelper.ThrowError(_engine, meaningfulException.Message);
-                    }
-
-                    throw meaningfulException;
+                    ExceptionHelper.ThrowMeaningfulException(_engine, exception);
                 }
             }
 
@@ -132,6 +124,5 @@ namespace Jint.Runtime.Interop
             newArgumentsCollection[nonParamsArgumentsCount] = jsArray;
             return newArgumentsCollection;
         }
-
     }
 }