Accessing dynamic objects returns incorrect type

[JakobTinhofer]

Version used

4.0.0

Describe the bug

When accessing members of members of dynamic objects from JS an exception is thrown:

System.InvalidCastException: 'Unable to cast object of type 'MemberType' to type 'System.Dynamic.DynamicObject'.'
     at Jint.Runtime.Interop.Reflection.DynamicObjectAccessor.DoGetValue(Object target, String memberName) in C:\...\Jint\Runtime\Interop\Reflection\DynamicObjectAccessor.cs:line 24 

Callstack:
 Jint.dll!Jint.Runtime.Interop.Reflection.DynamicObjectAccessor.DoGetValue(object target, string memberName) Line 24    C#
 Jint.dll!Jint.Runtime.Interop.Reflection.ReflectionAccessor.GetValue(Jint.Engine engine, object target, string memberName) Line 56 C#
 Jint.dll!Jint.Runtime.Descriptors.Specialized.ReflectionDescriptor.DoGet(Jint.Native.JsValue thisObj) Line 67  C#
 Jint.dll!Jint.Runtime.Descriptors.Specialized.ReflectionDescriptor.CustomValue.get() Line 61   C#
 Jint.dll!Jint.Native.Object.ObjectInstance.UnwrapJsValue(Jint.Runtime.Descriptors.PropertyDescriptor desc, Jint.Native.JsValue thisObject) Line 385    C#
 Jint.dll!Jint.Runtime.Interop.ObjectWrapper.Get(Jint.Native.JsValue property, Jint.Native.JsValue receiver) Line 210   C#
 Jint.dll!Jint.Engine.GetValue(Jint.Runtime.Reference reference, bool returnReferenceToPool) Line 599   C#
 Jint.dll!Jint.Runtime.Interpreter.Expressions.JintExpression.GetValue(Jint.Runtime.Interpreter.EvaluationContext context) Line 32  C#
 Jint.dll!Jint.Runtime.Interpreter.Statements.JintExpressionStatement.ExecuteInternal(Jint.Runtime.Interpreter.EvaluationContext context) Line 24   C#
 Jint.dll!Jint.Runtime.Interpreter.Statements.JintStatement.Execute(Jint.Runtime.Interpreter.EvaluationContext context) Line 42 C#

To Reproduce

[Fact]
public void ShouldAccessCustomDynamicObjectProperties()
{
    DynamicType t = new DynamicType();
    t["MemberKey"] = new MemberType() { Field = 4 };
    Engine e = new Engine().SetValue("dynamicObj", t);
    Assert.Equal(4, ((dynamic) t).MemberKey.Field); // Succeeds
    Assert.Equal(4, e.Evaluate("dynamicObj.MemberKey.Field")); // Throws InvalidCastException
}

// Any implementation of DynamicObject will fail 
class MemberType
{
    public int Field;
}
class DynamicType : DynamicObject
{
    private readonly Dictionary<string, object> _data = new();

    public override bool TryGetMember(GetMemberBinder binder, out object result)
    {
        if (_data.ContainsKey(binder.Name))
        {
            result = this[binder.Name];
            return true;
        }
        else return base.TryGetMember(binder, out result);
    }

    public object this[string key]
    {
        get => _data.GetValueOrDefault(key);
        set => _data[key] = value;
    }
}

Expected behavior

As in C#, the JS engine should return the value of the member's field (which is in this case 4). Instead, an exception is thrown.

Additional context

From looking at the callstack and digging around in the source a bit it seems like there is an issue with the implementation of the DynamicObjectAccessor. This accessor always sets the memberType to DynamicObject in its constructor, disregarding the actual type of the member field, which afaik cannot be known before being accessed. This then means the engine will also use the DynamicObjectAccessor when accessing any of its members, which requires a cast to DynamicObject that fails on any non-dynamic member types.

It is also worth noting that this only affects DynamicObject implementations, but not ExpandoObject.

Workaround I currently use a very crude fix to get the expected behavior: In ReflectionDescriptor.DoGet:

private JsValue DoGet(JsValue? thisObj)
{
    var value = _reflectionAccessor.GetValue(_engine, _target, _propertyName);
    // Previous: var type = _reflectionAccessor.MemberType;
    var type = _reflectionAccessor is DynamicObjectAccessor ? value?.GetType() ?? typeof(object) : _reflectionAccessor.MemberType;
    return JsValue.FromObjectWithType(_engine, value, type);
}

[lahma]

Thank you for the detailed problem description and investigation, should be fixed in main now.