From b0810bc939285ff339dc4645ebf7ef089e4a0675 Mon Sep 17 00:00:00 2001 From: Christopher Fujino Date: Fri, 29 Oct 2021 17:02:05 -0700 Subject: [PATCH] Fix analysis throwing string (#91435) --- .../lib/src/commands/analyze_once.dart | 7 +- .../flutter_tools/lib/src/dart/analysis.dart | 25 +++++- .../src/runner/flutter_command_runner.dart | 1 - .../commands.shard/hermetic/analyze_test.dart | 84 +++++++++++++++++++ 4 files changed, 114 insertions(+), 3 deletions(-) diff --git a/packages/flutter_tools/lib/src/commands/analyze_once.dart b/packages/flutter_tools/lib/src/commands/analyze_once.dart index a648a1a5ede..0101f0190b3 100644 --- a/packages/flutter_tools/lib/src/commands/analyze_once.dart +++ b/packages/flutter_tools/lib/src/commands/analyze_once.dart @@ -125,7 +125,12 @@ class AnalyzeOnce extends AnalyzeBase { // Completing the future in the callback can't fail. unawaited(server.onExit.then((int exitCode) { if (!analysisCompleter.isCompleted) { - analysisCompleter.completeError('analysis server exited: $exitCode'); + analysisCompleter.completeError( + // Include the last 20 lines of server output in exception message + Exception( + 'analysis server exited with code $exitCode and output:\n${server.getLogs(20)}', + ), + ); } })); diff --git a/packages/flutter_tools/lib/src/dart/analysis.dart b/packages/flutter_tools/lib/src/dart/analysis.dart index e5f572e56b0..f8eadcd1847 100644 --- a/packages/flutter_tools/lib/src/dart/analysis.dart +++ b/packages/flutter_tools/lib/src/dart/analysis.dart @@ -79,7 +79,7 @@ class AnalysisServer { final Stream errorStream = _process!.stderr .transform(utf8.decoder) .transform(const LineSplitter()); - errorStream.listen(_logger.printError); + errorStream.listen(_handleError); final Stream inStream = _process!.stdout .transform(utf8.decoder) @@ -94,6 +94,28 @@ class AnalysisServer { {'included': directories, 'excluded': []}); } + final List _logs = []; + + /// Aggregated STDOUT and STDERR logs from the server. + /// + /// This can be surfaced to the user if the server crashes. If [tail] is null, + /// returns all logs, else only the last [tail] lines. + String getLogs([int? tail]) { + if (tail == null) { + return _logs.join('\n'); + } + // Since List doesn't implement a .tail() method, we reverse it then use + // .take() + final Iterable reversedLogs = _logs.reversed; + final List firstTailLogs = reversedLogs.take(tail).toList(); + return firstTailLogs.reversed.join('\n'); + } + + void _handleError(String message) { + _logs.add('[stderr] $message'); + _logger.printError(message); + } + bool get didServerErrorOccur => _didServerErrorOccur; Stream get onAnalyzing => _analyzingController.stream; @@ -113,6 +135,7 @@ class AnalysisServer { } void _handleServerResponse(String line) { + _logs.add('[stdout] $line'); _logger.printTrace('<== $line'); final dynamic response = json.decode(line); diff --git a/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart b/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart index 542ae22acff..595772da8cc 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart @@ -312,7 +312,6 @@ class FlutterCommandRunner extends CommandRunner { return []; } - final List projectPaths = globals.fs.directory(rootPath) .listSync(followLinks: false) .expand((FileSystemEntity entity) { diff --git a/packages/flutter_tools/test/commands.shard/hermetic/analyze_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/analyze_test.dart index 0a03725c573..303f97c5dfd 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/analyze_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/analyze_test.dart @@ -4,15 +4,25 @@ // @dart = 2.8 +import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:file/memory.dart'; +import 'package:flutter_tools/src/artifacts.dart'; +import 'package:flutter_tools/src/base/logger.dart'; +import 'package:flutter_tools/src/base/platform.dart'; +import 'package:flutter_tools/src/base/terminal.dart'; import 'package:flutter_tools/src/cache.dart'; +import 'package:flutter_tools/src/commands/analyze.dart'; import 'package:flutter_tools/src/commands/analyze_base.dart'; import 'package:flutter_tools/src/dart/analysis.dart'; import '../../src/common.dart'; +import '../../src/context.dart'; +import '../../src/fake_process_manager.dart'; +import '../../src/test_flutter_command_runner.dart'; const String _kFlutterRoot = '/data/flutter'; +const int SIGABRT = -6; void main() { testWithoutContext('analyze generate correct errors message', () async { @@ -35,6 +45,80 @@ void main() { ); }); + group('analyze command', () { + FileSystem fileSystem; + Platform platform; + BufferLogger logger; + FakeProcessManager processManager; + Terminal terminal; + AnalyzeCommand command; + CommandRunner runner; + + setUpAll(() { + Cache.disableLocking(); + }); + + setUp(() { + fileSystem = MemoryFileSystem.test(); + platform = FakePlatform(); + logger = BufferLogger.test(); + processManager = FakeProcessManager.empty(); + terminal = Terminal.test(); + command = AnalyzeCommand( + artifacts: Artifacts.test(), + fileSystem: fileSystem, + logger: logger, + platform: platform, + processManager: processManager, + terminal: terminal, + ); + runner = createTestCommandRunner(command); + + // Setup repo roots + const String homePath = '/home/user/flutter'; + Cache.flutterRoot = homePath; + for (final String dir in ['dev', 'examples', 'packages']) { + fileSystem.directory(homePath).childDirectory(dir).createSync(recursive: true); + } + }); + + testUsingContext('SIGABRT throws Exception', () async { + const String stderr = 'Something bad happened!'; + processManager.addCommands( + [ + const FakeCommand( + // artifact paths are from Artifacts.test() and stable + command: [ + 'HostArtifact.engineDartSdkPath/bin/dart', + '--disable-dart-dev', + 'HostArtifact.engineDartSdkPath/bin/snapshots/analysis_server.dart.snapshot', + '--disable-server-feature-completion', + '--disable-server-feature-search', + '--sdk', + 'HostArtifact.engineDartSdkPath', + ], + exitCode: SIGABRT, + stderr: stderr, + ), + ], + ); + await expectLater( + runner.run(['analyze']), + throwsA( + isA().having( + (Exception e) => e.toString(), + 'description', + contains('analysis server exited with code $SIGABRT and output:\n[stderr] $stderr'), + ), + ), + ); + }, + overrides: { + FileSystem: () => fileSystem, + ProcessManager: () => processManager, + }); + }); + testWithoutContext('analyze inRepo', () { final FileSystem fileSystem = MemoryFileSystem.test(); fileSystem.directory(_kFlutterRoot).createSync(recursive: true);