Eliminate (caught) exceptions in startup for to simplify breaking-on-all-exceptions

[DanTup]

I have a long standing open issue in Dart-Code about using breaking on all exceptions. It seems to be quite common for people to enable this (in part, I think because Flutter catches exceptions and it's the only way to be able to examine state with the debugger), but when running tests there are a few places where exceptions are thrown during startup, for example:

To make things worse, VS Code doesn't handle this very well because there is no "user code" in the stack trace, so it doesn't know where to focus - so it doesn't focus anywhere. So what the user actually sees is:

I used to work around this by detecting Isolates that were the pub/test snapshots and never setting the exceptionPauseMode for them - however the names of isolates changed which broke that. I was able to re-do it using the isolateGroup names, however I discovered that the exception above is being thrown in the same isolate as the users test code, so that doesn't work either.

I don't have any great ideas for other fixes, so I'm wondering if it's possible to eliminate the exceptions that occur during startup here instead (the one in the screenshot above appears to be trying to parse <data:application/dart> as a URI)?

[jakemac53]

Agreed we should get rid of these if possible, I ran into this recently. I had to continue past at least 3 exceptions totally unrelated to my code before anything useful showed up :)

[DanTup]

If we can't get rid of some of them for any reason, another thought I had was having some ability to tell the test framework to pause just before running any tests/user code. That way we could delay setting the exception pause mode until then, rather than doing it before the test framework startup code. It's not as nice (since it needs specific handling from the editor), but it may be worth it to improve this experience.

[jakemac53]

The first exception is coming from the sdk, when trying to detect whether we are connected to a terminal or not, I don't see any real way of avoiding that one.

The stack trace ones are crashing due to how the data: uris appear in stack traces. The stack traces in question look like this:

#0      new Trace.current (package:stack_trace/src/trace.dart:87:43)
#1      Declarer.test (package:test_api/src/backend/declarer.dart:177:38)
#2      test (package:test_core/test_core.dart:138:13)
#3      main (<redacted>/test/pkgs/test/test/runner/compact_reporter_test.dart:16:3)
#4      _rootRun (dart:async/zone.dart:1190:13)
#5      _CustomZone.run (dart:async/zone.dart:1093:19)
#6      _runZoned (dart:async/zone.dart:1630:10)
#7      runZoned (dart:async/zone.dart:1550:10)
#8      Declarer.declare (package:test_api/src/backend/declarer.dart:131:7)
#9      RemoteListener.start.<anonymous closure>.<anonymous closure>.<anonymous closure> (package:test_api/src/remote_listener.dart:124:26)
<asynchronous suspension>
#10     RemoteListener.start.<anonymous closure>.<anonymous closure>.<anonymous closure> (package:test_api/src/remote_listener.dart)
#11     _rootRun (dart:async/zone.dart:1190:13)
#12     _CustomZone.run (dart:async/zone.dart:1093:19)
#13     _runZoned (dart:async/zone.dart:1630:10)
#14     runZonedGuarded (dart:async/zone.dart:1618:12)
#15     runZoned (dart:async/zone.dart:1547:12)
#16     RemoteListener.start.<anonymous closure>.<anonymous closure> (package:test_api/src/remote_listener.dart:70:9)
#17     _rootRun (dart:async/zone.dart:1190:13)
#18     _CustomZone.run (dart:async/zone.dart:1093:19)
#19     _runZoned (dart:async/zone.dart:1630:10)
#20     runZoned (dart:async/zone.dart:1550:10)
#21     StackTraceFormatter.asCurrent (package:test_api/src/backend/stack_trace_formatter.dart:42:7)
#22     RemoteListener.start.<anonymous closure> (package:test_api/src/remote_listener.dart:69:29)
#23     _rootRun (dart:async/zone.dart:1190:13)
#24     _CustomZone.run (dart:async/zone.dart:1093:19)
#25     _runZoned (dart:async/zone.dart:1630:10)
#26     runZoned (dart:async/zone.dart:1550:10)
#27     SuiteChannelManager.asCurrent (package:test_api/src/suite_channel_manager.dart:35:7)
#28     RemoteListener.start (package:test_api/src/remote_listener.dart:68:27)
#29     serializeSuite (package:test_core/src/runner/plugin/remote_platform_helpers.dart:35:20)
#30     main (<data:application/dart>:11:21)
#31     _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:297:32)
#32     _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:168:12)

Calling new Frame.parseVM(<line>) on line #30 throws because it isn't a valid uri due to the wrapping in <>. So the question is where that comes from and if its intentional. We could handle that inside the stack_trace package potentially but I am not sure why it comes through that way at all.

[DanTup]

I just noticed that this package already has a --pause-after-load flag. We're not currently using it because it's different to how we handle Dart (non-test) scripts (where we expect the isolates to be paused, and we unpause them - whereas this looks like a custom concept of pause/unpause).

It might be possible for VS Code to start using that, and just delay the setExceptionPauseMode calls until after the first pause event (I'm assuming we get an event) comes through.

[natebosch]

The first exception is coming from the sdk, when trying to detect whether we are connected to a terminal or not, I don't see any real way of avoiding that one.

Do you have more details? What line is this in the test runner? Do you have a stack for where this is in the SDK?

Calling new Frame.parseVM(<line>) on line 30 throws because it isn't a valid uri due to the wrapping in <>. So the question is where that comes from and if its intentional. We could handle that inside the stack_trace package potentially but I am not sure why it comes through that way at all.

Which line 30?

[jakemac53]

Do you have more details? What line is this in the test runner? Do you have a stack for where this is in the SDK?

Here is the call stack:

Stdout._terminalSize (dart:io-patch/stdio_patch.dart:146)
Stdout._hasTerminal (dart:io-patch/stdio_patch.dart:131)
Stdout.hasTerminal (dart:io/stdio.dart:222)
lineLength (Unknown Source:26)
lineLength (Unknown Source:0)
new PubCommandRunner (Unknown Source:84)
main (Unknown Source:8)
_startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:299)
_RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:168)

The way that the SDK detects whether a terminal is present relies on throwing an exception and catching it.

Which line 30?

#30 main (<data:application/dart>:11:21)

[natebosch]

https://dart-review.googlesource.com/c/sdk/+/152922 out for review to remove the exception during hasTerminal.

It looks like we used to get data: frames in a different format from the VM.

https://github.com/dart-lang/stacK_trace/blob/4d1ef4eddcd9b18c23bf19fd9d37245d3c81fa2e/test/frame_test.dart#L571-L575

That changed somewhere between Dart 2.1.0 and 2.2.0.

[natebosch]

@jakemac53 - For the stack trace issue, do you know where the throw comes in? The stack trace package looks like it will return UnparsedFrame() instead of throwing if you call Frame.parseVM with these examples.

Edit: I think I wasn't seeing the exception because I missed layer of wrapping in the package.

[DanTup]

Seems like these fixes are out - this doesn't happen in v2.9.0. Thanks!