Wrap VM service API methods in an error handler to convert any exceptions into type RPCError
#2144
Conversation
| Future<T> Function() asyncCallback, | ||
| ) { | ||
| try { | ||
| return asyncCallback(); |
There was a problem hiding this comment.
I think it needs to be await asyncCallback() to catch exceptions inside the callback. Also, would be great to add a test to make sure the exceptions are caught.
There was a problem hiding this comment.
Modified it to use asyncCallback().catchError(...) (adding an await would return result of type T not Future<T>.
Also pulled it into the shared utilities and added some tests.
dwds/lib/src/utilities/shared.dart
Outdated
| ) { | ||
| return asyncCallback().catchError((error) { | ||
| if (error is RPCError || error is SentinelException) { | ||
| throw error; |
There was a problem hiding this comment.
I wonder if it is possible to re-throw here? Otherwise we will lose the stack trace (it has been helpful in the past to see where the error happened)
There was a problem hiding this comment.
It isn't - but in reading the documentation I realized we can use test: to only catch exceptions that aren't RPCErrors or SentinelExceptions, so I've changed it to that.
|
|
||
| try { | ||
| await wrapInErrorHandlerAsync('exceptionCallback', exceptionCallback); | ||
| fail("RPCError not thrown."); |
There was a problem hiding this comment.
Optional: use throwsA matcher
For example (from chrome proxy server tests):
await expectLater( service.removeBreakpoint(isolate.id!, ''), throwsRPCError, );
There was a problem hiding this comment.
Hmm for some reason that pattern doesn't work on the last test case (it doesn't catch the exception). But I've updated the other test cases to that.
Fixes #2140, work towards flutter/flutter#126362