Dart-code: Provide an action to restart the analysis server

[jacob314]

Certain workflows involve actively switching branches a lot or using generated files in ways that require restarting the Dart analysis server for everything to work.

Long term, it would be nice if the analysis server never had to be to restarted but short term it would be really convenient to have an action to restart the server. IntelliJ has a button to refresh the analysis server which some users use 20X a day for this workflow.
Fyi @matanlurey

[DanTup]

I'm not a big fan of doing this (it's been discussed a lot before and we've had some success tracking down the issues with good repros), but if the root causes are hard to track down I don't add friction.

I've added this command:

Note: This currently restarts the extension and not just the analyzer. It will unregister and reregister everything in the extension. This means things like the Flutter device daemon will also be restarted (this is the same restart that happens when you change the SDK path or a setting that the server depends on). This is a little more aggressive than it needs to be, but much lighter than Reload Window which will tear down the whole extension host (and all other extensions). We don't currently have a clean way to restart only the analysis server (it could be done though).

If you use this command 10 times within a single workspace session, a prompt will be shown (only once, on the 10th call within hat workspace session) encouraging you to file an issue:

[matanlurey]

Thanks @DanTup. Personally I have been told my rationale for restarting (generated finals and switching branches internally) isn't prioritized, so I can't really file a bug - my only alternative is restarting the server (which I currently do with killall dart), so this will help a lot.

[DanTup]

generated finals and switching branches internally

I'm curious what "generated finals" are? Is switching branches internally different to externally? We did track down an issue relating to changing branches (https://github.com/Dart-Code/Dart-Code/issues/2329), but I wouldn't be surprised if there are more (the watching code seems quite complex because of differences across platforms etc.). If the issue is reproducible externally (and you have any idea of what sort of thing might cause it), I'd be interesting in poking around.

[matanlurey]

Ah sorry, generated _files_.

[matanlurey]

I unfortunately don't work externally in any work capacity so I am not able to provide a reproduction externally.

[DanTup]

Ah sorry, generated files.

Ah!

Generally, changing branches and generating files should work fine. The server watches the disk and should just handle everything. If you spot any particular patterns that might trigger it to go wrong that might be useful to repro, do let me know. With the LSP server being used in more places, this might come up where restarting the server isn't so easy, so it'd be good to get to the bottom of it.

[DanTup]

@matanlurey there's a beta release here that includes this if you want this functionality now:

https://github.com/Dart-Code/Dart-Code/releases/tag/v3.13.0-beta.3

There are no more planned changes before release (unless bugs come up). Provisional release notes are available at https://dartcode.org/releases/v3-13/.

[samandmoore]

My team and I also run into the need to restart the analysis server pretty often. We use the package freezed to generate data classes and union types for most of the models in our codebase. The codegen step takes a minute or two to run and when it's done, there are analysis errors for all of the generated files, for us this is over a hundred files. I've found that if I CMD+Click into one of the classes in a file, that seems to cause the analysis server to refresh for that file and the errors go away, but doing that for every file would be awful. Similarly, when switching branches with all these generate files, it's really common to see that new generated files on the new branch will exhibit this same analysis error behavior.

For what it's worth, on a much smaller project, the analysis server does just fine. I have a project with 5 generated files and the changes are picked up immediately and no analysis errors are shown.

This leads me to believe it has something to do with the number of files / generated files / files changed and recreated.

[DanTup]

@samandmoore thanks for the info! If you're able to make a project you can share that triggers this, I'd be interested in understanding what's going wrong (otherwise, if you can share an instrumentation log and details of what the codegen actions where, that might be useful).

[DanTup]

I'm wondering if the servers "reanalyze" method might be better thing to call here instead of restarting the server:

https://htmlpreview.github.io/?https://github.com/dart-lang/sdk/blob/master/pkg/analysis_server/doc/api.html#request_analysis.reanalyze

This would be much lighter (if it works) because it doesn't recreate all of the providers, re-scan for SDKs, etc.

I might add it as a new command with a plan to remove the restart option in the following version (so include both for a month) if it seems to also do the job.

[samandmoore]

@samandmoore thanks for the info! If you're able to make a project you can share that triggers this, I'd be interested in understanding what's going wrong (otherwise, if you can share an instrumentation log and details of what the codegen actions where, that might be useful).

I will look into creating and sharing an instrumentation log.

I'm wondering if the servers "reanalyze" method might be better thing to call here instead of restarting the server

That's interesting. Do you know if there's a way to invoke that behavior manually in the interim? If so, I could test it out.

[DanTup]

Looking at the code, we currently call this only if the dart.showTodos setting is changed. So you could try toggling that option next time you see this and see if it helps (@matanlurey it may be useful for you to test this too.. it'll be much faster than the new command and if it works, we can replace it with one that just calls this).

Thanks!

[samandmoore]

Looking at the code, we currently call this only if the dart.showTodos setting is changed.

@DanTup i just tested this. toggling the dart.showTodos setting caused analysis to re-run and it picked up the codegen changes which cleared the analysis errors. 🎉

i will test a few more times and report back if it _doesn't_ work.

[samandmoore]

oh, and i set up the instrumentation log. i will share that here once i reproduce this issue again.

[DanTup]

Great, thanks!

There's a preview build f the next version here you can switch to if you want - it has a command Dart: Reanalyze Project:

https://github.com/Dart-Code/Dart-Code/releases/tag/v3.14.0-alpha.1

Unless anyone reports this not working, I will remove the Dart: Restart Analysis Server command in a future release (however right now, it's not supported for LSP, so I'll leave it until that's supported at least).

[samandmoore]

@DanTup i will give that a try.

i'm curious, is there a reason that you'd want to remove the restart command? i'd like it not to be necessary, but it feels like it could be a useful tool in a pinch. i totally understand if you want to remove it though to avoid having to maintain it.

also, if it's helpful, i'm not using LSP yet because a few of my projects have a custom line width setting and i'm not using a new enough dart version with the fix for that. not sure if that impacts any of the issues i'm reporting, so i wanted to share 😄

[samandmoore]

thanks so much for all your work on this ❤️

[DanTup]

i'm curious, is there a reason that you'd want to remove the restart command?

Currently the restart command does a lot more than restart the analysis server - it also unregisters and reregistered all commands and providers with VS Code. This is quite expensive and also could be error-prone (for example if we forget to add something to the "subscriptions" list to be unregistered during the shutdown, we could end up with two registrations, or an error when trying to reregister - I don't think there's anything missing today, but it's an easy mistake to make in new code).

So if reanalyze works as expected, it's much faster and much less likely to break.

i'm not using LSP yet because a few of my projects have a custom line width setting and i'm not using a new enough dart version with the fix for that. not sure if that impacts any of the issues i'm reporting, so i wanted to share 😄

It probably doesn't change anything for this issue, but it definitely could be relevant when reporting other bugs. In the latest release I added which mode you're running in (DAS/LSP) to the log header, so generally if you're capturing logs it'll be included in there anyway (though if you're capturing an analyzer log it's generally obvious from the logged traffic anyway).

[samandmoore]

that makes sense. thanks for the explanation.