Skip to content

fix: delay request if process not ready yet #340

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion packages/custom_lint/lib/src/v2/custom_lint_analyzer_plugin.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import 'package:analyzer_plugin/src/protocol/protocol_internal.dart'
show ResponseResult;
import 'package:async/async.dart';
import 'package:custom_lint_core/custom_lint_core.dart';
import 'package:meta/meta.dart';
import 'package:package_config/package_config.dart';
import 'package:path/path.dart' as p;
import 'package:pub_semver/pub_semver.dart';
Expand Down Expand Up @@ -114,6 +115,11 @@ class CustomLintServer {
/// Whether plugins should include lints used for debugging.
final bool includeBuiltInLints;

/// Allow mocking a SocketCustomLintServerToClientChannel for test
@visibleForTesting
Future<SocketCustomLintServerToClientChannel?> get clientChannel =>
_clientChannel.safeFirst;

late final StreamSubscription<void> _requestSubscription;
StreamSubscription<void>? _clientChannelEventsSubscription;
late PluginVersionCheckParams _pluginVersionCheckParams;
Expand All @@ -122,6 +128,7 @@ class CustomLintServer {
BehaviorSubject<SocketCustomLintServerToClientChannel?>();
final _contextRoots = BehaviorSubject<AnalysisSetContextRootsParams>();
final _runner = PendingOperation();
final _delayedRequest = <Request>[];

/// A shorthand for accessing the current list of context roots.
Future<List<ContextRoot>?> get _allContextRoots {
Expand Down Expand Up @@ -186,7 +193,10 @@ class CustomLintServer {
orElse: () async {
return _runner.run(() async {
final clientChannel = await _clientChannel.safeFirst;
if (clientChannel == null) return null;
if (clientChannel == null || !clientChannel.initialized) {
_delayedRequest.add(request);
return null;
}

final response =
await clientChannel.sendAnalyzerPluginRequest(request);
Expand Down Expand Up @@ -291,6 +301,7 @@ class CustomLintServer {
return _closeFuture = Future(() async {
// Cancel pending operations
await _contextRoots.close();
_delayedRequest.clear();

// Flushes logs before stopping server.
await _runner.wait();
Expand Down Expand Up @@ -396,6 +407,14 @@ class CustomLintServer {
await clientChannel.init(
debug: configs.any((e) => e != null && e.debug),
);
_sendDelayedRequest();
}

void _sendDelayedRequest() {
for (final request in _delayedRequest) {
unawaited(_handleRequest(request));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of all of this logic if we're unawaiting the requests anyway?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is same _requestSubscription, we just need replay request after delay, so no need to await or handle response. I also added test for this MR

}
_delayedRequest.clear();
}

Future<void> _handleEvent(CustomLintEvent event) => _runner.run(() async {
Expand Down
13 changes: 13 additions & 0 deletions packages/custom_lint/lib/src/v2/server_to_client_channel.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import 'dart:io';

import 'package:analyzer_plugin/protocol/protocol.dart';
import 'package:analyzer_plugin/protocol/protocol_generated.dart';
import 'package:meta/meta.dart';
import 'package:path/path.dart';
import 'package:uuid/uuid.dart';

Expand Down Expand Up @@ -81,6 +82,16 @@ class SocketCustomLintServerToClientChannel {
final CustomLintWorkspace _workspace;

AnalysisSetContextRootsParams _contextRoots;
bool _initialized = false;

/// List request before initialized
@visibleForTesting
List<Request> initialRequest = [];

/// Initial state
///
/// Returns `true` if requested `analysis.setContextRoots`
bool get initialized => _initialized;

late final Stream<CustomLintMessage> _messages = _channel.messages
.map((e) => e! as Map<String, Object?>)
Expand Down Expand Up @@ -127,6 +138,7 @@ class SocketCustomLintServerToClientChannel {
sendAnalyzerPluginRequest(_version.toRequest(const Uuid().v4())),
sendAnalyzerPluginRequest(_contextRoots.toRequest(const Uuid().v4())),
]);
_initialized = true;
}

/// Updates the context roots on the client
Expand Down Expand Up @@ -231,6 +243,7 @@ void main(List<String> args) async {
/// Sends a request based on the analyzer_plugin protocol, expecting
/// an analyzer_plugin response.
Future<Response> sendAnalyzerPluginRequest(Request request) async {
if (!_initialized) initialRequest.add(request);
final response = await sendCustomLintRequest(
CustomLintRequest.analyzerPluginRequest(request, id: request.id),
);
Expand Down
5 changes: 3 additions & 2 deletions packages/custom_lint/test/run_plugin.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ Future<List<AnalysisErrorsParams>> runServerInCliModeForApp(
}

class ManualRunner {
ManualRunner(this.runner, this.channel);
ManualRunner(this.runner, this.channel, this.server);

final CustomLintRunner runner;
final ServerIsolateChannel channel;
final CustomLintServer server;

Future<void> get initialize => runner.initialize;

Expand Down Expand Up @@ -91,7 +92,7 @@ Future<ManualRunner> startRunnerForApp(

unawaited(runner.initialize);

return ManualRunner(runner, channel);
return ManualRunner(runner, channel, customLintServer);
});
}

Expand Down
38 changes: 38 additions & 0 deletions packages/custom_lint/test/server_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,44 @@ void fn() {}
);
});

test('Handles request before server initialized', () async {
final plugin = createPlugin(name: 'test_lint', main: helloWordPluginSource);

final app = createLintUsage(
source: {
'lib/main.dart': '''
void fn() {}

void fn2() {}
''',
'lib/another.dart': 'void fn() {}\n',
},
plugins: {'test_lint': plugin.uri},
name: 'test_app',
);

final runner = await startRunnerForApp(app, includeBuiltInLints: false);
final another = File(join(app.path, 'lib', 'another.dart'));
another.writeAsStringSync('test');
await runner.channel.sendRequest(
AnalysisUpdateContentParams({
another.path: AddContentOverlay(another.readAsStringSync()),
}),
);
await runner.initialize;

final initialRequest = (await runner.server.clientChannel)?.initialRequest;
// Request send before `SocketCustomLintServerToClientChannel.init` must delay,
// for ensure initialRequest is [PluginVersionCheckParams, AnalysisSetContextRootsParams]
expect(initialRequest, hasLength(2));
expect(
initialRequest?.last.method,
AnalysisSetContextRootsParams([]).toRequest('test').method,
);

await runner.close();
});

test('Handles files getting deleted', () async {
// Regression test for https://github.com/invertase/dart_custom_lint/issues/105
final plugin = createPlugin(name: 'test_lint', main: helloWordPluginSource);
Expand Down