Skip to content
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.

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

Hot reloads fail and breakpoints may not be hit if breakpoint scriptUris are prefixed with file:// #18441

Closed
DanTup opened this issue Jun 13, 2018 · 9 comments · Fixed by #21741
Assignees
Labels
a: debugging Debugging, breakpoints, expression evaluation

Comments

@DanTup
Copy link
Contributor

DanTup commented Jun 13, 2018

Currently when we send breakpoints from Dart Code we send each one in several different formats (local file system path, package: path, device-local path etc,) in an attempt to make sure we get the format the VM expects (since they're not well documented).

I'm trying to remove some of these and clean it up because there are other issues when the breakpoints in the IDE and those in the VM don't map one-to-one. I'm trying to settle on sending file system paths in a single common format, specifically as "URIs" like:

  • file:///blah/blah (macOS/Linux)
  • file://C:/blah/blah (Windows) Edit: Our test for this is passing on Windows now, so this may be macOS-only

However, if I send breakpoints in this format on Windows then hot reloads will time out (specifically, the PausePostRequest comes in, then we delete/re-add breakpoints, then resume, but it never hits the berakpoint and the hot reload reports a timeout). If I send without the file:// prefix it works fine. I think including the file:// prefix is correct; the field is called scriptUri (and there are other complications elsewhere if I don't send them).

Here are some logs of Flutter Run and Observatory for both with and without the prefixes. Here's how the end of the Flutter Run logs look:

With file:// prefix

[08:11:44]: ==> [{"id":"7","method":"app.restart","params":{"appId":"4e8251e6-1027-4f43-9490-21aad6001ad1","fullRestart":false,"pause":true}}]
[08:11:44]: <== [{"event":"app.progress","params":{"appId":"4e8251e6-1027-4f43-9490-21aad6001ad1","id":"5","progressId":"hot.reload","message":"Initializing hot reload..."}}]
[08:12:15]: <== [{"event":"app.progress","params":{"appId":"4e8251e6-1027-4f43-9490-21aad6001ad1","id":"5","progressId":"hot.reload","finished":true}}]
[08:12:15]: <== [{"id":"7","error":"TimeoutException: Request to Dart VM Service timed out: ext.flutter.evict({value: AssetManifest.json, isolateId: isolates/839716787})"}]

Without file:// prefix

[08:13:44]: ==> [{"id":"7","method":"app.restart","params":{"appId":"34b1335f-7804-48bd-a96a-7d638307f5ed","fullRestart":false,"pause":true}}]
[08:13:44]: <== [{"event":"app.progress","params":{"appId":"34b1335f-7804-48bd-a96a-7d638307f5ed","id":"4","progressId":"hot.reload","message":"Initializing hot reload..."}}]
[08:13:50]: <== [{"event":"app.progress","params":{"appId":"34b1335f-7804-48bd-a96a-7d638307f5ed","id":"4","progressId":"hot.reload","finished":true}}]
[08:13:50]: <== Reloaded 0 of 517 libraries in 5,750ms.
[08:13:50]: <== [{"id":"7","result":{"code":0,"message":"Reloaded 0 of 517 libraries"}}]

And here's what the Observatory log looks like after sending a resume after re-transmitting breakpoints:

Without file://

[08:11:45]: ==> {"id":"546","method":"resume","params":{"isolateId":"isolates/839716787"}}
[08:11:45]: <== {"jsonrpc":"2.0", "result":{"type":"Success"},"id":"546"}
[08:11:45]: <== {"jsonrpc":"2.0","method":"streamNotify","params":{"streamId":"Debug","event":{"type":"Event","kind":"Resume","isolate":{"type":"@Isolate","fixedId":true,"id":"isolates\/839716787","name":"main.dart:main()","number":"839716787"},"timestamp":1528873866356}}}

(it didn't hit a beakpoint, and the hot reload timed out)

Without file:// prefix

[08:13:44]: ==> {"id":"546","method":"resume","params":{"isolateId":"isolates/256354857"}}
[08:13:44]: <== {"jsonrpc":"2.0", "result":{"type":"Success"},"id":"546"}
[08:13:44]: <== {"jsonrpc":"2.0","method":"streamNotify","params":{"streamId":"Debug","event":{"type":"Event","kind":"Resume","isolate":{"type":"@Isolate","fixedId":true,"id":"isolates\/256354857","name":"main.dart:main()","number":"256354857"},"timestamp":1528873986076}}}
[08:13:45]: <== {"jsonrpc":"2.0","method":"streamNotify","params":{"streamId":"Debug","event":{"type":"Event","kind":"PauseBreakpoint","isolate":…

(the hot reload finished and the breakpoint was hit)

@DanTup DanTup changed the title Hot reloads fail on Windows if breaking scriptUris are prefixed with file:// Hot reloads fail on Windows if breakpoint scriptUris are prefixed with file:// Jun 13, 2018
@DanTup
Copy link
Contributor Author

DanTup commented Jun 13, 2018

@aam not sure if you have an ideas on this? The format of these paths is not well defined but I think there really should be a common format that works for all types (VM, Flutter Test, Flutter Run, Flutter Run after Hot Reload).

@DanTup
Copy link
Contributor Author

DanTup commented Jun 18, 2018

While trying to write an integration test for this, I found that this might be broken elsewhere too. If I send a breakpoint as a file:// URI on macOS it fails to hit the breakpoint:

final String breakpoint = new Uri.file(fs.path.join(_tempDir.path, 'lib', 'main.dart')).toString();
// final String breakpoint = fs.path.join(_tempDir.path, 'lib', 'main.dart');
await isolate.addBreakpoint(breakpoint, 8);

Depending on which one of those first two lines I uncomment, the test will either pass or time out (waiting for the isolate to pause).

@DanTup DanTup changed the title Hot reloads fail on Windows if breakpoint scriptUris are prefixed with file:// Hot reloads fail and breakpoints may not be hit if breakpoint scriptUris are prefixed with file:// Jun 18, 2018
@DanTup
Copy link
Contributor Author

DanTup commented Jun 18, 2018

There's a failing test in flutter_tools for this in #18560.

@aam aam self-assigned this Jun 19, 2018
@DanTup DanTup added the a: debugging Debugging, breakpoints, expression evaluation label Jun 26, 2018
DanTup added a commit that referenced this issue Jun 27, 2018
@DanTup
Copy link
Contributor Author

DanTup commented Jul 2, 2018

Btw, I don't have a Linux machine locally to test, but on the CI it seems like this issue happens on macOS but not on Linux (seems to work fine on Linux).

@DanTup
Copy link
Contributor Author

DanTup commented Aug 30, 2018

Our test that was hitting this on Windows seems to be passing now (see #21201 (comment)) so this may be macOS-only.

@DanTup
Copy link
Contributor Author

DanTup commented Oct 3, 2018

@aam this appears to be not working on macOS. I have an open PR here:

#22510

This changes the tests over to always using file URIs, however on macOS they fail. If you clone that branch on macOS you can repro by running expression_evaluation_test.dart. If you change it to use FS paths (by adding .toFilePath() to the call to isolate.addBreakpoint in test_driver.dart addBreakpoont() then the tests will pass.

@DanTup DanTup reopened this Oct 3, 2018
@DanTup
Copy link
Contributor Author

DanTup commented Oct 3, 2018

Scratch that - I just re-read what the fix was (symlinks). Same thing may just need applying here - I'll see if I can make the fix it more general.

@DanTup
Copy link
Contributor Author

DanTup commented Oct 3, 2018

Yep, same thing. Sorry for the bother! :)

@DanTup DanTup closed this as completed Oct 3, 2018
@github-actions
Copy link

github-actions bot commented Sep 1, 2021

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: debugging Debugging, breakpoints, expression evaluation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants