Skip to content

Log entire exception message instead of first line #1782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 10, 2022
Merged

Log entire exception message instead of first line #1782

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a2cd401
Include entire description up to stack trace
elliette Nov 9, 2022
0752841
Remove unnecessary import
elliette Nov 9, 2022
ce763c0
Add more tests
elliette Nov 10, 2022
2f3ba9a
Remove print statements
elliette Nov 10, 2022
c7654b1
Merge branch 'master' into assert-log
elliette Nov 10, 2022
39f43b3
Update CHANGELOG
elliette Nov 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions dwds/lib/src/debugging/inspector.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ class AppInspector implements AppInspectorInterface {
/// Regex used to extract the message from an exception description.
static final exceptionMessageRegex = RegExp(r'^.*$', multiLine: true);

/// Regex used to extract a stack trace line from the exception description.
static final stackTraceLineRegex = RegExp(r'^\s*at\s.*$', multiLine: true);

AppInspector._(
this._appConnection,
this._isolate,
Expand Down Expand Up @@ -611,8 +614,17 @@ class AppInspector implements AppInspectorInterface {
if (mappedStack == null || mappedStack.isEmpty) {
return description;
}
var message = exceptionMessageRegex.firstMatch(description)?.group(0);
message = (message != null) ? '$message\n' : '';
final message = _allLinesBeforeStackTrace(description);
return '$message$mappedStack';
}

String _allLinesBeforeStackTrace(String description) {
var message = '';
for (final match in exceptionMessageRegex.allMatches(description)) {
final isStackTraceLine = stackTraceLineRegex.hasMatch(match[0] ?? '');
if (isStackTraceLine) break;
message += '${match[0]}\n';
}
return message;
}
}
40 changes: 40 additions & 0 deletions dwds/test/inspector_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,20 @@ void main() {
});
});

group('mapExceptionStackTrace', () {
test('with a stack trace', () async {
final result =
await inspector.mapExceptionStackTrace(jsExceptionWithStackTrace);
expect(result, equals(formattedWithStackTrace));
});

test('without a stack trace', () async {
final result =
await inspector.mapExceptionStackTrace(jsExceptionNoStackTrace);
expect(result, equals(formattedNoStackTrace));
});
});

test('send toString', () async {
final remoteObject = await libraryPublicFinal();
final toString =
Expand Down Expand Up @@ -203,3 +217,29 @@ void main() {
});
});
}

final jsExceptionWithStackTrace = '''
Error: Assertion failed: org-dartlang-app:///web/scopes_main.dart:4:11
false
"THIS IS THE ASSERT MESSAGE"
at Object.assertFailed (org-dartlang-app:///web/scopes_main.dart.js:5297:15)
''';

final formattedWithStackTrace = '''
Error: Assertion failed: org-dartlang-app:///web/scopes_main.dart:4:11
false
"THIS IS THE ASSERT MESSAGE"
org-dartlang-app:///web/scopes_main.dart.js 5297:15 assertFailed
''';

final jsExceptionNoStackTrace = '''
Error: Assertion failed: org-dartlang-app:///web/scopes_main.dart:4:11
false
"THIS IS THE ASSERT MESSAGE"
''';

final formattedNoStackTrace = '''
Error: Assertion failed: org-dartlang-app:///web/scopes_main.dart:4:11
false
"THIS IS THE ASSERT MESSAGE"
''';
Copy link
Contributor

@annagrin annagrin Nov 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need more tests here - how about adding examples of regular stack traces in exceptions that a program throws, with multiple stack frames?

Copy link
Contributor

@annagrin annagrin Nov 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is an example of an exception description when I add throw Exception(blah at blahblah') to a dart web program:

Error: Exception: blah at blahblah
    at Object.throw_ [as throw] (http://127.0.0.1:56165/packages/build_web_compilers/src/dev_compiler/dart_sdk.js:5014:11)
    at main$ (http://127.0.0.1:56165/main.unsound.ddc.js:53:15)
at http://127.0.0.1:56165/main.dart.bootstrap.js:267:10
    at Array.forEach (<anonymous>)
at window.$dartRunMain (http://127.0.0.1:56165/main.dart.bootstrap.js:266:32)
    at <anonymous>:1:8
    at Object.runMain (http://127.0.0.1:56165/dwds/src/injected/client.js:8825:21)
    at http://127.0.0.1:56165/dwds/src/injected/client.js:25106:19
    at _wrapJsFunctionForAsync_closure.$protected (http://127.0.0.1:56165/dwds/src/injected/client.js:3448:15)
    at _wrapJsFunctionForAsync_closure.call$2 (http://127.0.0.1:56165/dwds/src/injected/client.js:11529:12)
    at Object._asyncStartSync (http://127.0.0.1:56165/dwds/src/injected/client.js:3412:20)
    at main__closure4.$call$body$main__closure (http://127.0.0.1:56165/dwds/src/injected/client.js:25122:16)
    at main__closure4.call$1 (http://127.0.0.1:56165/dwds/src/injected/client.js:25034:19)
    at StaticClosure._rootRunUnary (http://127.0.0.1:56165/dwds/src/injected/client.js:3816:16)
    at _CustomZone.runUnary$2$2 (http://127.0.0.1:56165/dwds/src/injected/client.js:12928:39)
    at _CustomZone.runUnaryGuarded$1$2 (http://127.0.0.1:56165/dwds/src/injected/client.js:12875:14)
    at _ControllerSubscription._sendData$1 (http://127.0.0.1:56165/dwds/src/injected/client.js:12468:19)
    at _DelayedData.perform$1 (http://127.0.0.1:56165/dwds/src/injected/client.js:12651:59)
    at _PendingEvents_schedule_closure.call$0 (http://127.0.0.1:56165/dwds/src/injected/client.js:12710:14)
    at Object._microtaskLoop (http://127.0.0.1:56165/dwds/src/injected/client.js:3656:24)
    at StaticClosure._startMicrotaskLoop (http://127.0.0.1:56165/dwds/src/injected/client.js:3662:11)
    at _AsyncRun__initializeScheduleImmediate_internalCallback.call$1 (http://127.0.0.1:56165/dwds/src/injected/client.js:11406:9)
    at invokeClosure (http://127.0.0.1:56165/dwds/src/injected/client.js:1227:26)
    at MutationObserver.<anonymous> (http://127.0.0.1:56165/dwds/src/injected/client.js:1246:18)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here is the same exception mapped to dart (we need to fix that translation it seems but this is another issue):

Error: Exception: blah at blahblah
dart:sdk_internal 5014:11             throw_
main.dart 20:3                        main$
main.dart.bootstrap.js 267:10         <fn>
main.dart.bootstrap.js 266:32         $dartRunMain
%3Canonymous%3E 1:8                   <fn>
dwds/src/injected/client.js 8825:21   runMain
dwds/src/injected/client.js 25106:19  <fn>
dwds/src/injected/client.js 3448:15   $protected
dwds/src/injected/client.js 11529:12  call$2
dwds/src/injected/client.js 3412:20   _asyncStartSync
dwds/src/injected/client.js 25122:16  $call$body$main__closure
dwds/src/injected/client.js 25034:19  call$1
dwds/src/injected/client.js 3816:16   _rootRunUnary
dwds/src/injected/client.js 12928:39  runUnary$2$2
dwds/src/injected/client.js 12875:14  runUnaryGuarded$1$2
dwds/src/injected/client.js 12468:19  _sendData$1
dwds/src/injected/client.js 12651:59  perform$1
dwds/src/injected/client.js 12710:14  call$0
dwds/src/injected/client.js 3656:24   _microtaskLoop
dwds/src/injected/client.js 3662:11   _startMicrotaskLoop
dwds/src/injected/client.js 11406:9   call$1
dwds/src/injected/client.js 1227:26   invokeClosure
dwds/src/injected/client.js 1246:18   <fn>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion! Added a few more test cases