Skip to content

Commit aa4b8ee

Browse files
ilonatommylewing
ilonatommy
and
lewing
authored
[wasm][debugger] Fix exceptions on eval (#64374)
* Fixed surfacing ReturnAsError exceptions to browser and VS. * Added stack trace to the custom exception. * Update src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs Typo fix, thanks @lewing. Co-authored-by: Larry Ewing <lewing@microsoft.com> * Removed unnecessary changes. * Refactor. * Restored exception throwing on both null arguments. * Update src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs Co-authored-by: Larry Ewing <lewing@microsoft.com> * Applied @radical suggestions. * Getter fix. * Arguments reduction. * Remove IsErr. Co-authored-by: Larry Ewing <lewing@microsoft.com>
1 parent e0b0bb4 commit aa4b8ee

File tree

10 files changed

+66
-47
lines changed

10 files changed

+66
-47
lines changed

‎src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -125,42 +125,42 @@ public struct Result
125125
public JObject Value { get; private set; }
126126
public JObject Error { get; private set; }
127127

128-
public bool IsOk => Value != null;
129-
public bool IsErr => Error != null;
128+
public bool IsOk => Error == null;
130129

131-
private Result(JObject result, JObject error)
130+
private Result(JObject resultOrError, bool isError)
132131
{
133-
if (result != null && error != null)
134-
throw new ArgumentException($"Both {nameof(result)} and {nameof(error)} arguments cannot be non-null.");
135-
136-
bool resultHasError = string.Equals((result?["result"] as JObject)?["subtype"]?.Value<string>(), "error");
137-
if (result != null && resultHasError)
132+
bool resultHasError = isError || string.Equals((resultOrError?["result"] as JObject)?["subtype"]?.Value<string>(), "error");
133+
if (resultHasError)
138134
{
139-
this.Value = null;
140-
this.Error = result;
135+
Value = null;
136+
Error = resultOrError;
141137
}
142138
else
143139
{
144-
this.Value = result;
145-
this.Error = error;
140+
Value = resultOrError;
141+
Error = null;
146142
}
147143
}
148-
149144
public static Result FromJson(JObject obj)
150145
{
151-
//Log ("protocol", $"from result: {obj}");
152-
return new Result(obj["result"] as JObject, obj["error"] as JObject);
146+
var error = obj["error"] as JObject;
147+
if (error != null)
148+
return new Result(error, true);
149+
var result = obj["result"] as JObject;
150+
return new Result(result, false);
153151
}
154152

155-
public static Result Ok(JObject ok) => new Result(ok, null);
153+
public static Result Ok(JObject ok) => new Result(ok, false);
156154

157155
public static Result OkFromObject(object ok) => Ok(JObject.FromObject(ok));
158156

159-
public static Result Err(JObject err) => new Result(null, err);
157+
public static Result Err(JObject err) => new Result(err, true);
158+
159+
public static Result Err(string msg) => new Result(JObject.FromObject(new { message = msg }), true);
160160

161-
public static Result Err(string msg) => new Result(null, JObject.FromObject(new { message = msg }));
161+
public static Result UserVisibleErr(JObject result) => new Result { Value = result };
162162

163-
public static Result Exception(Exception e) => new Result(null, JObject.FromObject(new { message = e.Message }));
163+
public static Result Exception(Exception e) => new Result(JObject.FromObject(new { message = e.Message }), true);
164164

165165
public JObject ToJObject(MessageId target)
166166
{
@@ -186,7 +186,7 @@ public JObject ToJObject(MessageId target)
186186

187187
public override string ToString()
188188
{
189-
return $"[Result: IsOk: {IsOk}, IsErr: {IsErr}, Value: {Value?.ToString()}, Error: {Error?.ToString()} ]";
189+
return $"[Result: IsOk: {IsOk}, IsErr: {!IsOk}, Value: {Value?.ToString()}, Error: {Error?.ToString()} ]";
190190
}
191191
}
192192

‎src/mono/wasm/debugger/BrowserDebugProxy/DevToolsProxy.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ internal void SendResponse(MessageId id, Result result, CancellationToken token)
234234
private void SendResponseInternal(MessageId id, Result result, CancellationToken token)
235235
{
236236
JObject o = result.ToJObject(id);
237-
if (result.IsErr)
237+
if (!result.IsOk)
238238
logger.LogError($"sending error response for id: {id} -> {result}");
239239

240240
Send(this.ide, o, token);

‎src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs‎

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -390,17 +390,21 @@ internal static async Task<JObject> CompileAndRunTheExpression(string expression
390390
if (expressionTree == null)
391391
throw new Exception($"BUG: Unable to evaluate {expression}, could not get expression from the syntax tree");
392392

393-
try {
393+
try
394+
{
394395
var newScript = script.ContinueWith(
395396
string.Join("\n", findVarNMethodCall.variableDefinitions) + "\nreturn " + syntaxTree.ToString());
396397

397398
var state = await newScript.RunAsync(cancellationToken: token);
398-
399399
return JObject.FromObject(ConvertCSharpToJSType(state.ReturnValue, state.ReturnValue.GetType()));
400400
}
401-
catch (Exception)
401+
catch (CompilationErrorException cee)
402+
{
403+
throw new ReturnAsErrorException($"Cannot evaluate '{expression}': {cee.Message}", "CompilationError");
404+
}
405+
catch (Exception ex)
402406
{
403-
throw new ReturnAsErrorException($"Cannot evaluate '{expression}'.", "CompilationError");
407+
throw new Exception($"Internal Error: Unable to run {expression}, error: {ex.Message}.", ex);
404408
}
405409
}
406410

@@ -429,22 +433,38 @@ private static object ConvertCSharpToJSType(object v, Type type)
429433

430434
internal class ReturnAsErrorException : Exception
431435
{
432-
public Result Error { get; }
436+
private Result _error;
437+
public Result Error
438+
{
439+
get
440+
{
441+
_error.Value["exceptionDetails"]["stackTrace"] = StackTrace;
442+
return _error;
443+
}
444+
set { }
445+
}
433446
public ReturnAsErrorException(JObject error)
434447
=> Error = Result.Err(error);
435448

436449
public ReturnAsErrorException(string message, string className)
437450
{
438-
Error = Result.Err(JObject.FromObject(new
451+
var result = new
439452
{
440-
result = new
453+
type = "object",
454+
subtype = "error",
455+
description = message,
456+
className
457+
};
458+
_error = Result.UserVisibleErr(JObject.FromObject(
459+
new
441460
{
442-
type = "object",
443-
subtype = "error",
444-
description = message,
445-
className
446-
}
447-
}));
461+
result = result,
462+
exceptionDetails = new
463+
{
464+
exception = result,
465+
stackTrace = StackTrace
466+
}
467+
}));
448468
}
449469
}
450470
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ async Task<JObject> ResolveAsLocalOrThisMember(string name)
219219
if (scopeCache.Locals.Count == 0 && !localsFetched)
220220
{
221221
Result scope_res = await proxy.GetScopeProperties(sessionId, scopeId, token);
222-
if (scope_res.IsErr)
222+
if (!scope_res.IsOk)
223223
throw new Exception($"BUG: Unable to get properties for scope: {scopeId}. {scope_res}");
224224
localsFetched = true;
225225
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -674,7 +674,7 @@ private async Task<bool> CallOnFunction(MessageId id, JObject args, Cancellation
674674
return false;
675675
}
676676
Result res = await SendMonoCommand(id, MonoCommands.CallFunctionOn(RuntimeId, args), token);
677-
if (res.IsErr)
677+
if (!res.IsOk)
678678
{
679679
SendResponse(id, res, token);
680680
return true;
@@ -1004,7 +1004,7 @@ async Task<bool> SkipMethod(bool isSkippable, bool shouldBeSkipped, StepKind ste
10041004
private async Task<bool> OnReceiveDebuggerAgentEvent(SessionId sessionId, JObject args, CancellationToken token)
10051005
{
10061006
Result res = await SendMonoCommand(sessionId, MonoCommands.GetDebuggerAgentBufferReceived(RuntimeId), token);
1007-
if (res.IsErr)
1007+
if (!res.IsOk)
10081008
return false;
10091009

10101010
ExecutionContext context = GetContext(sessionId);

‎src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -435,10 +435,10 @@ public MonoBinaryReader(byte [] data) : base(new MemoryStream(data)) {}
435435
public static MonoBinaryReader From(Result result)
436436
{
437437
byte[] newBytes = Array.Empty<byte>();
438-
if (!result.IsErr) {
438+
if (result.IsOk) {
439439
newBytes = Convert.FromBase64String(result.Value?["result"]?["value"]?["value"]?.Value<string>());
440440
}
441-
return new MonoBinaryReader(new MemoryStream(newBytes), result.IsErr);
441+
return new MonoBinaryReader(new MemoryStream(newBytes), !result.IsOk);
442442
}
443443

444444
public override string ReadString()

‎src/mono/wasm/debugger/DebuggerTestSuite/CallFunctionOnTests.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,6 @@ public async Task CFOWithSilentReturnsErrors(string eval_fn, string bp_loc, int
534534
// doesn't get reported, and the execution is NOT paused even with setPauseOnException=true
535535
result = await cli.SendCommand("Runtime.callFunctionOn", cfo_args, token);
536536
Assert.False(result.IsOk, "result.IsOk");
537-
Assert.True(result.IsErr, "result.IsErr");
538537

539538
var hasErrorMessage = result.Error["exceptionDetails"]?["exception"]?["description"]?.Value<string>()?.Contains(error_msg);
540539
Assert.True((hasErrorMessage ?? false), "Exception message not found");
@@ -785,7 +784,7 @@ public async Task RunOnInvalidCfoId(string eval_fn, string bp_loc, int line, int
785784
});
786785

787786
var res = await cli.SendCommand("Runtime.callFunctionOn", cfo_args, token);
788-
Assert.True(res.IsErr);
787+
Assert.False(res.IsOk);
789788
});
790789

791790
[Theory]
@@ -811,7 +810,7 @@ public async Task RunOnInvalidThirdSegmentOfObjectId(string eval_fn, string bp_l
811810
});
812811

813812
var res = await cli.SendCommand("Runtime.callFunctionOn", cfo_args, token);
814-
Assert.True(res.IsErr);
813+
Assert.False(res.IsOk);
815814
}
816815

817816
[Theory]

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,7 +1008,7 @@ internal async Task<Result> RemoveBreakpoint(string id, bool expect_ok = true)
10081008
});
10091009

10101010
var res = await cli.SendCommand("Debugger.removeBreakpoint", remove_bp, token);
1011-
Assert.True(expect_ok ? res.IsOk : res.IsErr);
1011+
Assert.True(expect_ok ? res.IsOk : !res.IsOk);
10121012

10131013
return res;
10141014
}
@@ -1020,7 +1020,7 @@ internal async Task<Result> SetBreakpoint(string url_key, int line, int column,
10201020
JObject.FromObject(new { lineNumber = line, columnNumber = column, urlRegex = url_key, condition });
10211021

10221022
var bp1_res = await cli.SendCommand("Debugger.setBreakpointByUrl", bp1_req, token);
1023-
Assert.True(expect_ok ? bp1_res.IsOk : bp1_res.IsErr);
1023+
Assert.True(expect_ok ? bp1_res.IsOk : !bp1_res.IsOk);
10241024

10251025
return bp1_res;
10261026
}

‎src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ public async Task OpenSessionAsync(Func<InspectorClient, CancellationToken, List
259259
}
260260

261261
Result res = completedTask.Result;
262-
if (res.IsErr)
262+
if (!res.IsOk)
263263
throw new ArgumentException($"Command {cmd_name} failed with: {res.Error}. Remaining commands: {RemainingCommandsToString(cmd_name, init_cmds)}");
264264

265265
init_cmds.RemoveAt(cmdIdx);

‎src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public async Task ExceptionThrownInJS()
3434
});
3535

3636
var eval_res = await cli.SendCommand("Runtime.evaluate", eval_req, token);
37-
Assert.True(eval_res.IsErr);
37+
Assert.False(eval_res.IsOk);
3838
Assert.Equal("Uncaught", eval_res.Error["exceptionDetails"]?["text"]?.Value<string>());
3939
}
4040

@@ -245,7 +245,7 @@ await EvaluateAndCheck(
245245
});
246246

247247
var frame_props = await cli.SendCommand("Runtime.getProperties", get_prop_req, token);
248-
Assert.True(frame_props.IsErr);
248+
Assert.False(frame_props.IsOk);
249249
}
250250
);
251251
}

0 commit comments

Comments
 (0)