[wasm][debugger] Remove JToken properties that are for internal use only

src/mono/wasm/debugger/BrowserDebugProxy/Common/InternalUseFieldName.cs

@@ -0,0 +1,38 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#nullable enable
+
+using System.Collections.Generic;
+
+namespace Microsoft.WebAssembly.Diagnostics;
+
+internal sealed class InternalUseFieldName
+{
+    public static InternalUseFieldName Hidden = new(nameof(Hidden));
+    public static InternalUseFieldName State = new(nameof(State));
+    public static InternalUseFieldName Section = new(nameof(Section));
+    public static InternalUseFieldName Owner = new(nameof(Owner));
+    public static InternalUseFieldName IsStatic = new(nameof(IsStatic));
+    public static InternalUseFieldName IsNewSlot = new(nameof(IsNewSlot));
+    public static InternalUseFieldName IsBackingField = new(nameof(IsBackingField));
+    public static InternalUseFieldName ParentTypeId = new(nameof(ParentTypeId));
+
+    private static readonly HashSet<string> s_names = new()
+    {
+        Hidden.Name,
+        State.Name,
+        Section.Name,
+        Owner.Name,
+        IsStatic.Name,
+        IsNewSlot.Name,
+        IsBackingField.Name,
+        ParentTypeId.Name
+    };
+
+    private InternalUseFieldName(string fieldName) => Name = $"__{fieldName}__";
+
+    public static int Count => s_names.Count;
+    public static bool IsKnown(string name) => !string.IsNullOrEmpty(name) && s_names.Contains(name);
+    public string Name { get; init; }
+}

src/mono/wasm/debugger/BrowserDebugProxy/MemberObjectsExplorer.cs

@@ -63,17 +63,17 @@ private static async Task<JObject> ReadFieldValue(
                 // for backing fields, we are getting it from the properties
                 typePropertiesBrowsableInfo.TryGetValue(field.Name, out state);
             }
-            fieldValue["__state"] = state?.ToString();
-            fieldValue["__section"] = field.Attributes.HasFlag(FieldAttributes.Private)
+            fieldValue[InternalUseFieldName.State.Name] = state?.ToString();
+            fieldValue[InternalUseFieldName.Section.Name] = field.Attributes.HasFlag(FieldAttributes.Private)
                 ? "private" : "result";
 
             if (field.IsBackingField)
             {
-                fieldValue["__isBackingField"] = true;
-                fieldValue["__parentTypeId"] = parentTypeId;
+                fieldValue[InternalUseFieldName.IsBackingField.Name] = true;
+                fieldValue[InternalUseFieldName.ParentTypeId.Name] = parentTypeId;
             }
             if (field.Attributes.HasFlag(FieldAttributes.Static))
-                fieldValue["__isStatic"] = true;
+                fieldValue[InternalUseFieldName.IsStatic.Name] = true;
 
             if (getObjectOptions.HasFlag(GetObjectCommandOptions.WithSetter))
             {
@@ -236,7 +236,7 @@ public static async Task<JArray> ExpandFieldValues(
                 if (typeInfo.Info.IsNonUserCode && getCommandOptions.HasFlag(GetObjectCommandOptions.JustMyCode) && field.Attributes.HasFlag(FieldAttributes.Private))
                     continue;
 
-                if (!Enum.TryParse(fieldValue["__state"].Value<string>(), out DebuggerBrowsableState fieldState)
+                if (!Enum.TryParse(fieldValue[InternalUseFieldName.State.Name].Value<string>(), out DebuggerBrowsableState fieldState)
                     || fieldState == DebuggerBrowsableState.Collapsed)
                 {
                     fieldValues.Add(fieldValue);
@@ -296,11 +296,9 @@ public static async Task<JArray> GetExpandedMemberValues(
 
             JArray GetHiddenElement()
             {
-                return new JArray(JObject.FromObject(new
-                {
-                    name = namePrefix,
-                    __hidden = true
-                }));
+                var emptyHidden = JObject.FromObject(new { name = namePrefix });
+                emptyHidden.Add(InternalUseFieldName.Hidden.Name, true);
+                return new JArray(emptyHidden);
             }
         }
 
@@ -361,14 +359,14 @@ public static async Task<Dictionary<string, JObject>> ExpandPropertyValues(
                     continue;
                 }
 
-                bool isExistingMemberABackingField = existingMember["__isBackingField"]?.Value<bool>() == true;
+                bool isExistingMemberABackingField = existingMember[InternalUseFieldName.IsBackingField.Name]?.Value<bool>() == true;
                 if (isOwn && !isExistingMemberABackingField)
                 {
                     // repeated propname on the same type! cannot happen
                     throw new Exception($"Internal Error: should not happen. propName: {propName}. Existing all members: {string.Join(",", allMembers.Keys)}");
                 }
 
-                bool isExistingMemberABackingFieldOwnedByThisType = isExistingMemberABackingField && existingMember["__owner"]?.Value<string>() == typeName;
+                bool isExistingMemberABackingFieldOwnedByThisType = isExistingMemberABackingField && existingMember[InternalUseFieldName.Owner.Name]?.Value<string>() == typeName;
                 if (isExistingMemberABackingField && (isOwn || isExistingMemberABackingFieldOwnedByThisType))
                 {
                     // this is the property corresponding to the backing field in *this* type
@@ -382,8 +380,8 @@ public static async Task<Dictionary<string, JObject>> ExpandPropertyValues(
                 {
                     // this has `new` keyword if it is newSlot but direct child was not a newSlot:
                     var child = allMembers.FirstOrDefault(
-                        kvp => (kvp.Key == propName || kvp.Key.StartsWith($"{propName} (")) && kvp.Value["__parentTypeId"]?.Value<int>() == typeId).Value;
-                    bool wasOverriddenByDerivedType = child != null && child["__isNewSlot"]?.Value<bool>() != true;
+                        kvp => (kvp.Key == propName || kvp.Key.StartsWith($"{propName} (")) && kvp.Value[InternalUseFieldName.ParentTypeId.Name]?.Value<int>() == typeId).Value;
+                    bool wasOverriddenByDerivedType = child != null && child[InternalUseFieldName.IsNewSlot.Name]?.Value<bool>() != true;
                     if (wasOverriddenByDerivedType)
                     {
                         /*
@@ -409,7 +407,7 @@ public static async Task<Dictionary<string, JObject>> ExpandPropertyValues(
                  */
 
                 JObject backingFieldForHiddenProp = allMembers.GetValueOrDefault(overriddenOrHiddenPropName);
-                if (backingFieldForHiddenProp is null || backingFieldForHiddenProp["__isBackingField"]?.Value<bool>() != true)
+                if (backingFieldForHiddenProp is null || backingFieldForHiddenProp[InternalUseFieldName.IsBackingField.Name]?.Value<bool>() != true)
                 {
                     // hiding with a non-auto property, so nothing to adjust
                     // add the new property
@@ -424,12 +422,12 @@ public static async Task<Dictionary<string, JObject>> ExpandPropertyValues(
 
             async Task UpdateBackingFieldWithPropertyAttributes(JObject backingField, string autoPropName, MethodAttributes getterMemberAccessAttrs, DebuggerBrowsableState? state)
             {
-                backingField["__section"] = getterMemberAccessAttrs switch
+                backingField[InternalUseFieldName.Section.Name] = getterMemberAccessAttrs switch
                 {
                     MethodAttributes.Private => "private",
                     _ => "result"
                 };
-                backingField["__state"] = state?.ToString();
+                backingField[InternalUseFieldName.State.Name] = state?.ToString();
 
                 if (state is null)
                     return;
@@ -472,16 +470,16 @@ async Task AddProperty(
                 }
 
                 propRet["isOwn"] = isOwn;
-                propRet["__section"] = getterAttrs switch
+                propRet[InternalUseFieldName.Section.Name] = getterAttrs switch
                 {
                     MethodAttributes.Private => "private",
                     _ => "result"
                 };
-                propRet["__state"] = state?.ToString();
+                propRet[InternalUseFieldName.State.Name] = state?.ToString();
                 if (parentTypeId != -1)
                 {
-                    propRet["__parentTypeId"] = parentTypeId;
-                    propRet["__isNewSlot"] = isNewSlot;
+                    propRet[InternalUseFieldName.ParentTypeId.Name] = parentTypeId;
+                    propRet[InternalUseFieldName.IsNewSlot.Name] = isNewSlot;
                 }
 
                 string namePrefix = GetNamePrefixForValues(propNameWithSufix, typeName, isOwn, state);
@@ -586,7 +584,7 @@ public static async Task<GetMembersResult> GetObjectMemberValues(
                     if (getCommandType.HasFlag(GetObjectCommandOptions.AccessorPropertiesOnly))
                     {
                         foreach (var f in allFields)
-                            f["__hidden"] = true;
+                            f[InternalUseFieldName.Hidden.Name] = true;
                     }
                     AddOnlyNewFieldValuesByNameTo(allFields, allMembers, typeName, isOwn);
                 }
@@ -632,8 +630,8 @@ static void AddOnlyNewFieldValuesByNameTo(JArray namedValues, IDictionary<string
                     if (valuesDict.TryAdd(name, item as JObject))
                     {
                         // new member
-                        if (item["__isBackingField"]?.Value<bool>() == true)
-                            item["__owner"] = typeName;
+                        if (item[InternalUseFieldName.IsBackingField.Name]?.Value<bool>() == true)
+                            item[InternalUseFieldName.Owner.Name] = typeName;
                         continue;
                     }
 
@@ -676,6 +674,33 @@ public GetMembersResult(JArray value, bool sortByAccessLevel)
             PrivateMembers = t.PrivateMembers;
         }
 
+        public void CleanUp()
+        {
+            JProperty[] toRemoveInObject = new JProperty[InternalUseFieldName.Count];
+
+            CleanUpJArray(Result);
+            CleanUpJArray(PrivateMembers);
+
+            void CleanUpJArray(JArray arr)
+            {
+                foreach (JToken item in arr)
+                {
+                    if (item is not JObject jobj || jobj.Count == 0)
+                        continue;
+
+                    int removeCount = 0;
+                    foreach (JProperty jp in jobj.Properties())
+                    {
+                        if (InternalUseFieldName.IsKnown(jp.Name))
+                            toRemoveInObject[removeCount++] = jp;
+                    }
+
+                    for (int i = 0; i < removeCount; i++)
+                        toRemoveInObject[i].Remove();
+                }
+            }
+        }
+
         public static GetMembersResult FromValues(IEnumerable<JToken> values, bool splitMembersByAccessLevel = false) =>
             FromValues(new JArray(values), splitMembersByAccessLevel);
 
@@ -694,10 +719,10 @@ public static GetMembersResult FromValues(JArray values, bool splitMembersByAcce
 
         private void Split(JToken member)
         {
-            if (member["__hidden"]?.Value<bool>() == true)
+            if (member[InternalUseFieldName.Hidden.Name]?.Value<bool>() == true)
                 return;
 
-            if (member["__section"]?.Value<string>() is not string section)
+            if (member[InternalUseFieldName.Section.Name]?.Value<string>() is not string section)
             {
                 Result.Add(member);
                 return;

src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs

@@ -262,9 +262,14 @@ async Task<JObject> ResolveAsLocalOrThisMember(string name)
             {
                 if (scopeCache.Locals.Count == 0 && !localsFetched)
                 {
-                    Result scope_res = await proxy.GetScopeProperties(sessionId, scopeId, token);
-                    if (!scope_res.IsOk)
-                        throw new ExpressionEvaluationFailedException($"BUG: Unable to get properties for scope: {scopeId}. {scope_res}");
+                    try
+                    {
+                        await proxy.GetScopeProperties(sessionId, scopeId, token);
+                    }
+                    catch (Exception ex)
+                    {
+                        throw new ExpressionEvaluationFailedException($"BUG: Unable to get properties for scope: {scopeId}. {ex}");
+                    }
                     localsFetched = true;
                 }
 

src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs

@@ -756,15 +756,15 @@ internal async Task<ValueOrError<GetMembersResult>> RuntimeGetObjectMembers(Sess
             {
                 switch (objectId.Scheme)
                 {
+                    // ToDo: fix Exception types here
                     case "scope":
-                        Result resScope = await GetScopeProperties(id, objectId.Value, token);
-                        return resScope.IsOk
-                            ? ValueOrError<GetMembersResult>.WithValue(
-                                new GetMembersResult((JArray)resScope.Value?["result"], sortByAccessLevel: false))
-                            : ValueOrError<GetMembersResult>.WithError(resScope);
+                        GetMembersResult resScope = await GetScopeProperties(id, objectId.Value, token);
+                        resScope.CleanUp();
+                        return ValueOrError<GetMembersResult>.WithValue(resScope);
                     case "valuetype":
                         var resValue = await MemberObjectsExplorer.GetValueTypeMemberValues(
                             context.SdbAgent, objectId.Value, getObjectOptions, token, sortByAccessLevel, includeStatic: true);
+                        resValue?.CleanUp();
                         return resValue switch
                         {
                             null => ValueOrError<GetMembersResult>.WithError($"Could not get properties for {objectId}"),
@@ -779,6 +779,7 @@ internal async Task<ValueOrError<GetMembersResult>> RuntimeGetObjectMembers(Sess
                     case "object":
                         var resObj = await MemberObjectsExplorer.GetObjectMemberValues(
                             context.SdbAgent, objectId.Value, getObjectOptions, token, sortByAccessLevel, includeStatic: true);
+                        resObj.CleanUp();
                         return ValueOrError<GetMembersResult>.WithValue(resObj);
                     case "pointer":
                         var resPointer = new JArray { await context.SdbAgent.GetPointerContent(objectId.Value, token) };
@@ -1407,36 +1408,35 @@ private async Task<bool> OnEvaluateOnCallFrame(MessageId msg_id, int scopeId, st
             return true;
         }
 
-        internal async Task<Result> GetScopeProperties(SessionId msg_id, int scopeId, CancellationToken token)
+        internal async Task<GetMembersResult> GetScopeProperties(SessionId msg_id, int scopeId, CancellationToken token)
         {
             try
             {
                 ExecutionContext context = GetContext(msg_id);
                 Frame scope = context.CallStack.FirstOrDefault(s => s.Id == scopeId);
                 if (scope == null)
-                    return Result.Err(JObject.FromObject(new { message = $"Could not find scope with id #{scopeId}" }));
+                    throw new Exception($"Could not find scope with id #{scopeId}");
 
                 VarInfo[] varIds = scope.Method.Info.GetLiveVarsAt(scope.Location.IlLocation.Offset);
 
                 var values = await context.SdbAgent.StackFrameGetValues(scope.Method, context.ThreadId, scopeId, varIds, token);
                 if (values != null)
                 {
                     if (values == null || values.Count == 0)
-                        return Result.OkFromObject(new { result = Array.Empty<object>() });
+                        return new GetMembersResult();
 
                     PerScopeCache frameCache = context.GetCacheForScope(scopeId);
                     foreach (JObject value in values)
                     {
                         frameCache.Locals[value["name"]?.Value<string>()] = value;
                     }
-                    return Result.OkFromObject(new { result = values });
+                    return GetMembersResult.FromValues(values);
                 }
-                return Result.OkFromObject(new { result = Array.Empty<object>() });
+                return new GetMembersResult();
             }
             catch (Exception exception)
             {
-                Log("verbose", $"Error resolving scope properties {exception.Message}");
-                return Result.Exception(exception);
+                throw new Exception($"Error resolving scope properties {exception.Message}");
             }
         }
 

src/mono/wasm/debugger/BrowserDebugProxy/ValueTypeClass.cs

@@ -98,15 +98,15 @@ JObject GetFieldWithMetadata(FieldTypeClass field, JObject fieldValue, bool isSt
                 if (isStatic)
                     fieldValue["name"] = field.Name;
                 FieldAttributes attr = field.Attributes & FieldAttributes.FieldAccessMask;
-                fieldValue["__section"] = attr == FieldAttributes.Private ? "private" : "result";
+                fieldValue[InternalUseFieldName.Section.Name] = attr == FieldAttributes.Private ? "private" : "result";
 
                 if (field.IsBackingField)
                 {
-                    fieldValue["__isBackingField"] = true;
+                    fieldValue[InternalUseFieldName.IsBackingField.Name] = true;
                     return fieldValue;
                 }
                 typeFieldsBrowsableInfo.TryGetValue(field.Name, out DebuggerBrowsableState? state);
-                fieldValue["__state"] = state?.ToString();
+                fieldValue[InternalUseFieldName.State.Name] = state?.ToString();
                 return fieldValue;
             }
         }
@@ -244,7 +244,7 @@ public async Task ExpandedFieldValues(MonoSDBHelper sdbHelper, bool includeStati
             JArray visibleFields = new();
             foreach (JObject field in fields)
             {
-                if (!Enum.TryParse(field["__state"]?.Value<string>(), out DebuggerBrowsableState state))
+                if (!Enum.TryParse(field[InternalUseFieldName.State.Name]?.Value<string>(), out DebuggerBrowsableState state))
                 {
                     visibleFields.Add(field);
                     continue;

src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs

@@ -925,6 +925,18 @@ internal async Task<JToken> GetObjectOnLocals(JToken locals, string name)
             return await GetProperties(objectId);
         }
 
+        internal void AssertInternalUseFieldsAreRemoved(JToken item)
+        {
+            if (item is JObject jobj && jobj.Count != 0)
+            {
+                foreach (JProperty jp in jobj.Properties())
+                {
+                    Assert.False(InternalUseFieldName.IsKnown(jp.Name),
+                     $"Property {jp.Name} of object: {jobj} is for internal proxy use and should not be exposed externally.");
+                }
+            }
+        }
+
         /* @fn_args is for use with `Runtime.callFunctionOn` only */
         internal virtual async Task<JToken> GetProperties(string id, JToken fn_args = null, bool? own_properties = null, bool? accessors_only = null, bool expect_ok = true)
         {
@@ -978,6 +990,7 @@ internal virtual async Task<JToken> GetProperties(string id, JToken fn_args = nu
             {
                 foreach (var p in locals)
                 {
+                    AssertInternalUseFieldsAreRemoved(p);
                     if (p["name"]?.Value<string>() == "length" && p["enumerable"]?.Value<bool>() != true)
                     {
                         p.Remove();
@@ -1035,6 +1048,7 @@ internal virtual async Task<JToken> GetProperties(string id, JToken fn_args = nu
             {
                 foreach (var p in locals)
                 {
+                    AssertInternalUseFieldsAreRemoved(p);
                     if (p["name"]?.Value<string>() == "length" && p["enumerable"]?.Value<bool>() != true)
                     {
                         p.Remove();