While reviewing logs for flaky DAP tests, I found that these tests have been failing sometimes with errors that look like the isolate might not have paused on the `debugger()` call yet:
```
evaluateInFrame: (113) Expression compilation error
_buildExpressionEvaluationScope: invalid 'frameIndex' parameter: 0
```
and
```
evaluateInFrame: (113) Expression compilation error
org-dartlang-debug:synthetic_debug_expression:1:1: Error: Undefined name 'myInstance'.
myInstance
^^^^^^^^^^
```
This waits for the isolate to be paused, not just runnable, which I think might fix this.
Change-Id: Ib1d6b1c44a7ef93452065aa43c4d05058914db28
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/395302
Reviewed-by: Derek Xu <derekx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
devtools_shared 11.0.0 updates some signatures to use a new DTDInfo class to support both a local + exposed URI for DTD. A new CLI flag `--dtd-exposed-uri` for `devtools server` allows passing this second URI.
The DevTools server will use `--dtd-uri` to connect to DTD itself, but serve `--dtd-exposed-uri` to the frontend.
`--dtd-exposed-uri` is entirely optional and if not supplied, the value from `--dtd-uri` will be used in its place.
Change-Id: I5ab052ff9c4e7b2b186c1592f1ba2d63b7711113
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/383400
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Kenzie Davisson <kenzieschmoll@google.com>
Reviewed-by: Kenzie Davisson <kenzieschmoll@google.com>
Part of the process to remove uses of
DartDevelopmentService.startDartDevelopmentService in favor of launching
with `dart development-service`.
Change-Id: I857aee932c0b2fc6ae2172eb106b78c76d536295
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/380760
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
Before this change, Service.controlWebServer would block on the first
call, waiting until DDS had finished starting to respond, while
subsequent calls would return as soon as the VM service HTTP server had
started, exposing the internal VM service URI. This could result in
clients connecting directly to the VM service while DDS was
initializing, only for them to be disconnected immediately once DDS
invoked _yieldControlToDDS.
Fixes https://github.com/dart-lang/sdk/issues/56254
TEST=pkg/dds/test/control_web_server_regress_56254_test.dart
Change-Id: Ic707882263caa9ffe832e768c37492dfde43b611
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/379700
Auto-Submit: Ben Konyi <bkonyi@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
Reviewed-by: Liam Appelbe <liama@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Capturing debug logs that includes VM Service traffic is useful for debugging. Previously with the new DAPs, this could only be enabled at session start which meant users would have to capture very large log files.
This adds the ability to enable/disable this logging while a debug session is already running. A related change in Dart-Code will send this request when you use the "Capture Debugging Logs" command, so you can start a debug session, get to the point of reproducing an issue, and then just capture the relevant logs (VM Service traffic included).
Fixes https://github.com/Dart-Code/Dart-Code/issues/5197
Change-Id: I7e766e57bd80819eb5da164e544903fae23927ed
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/377222
Commit-Queue: Helin Shiah <helinx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
This reverts commit 3370a4e393.
Reason for revert: Fixes landed in google3 (cl/650630536).
Original change's description:
> Revert "Reapply "[ Service ] Start DDS and serve DevTools when the VM service is started via dart:developer""
>
> This reverts commit 44d4451476.
>
> Reason for revert: b/350443042
>
> Original change's description:
> > Reapply "[ Service ] Start DDS and serve DevTools when the VM service is started via dart:developer"
> >
> > In the previous version of this change, if the user had 'dart' on their
> > PATH and invoked 'dart compile js' (which spawns the VM service after
> > compilation completes), the VM service would attempt to spawn DDS using
> > './dart' as the executable path instead of 'dart'. This would result in
> > DDS failing to start, causing the VM to print an error and hang.
> >
> > This updated change checks to see if the parent directory of
> > `Platform.executable` is '.' and then verifies if './dart' exists or
> > not. If it doesn't, 'dart' is likely on the user's PATH and should be
> > used directly as the executable path.
> >
> > See https://github.com/dart-lang/sdk/issues/56087 for details.
> >
> > This reverts commit 4b88698e48.
> >
> > TEST=pkg/dds/test/control_web_server_starts_dds_with_dart_on_path_test.dart
> >
> > Change-Id: Id0f1dadd01d9202cbf7717f31393b43171cf3968
> > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/373561
> > Auto-Submit: Ben Konyi <bkonyi@google.com>
> > Reviewed-by: Derek Xu <derekx@google.com>
> > Commit-Queue: Ben Konyi <bkonyi@google.com>
>
> Change-Id: I424c4b91b0b108ae4c9dffa0059ed90c918897e3
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/373744
> Reviewed-by: Siva Annamalai <asiva@google.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Reviewed-by: Srujan Gaddam <srujzs@google.com>
> Commit-Queue: Siva Annamalai <asiva@google.com>
> Auto-Submit: Ivan Inozemtsev <iinozemtsev@google.com>
Change-Id: Id7ddff5a80399203fb8058d84bc89fda2073da95
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/375000
Auto-Submit: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
This reverts commit 44d4451476.
Reason for revert: b/350443042
Original change's description:
> Reapply "[ Service ] Start DDS and serve DevTools when the VM service is started via dart:developer"
>
> In the previous version of this change, if the user had 'dart' on their
> PATH and invoked 'dart compile js' (which spawns the VM service after
> compilation completes), the VM service would attempt to spawn DDS using
> './dart' as the executable path instead of 'dart'. This would result in
> DDS failing to start, causing the VM to print an error and hang.
>
> This updated change checks to see if the parent directory of
> `Platform.executable` is '.' and then verifies if './dart' exists or
> not. If it doesn't, 'dart' is likely on the user's PATH and should be
> used directly as the executable path.
>
> See https://github.com/dart-lang/sdk/issues/56087 for details.
>
> This reverts commit 4b88698e48.
>
> TEST=pkg/dds/test/control_web_server_starts_dds_with_dart_on_path_test.dart
>
> Change-Id: Id0f1dadd01d9202cbf7717f31393b43171cf3968
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/373561
> Auto-Submit: Ben Konyi <bkonyi@google.com>
> Reviewed-by: Derek Xu <derekx@google.com>
> Commit-Queue: Ben Konyi <bkonyi@google.com>
Change-Id: I424c4b91b0b108ae4c9dffa0059ed90c918897e3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/373744
Reviewed-by: Siva Annamalai <asiva@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Commit-Queue: Siva Annamalai <asiva@google.com>
Auto-Submit: Ivan Inozemtsev <iinozemtsev@google.com>
In the previous version of this change, if the user had 'dart' on their
PATH and invoked 'dart compile js' (which spawns the VM service after
compilation completes), the VM service would attempt to spawn DDS using
'./dart' as the executable path instead of 'dart'. This would result in
DDS failing to start, causing the VM to print an error and hang.
This updated change checks to see if the parent directory of
`Platform.executable` is '.' and then verifies if './dart' exists or
not. If it doesn't, 'dart' is likely on the user's PATH and should be
used directly as the executable path.
See https://github.com/dart-lang/sdk/issues/56087 for details.
This reverts commit 4b88698e48.
TEST=pkg/dds/test/control_web_server_starts_dds_with_dart_on_path_test.dart
Change-Id: Id0f1dadd01d9202cbf7717f31393b43171cf3968
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/373561
Auto-Submit: Ben Konyi <bkonyi@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
This reverts commit 64d4689a78.
Reason for revert: Platform.executable doesn't return a path, just an executable name, which can cause breakages.
TEST=ci
Original change's description:
> [ Service ] Start DDS and serve DevTools when the VM service is started via dart:developer
>
> TEST=pkg/dds/test/control_web_server_starts_dds_test.dart
>
> Change-Id: I2e51783592912e5a719685f2ab5e7537b7a59586
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366560
> Commit-Queue: Ben Konyi <bkonyi@google.com>
> Reviewed-by: Derek Xu <derekx@google.com>
Change-Id: I4ceaee4b5ca8f5557e90cd914ee69fe9aa5f85a7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/373381
Reviewed-by: Siva Annamalai <asiva@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
Commit-Queue: Siva Annamalai <asiva@google.com>
This moves the CLI logic into lib/src/dds_cli_entrypoint.dart so it can
be invoked by a custom google3 entrypoint (bin/dds.dart can't be
imported using a package: URI).
Since google3 will be able to perform some custom configuration of DDS,
google3 related logic (e.g., `BazelUriConverter`) is also removed. This
is technically a breaking change, but should be safe as this
functionality isn't currently being used.
Change-Id: I54d8a9927ff2df70e013ca5c8bc1d510b0b95f02
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/371520
Reviewed-by: Derek Xu <derekx@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
Auto-Submit: Ben Konyi <bkonyi@google.com>
When a class contains both a field and getter with the same name, we'd include both in the `variables` response. In most cases these are the same value (although it's not guaranteed).
I've chosen to just hide the field in this case and always show the result from evaluating the getter (since I think that's what the user would expect, even in the case where they happen to have different values). Another option could be to show both (but change the name so that fields/getters are shown differently), however in that would change the display (for example adding `get ` in front of all getters) we should probably only do that if it's clear there is demand for it.
Fixes https://github.com/Dart-Code/Dart-Code/issues/5128
Change-Id: I9e23d22a844ee22c38988456b1f275422c5c9e04
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/370640
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
The test runner only runs files that end with "_test.dart". This is
evidenced by
https://dart-current-results.web.app/#/filter=pkg/dds/test&showAll
not showing any results for the tests renamed in this CL.
The tests renamed in this CL were found by runnning
`find pkg/dds/test | sed "/.*_test[.]dart/d"`
Some changes had to be made to
pkg/dds/test/client_resume_approvals_no_longer_require_permission_test.dart
after renaming it, the other tests did not have to be changed after
getting renamed.
Change-Id: Ib3b30ba784b182e4cc014d93aa5d8a792f8bb457
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/367700
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
IntelliJ expects a "type" field for events; otherwise the JSON will not be parsed correctly. This came up as part of making the widget rebuild table in DevTools clickable (https://github.com/flutter/devtools/issues/4564)
I'm not totally sure this is the change I need, as I don't know how to test this out end-to-end (build Dart SDK and put into Flutter SDK, then run an app with these changes in DDS?). I think it's right because I'm modifying a spot that was first added in https://dart-review.googlesource.com/c/sdk/+/304981, which added `postEvent` in dds_service_extension.
Change-Id: I9a78903dca7855896b6a9b9c3f9e1d62dcb7099e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/368202
Reviewed-by: Dan Chevalier <danchevalier@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
This test shows up as flaky quite often. It's waiting for breakpoint Resolved events by just calling `await pumpEventQueue(times: 5000);` but it appears this sometimes doesn't get all of the events.
This change waits up to 5s for the events to arrive (because we unfortunately have no signal to know when they should have come through). I'm hopeful 5s is more than enough and this won't flake, but we could increase this as not - the goal of this test is not to verify performance but just ensure the events do arrive.
Change-Id: Ia2236b65bbb5d11e2a5519d49869c6b87202a940
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/367301
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Sometimes we see unhandled exceptions during this setup work, see:
- https://github.com/flutter/flutter/issues/148346
- https://github.com/flutter/flutter/issues/148348
Currently these are unhandled and bring the debug adapter down. In VS Code, the error (send to stderr) is not visible to the user so they just see a silent crash which makes it very difficult to report (and the Flutter crash reports don't have any context).
This change will send the exception to the client and then cleanly terminate, which should give the user more information to open a good bug report (assuming the issue wasn't just something like them terminating the app as it was starting).
Change-Id: I4aefbc278e6a0708924c6fa41c5179d581117689
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366662
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
In a recent change I wrapped these exceptions to provide better failure messages in tests, but I missed updating these tests that had matchers against them.
For reasons I don't yet understand, the trybots that ran on that change didn't include these tests so didn't catch this.
Change-Id: I5227842d2317bbe33186670141586c315f98a5c9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366665
Reviewed-by: Helin Shiah <helinx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Helin Shiah <helinx@google.com>
Danny Tuppeny