sdk icon indicating copy to clipboard operation
sdk copied to clipboard

Enabling analyzer plugins prevents dart analyze running without an internet connection

Open incendial opened this issue 3 years ago • 4 comments

Hi, received a very strange report https://github.com/dart-code-checker/dart-code-metrics/issues/976 that having a plugins entry filled in the analysis_options (thus enabling the plugin), actually prevents dart analyze to run when there is no internet connection.

I truly believe that DCM code has nothing to do with it, could you please take a look? In the linked issue there is a repro example.

incendial avatar Sep 09 '22 11:09 incendial

/fyi @bwilkerson

pq avatar Sep 09 '22 18:09 pq

It would appear that the code to run pub to prepare a plugin during execution doesn't adequately guard against failures due to the lack of an internet connection.

What ought to happen is that the plugin should be marked as being un-runnable and then the remainder of analysis should proceed as if the plugin hadn't been installed.

bwilkerson avatar Sep 09 '22 21:09 bwilkerson

It would appear that the code to run pub to prepare a plugin during execution doesn't adequately guard against failures due to the lack of an internet connection.

What ought to happen is that the plugin should be marked as being un-runnable and then the remainder of analysis should proceed as if the plugin hadn't been installed.

Thanks! Can the plugin preparation be cached? I mean, the analyzer right now doesn't wait for the plugin to finish so this problem only affects the analyzer, but when it will wait, discarding the plugin analysis result only when the internet connection is lost looks rather implicit.

incendial avatar Sep 10 '22 15:09 incendial

Can the plugin preparation be cached?

It's been a long time since I've looked at this code, so I might not be remembering correctly, but I thought it was already caching the installed plugin. But probably I'm remembering something that I intended to implement but didn't get around to doing.

So, as long as the plugin is only added to the cache after pub has successfully run, I don't see any problem with that.

Of course, it won't stop the hang if pub fails because of not being able to access the internet, it will just negate the need to run pub more than once.

bwilkerson avatar Sep 12 '22 20:09 bwilkerson

This came up today on the Flutter discord so I'm adding some notes here while in my head in case they're useful.

@deadlyrazer reported that the analysis server was broken whenever internet access to some sites in their country was restricted. Capturing an instrumentation log didn't initially reveal much - the server seemed to start up but then there was just no traffic and the server was unresponsive.

The log did reveal that the custom_lint plugin was being loaded, so they tried without it and that seemed to solve the problem.

So I created a project using the plugin and configured in invalid DNS server (I think DNS is how their sites are being blocked), and I was able to reproduce the issue. The server did start working after some time (but far longer than I would probably have waited before declaring it broken) and at that point the instrumentation log contained a little more information that led me here:

1683730145263:Err:Failed to run pub upgrade
  pluginFolder = /Users/danny/.dartServer/.plugin_manager/ccb9db0201e329c63e111ea0c2842525/analyzer_plugin
  exitCode = 69
  stdout = Resolving dependencies...

  stderr = Got socket error trying to find package custom_lint at https:://pub.dev.

I suspect the issue is around here, where the Pub process is run synchronously and server waits for it to complete:

https://github.com/dart-lang/sdk/blob/ea21df9dbe71d92e8795bbe63423a55519788c84/pkg/analysis_server/lib/src/plugin/plugin_manager.dart#L634-L639

Because it's runSync, I think it will block this isolate entirely for the duration (and if this is in the main server isolate, that would explain server not being responsive to anything else). For me the process did eventually quit, but I don't know if there are cases (certain types of internet connectivity issues) where it might keep retrying - or how the amount of time before it exits may vary.

Even if the plugins can't function in this situation, I wonder if this code could be in some way async and not block the isolate? (I don't know if server needs to know about all plugins at startup, or they could be "loaded" later).

Of course, how much effort it's worth spending here may also depend on the roadmap for plugins.

DanTup avatar May 10 '23 15:05 DanTup

Plugins could potentially be loaded later, but we're probably missing any kind of signal indicating that the analysis contexts would need to be reanalyzed when that happened. I'm not sure what the impact of that would be.

bwilkerson avatar May 10 '23 15:05 bwilkerson