Commit cccd917a authored by Dan Rubel's avatar Dan Rubel Committed by GitHub

Refactor flutter command exit code - part 1 (#6803)

* convert flutter commands to use ToolExit for non-zero exit code
* add convenience method throwToolExit
* print ToolExit message iff not null
parent b04f35e8
...@@ -104,7 +104,8 @@ Future<Null> main(List<String> args) async { ...@@ -104,7 +104,8 @@ Future<Null> main(List<String> args) async {
// Argument error exit code. // Argument error exit code.
_exit(64); _exit(64);
} else if (error is ToolExit) { } else if (error is ToolExit) {
stderr.writeln(error.message); if (error.message != null)
stderr.writeln(error.message);
if (verbose) { if (verbose) {
stderr.writeln(); stderr.writeln();
stderr.writeln(chain.terse.toString()); stderr.writeln(chain.terse.toString());
......
...@@ -23,6 +23,14 @@ String get homeDirPath { ...@@ -23,6 +23,14 @@ String get homeDirPath {
} }
String _homeDirPath; String _homeDirPath;
/// Throw a specialized exception for expected situations
/// where the tool should exit with a clear message to the user
/// and no stack trace unless the --verbose option is specified.
/// For example: network errors
void throwToolExit(String message, { int exitCode }) {
throw new ToolExit(message, exitCode: exitCode);
}
/// Specialized exception for expected situations /// Specialized exception for expected situations
/// where the tool should exit with a clear message to the user /// where the tool should exit with a clear message to the user
/// and no stack trace unless the --verbose option is specified. /// and no stack trace unless the --verbose option is specified.
......
...@@ -20,7 +20,7 @@ Future<List<int>> fetchUrl(Uri url) async { ...@@ -20,7 +20,7 @@ Future<List<int>> fetchUrl(Uri url) async {
printTrace('Received response statusCode=${response.statusCode}'); printTrace('Received response statusCode=${response.statusCode}');
if (response.statusCode != 200) { if (response.statusCode != 200) {
throw new ToolExit( throwToolExit(
'Download failed: $url\n' 'Download failed: $url\n'
' because (${response.statusCode}) ${response.reasonPhrase}', ' because (${response.statusCode}) ${response.reasonPhrase}',
exitCode: kNetworkProblemExitCode, exitCode: kNetworkProblemExitCode,
......
...@@ -9,6 +9,7 @@ import 'dart:io'; ...@@ -9,6 +9,7 @@ import 'dart:io';
import 'package:args/args.dart'; import 'package:args/args.dart';
import 'package:path/path.dart' as path; import 'package:path/path.dart' as path;
import '../base/common.dart';
import '../base/logger.dart'; import '../base/logger.dart';
import '../base/utils.dart'; import '../base/utils.dart';
import '../cache.dart'; import '../cache.dart';
...@@ -53,7 +54,10 @@ class AnalyzeContinuously extends AnalyzeBase { ...@@ -53,7 +54,10 @@ class AnalyzeContinuously extends AnalyzeBase {
await server.start(); await server.start();
final int exitCode = await server.onExit; final int exitCode = await server.onExit;
printStatus('Analysis server exited with code $exitCode.'); String message = 'Analysis server exited with code $exitCode.';
if (exitCode != 0)
throwToolExit(message, exitCode: exitCode);
printStatus(message);
return 0; return 0;
} }
......
...@@ -10,6 +10,7 @@ import 'package:args/args.dart'; ...@@ -10,6 +10,7 @@ import 'package:args/args.dart';
import 'package:path/path.dart' as path; import 'package:path/path.dart' as path;
import 'package:yaml/yaml.dart' as yaml; import 'package:yaml/yaml.dart' as yaml;
import '../base/common.dart';
import '../cache.dart'; import '../cache.dart';
import '../dart/analysis.dart'; import '../dart/analysis.dart';
import '../globals.dart'; import '../globals.dart';
...@@ -123,12 +124,18 @@ class AnalyzeOnce extends AnalyzeBase { ...@@ -123,12 +124,18 @@ class AnalyzeOnce extends AnalyzeBase {
// prepare a union of all the .packages files // prepare a union of all the .packages files
if (dependencies.hasConflicts) { if (dependencies.hasConflicts) {
printError(dependencies.generateConflictReport()); StringBuffer message = new StringBuffer();
printError('Make sure you have run "pub upgrade" in all the directories mentioned above.'); message.writeln(dependencies.generateConflictReport());
if (dependencies.hasConflictsAffectingFlutterRepo) message.writeln('Make sure you have run "pub upgrade" in all the directories mentioned above.');
printError('For packages in the flutter repository, try using "flutter update-packages --upgrade" to do all of them at once.'); if (dependencies.hasConflictsAffectingFlutterRepo) {
printError('If this does not help, to track down the conflict you can use "pub deps --style=list" and "pub upgrade --verbosity=solver" in the affected directories.'); message.writeln(
return 1; 'For packages in the flutter repository, try using '
'"flutter update-packages --upgrade" to do all of them at once.');
}
message.write(
'If this does not help, to track down the conflict you can use '
'"pub deps --style=list" and "pub upgrade --verbosity=solver" in the affected directories.');
throwToolExit(message.toString());
} }
Map<String, String> packages = dependencies.asPackageMap(); Map<String, String> packages = dependencies.asPackageMap();
...@@ -185,11 +192,11 @@ class AnalyzeOnce extends AnalyzeBase { ...@@ -185,11 +192,11 @@ class AnalyzeOnce extends AnalyzeBase {
writeBenchmark(stopwatch, errorCount, membersMissingDocumentation); writeBenchmark(stopwatch, errorCount, membersMissingDocumentation);
if (errorCount > 0) { if (errorCount > 0) {
// we consider any level of error to be an error exit (we don't report different levels)
if (membersMissingDocumentation > 0 && flutterRepo) if (membersMissingDocumentation > 0 && flutterRepo)
printError('[lint] $membersMissingDocumentation public ${ membersMissingDocumentation == 1 ? "member lacks" : "members lack" } documentation (ran in ${elapsed}s)'); throwToolExit('[lint] $membersMissingDocumentation public ${ membersMissingDocumentation == 1 ? "member lacks" : "members lack" } documentation (ran in ${elapsed}s)');
else else
print('(Ran in ${elapsed}s)'); throwToolExit('(Ran in ${elapsed}s)');
return 1; // we consider any level of error to be an error exit (we don't report different levels)
} }
if (argResults['congratulate']) { if (argResults['congratulate']) {
if (membersMissingDocumentation > 0 && flutterRepo) { if (membersMissingDocumentation > 0 && flutterRepo) {
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
import 'dart:async'; import 'dart:async';
import '../base/common.dart';
import '../base/process.dart'; import '../base/process.dart';
import '../cache.dart'; import '../cache.dart';
import '../globals.dart'; import '../globals.dart';
...@@ -20,16 +21,14 @@ class ChannelCommand extends FlutterCommand { ...@@ -20,16 +21,14 @@ class ChannelCommand extends FlutterCommand {
String get invocation => '${runner.executableName} $name [<channel-name>]'; String get invocation => '${runner.executableName} $name [<channel-name>]';
@override @override
Future<int> runCommand() async { Future<int> runCommand() {
switch (argResults.rest.length) { switch (argResults.rest.length) {
case 0: case 0:
return await _listChannels(); return _listChannels();
case 1: case 1:
return await _switchChannel(argResults.rest[0]); return _switchChannel(argResults.rest[0]);
default: default:
printStatus('Too many arguments.'); throw new ToolExit('Too many arguments.\n$usage');
printStatus(usage);
return 2;
} }
} }
...@@ -39,7 +38,7 @@ class ChannelCommand extends FlutterCommand { ...@@ -39,7 +38,7 @@ class ChannelCommand extends FlutterCommand {
workingDirectory: Cache.flutterRoot); workingDirectory: Cache.flutterRoot);
printStatus('Flutter channels:'); printStatus('Flutter channels:');
return runCommandAndStreamOutput( int result = await runCommandAndStreamOutput(
<String>['git', 'branch', '-r'], <String>['git', 'branch', '-r'],
workingDirectory: Cache.flutterRoot, workingDirectory: Cache.flutterRoot,
mapFunction: (String line) { mapFunction: (String line) {
...@@ -51,13 +50,19 @@ class ChannelCommand extends FlutterCommand { ...@@ -51,13 +50,19 @@ class ChannelCommand extends FlutterCommand {
return ' $branchName'; return ' $branchName';
}, },
); );
if (result != 0)
throwToolExit('List channels failed: $result', exitCode: result);
return 0;
} }
Future<int> _switchChannel(String branchName) { Future<int> _switchChannel(String branchName) async {
printStatus('Switching to flutter channel named $branchName'); printStatus('Switching to flutter channel named $branchName');
return runCommandAndStreamOutput( int result = await runCommandAndStreamOutput(
<String>['git', 'checkout', branchName], <String>['git', 'checkout', branchName],
workingDirectory: Cache.flutterRoot, workingDirectory: Cache.flutterRoot,
); );
if (result != 0)
throwToolExit('Switch channel failed: $result', exitCode: result);
return 0;
} }
} }
...@@ -8,6 +8,7 @@ import 'dart:io'; ...@@ -8,6 +8,7 @@ import 'dart:io';
import 'package:path/path.dart' as path; import 'package:path/path.dart' as path;
import '../android/android.dart' as android; import '../android/android.dart' as android;
import '../base/common.dart';
import '../base/utils.dart'; import '../base/utils.dart';
import '../cache.dart'; import '../cache.dart';
import '../dart/pub.dart'; import '../dart/pub.dart';
...@@ -46,28 +47,23 @@ class CreateCommand extends FlutterCommand { ...@@ -46,28 +47,23 @@ class CreateCommand extends FlutterCommand {
@override @override
Future<int> runCommand() async { Future<int> runCommand() async {
if (argResults.rest.isEmpty) { if (argResults.rest.isEmpty)
printError('No option specified for the output directory.'); throwToolExit('No option specified for the output directory.\n$usage', exitCode: 2);
printError(usage);
return 2;
}
if (argResults.rest.length > 1) { if (argResults.rest.length > 1) {
printError('Multiple output directories specified.'); String message = 'Multiple output directories specified.';
for (String arg in argResults.rest) { for (String arg in argResults.rest) {
if (arg.startsWith('-')) { if (arg.startsWith('-')) {
printError('Try moving $arg to be immediately following $name'); message += '\nTry moving $arg to be immediately following $name';
break; break;
} }
} }
return 2; throwToolExit(message, exitCode: 2);
} }
if (Cache.flutterRoot == null) { if (Cache.flutterRoot == null)
printError('Neither the --flutter-root command line flag nor the FLUTTER_ROOT environment\n' throwToolExit('Neither the --flutter-root command line flag nor the FLUTTER_ROOT environment\n'
'variable was specified. Unable to find package:flutter.'); 'variable was specified. Unable to find package:flutter.', exitCode: 2);
return 2;
}
await Cache.instance.updateAll(); await Cache.instance.updateAll();
...@@ -75,16 +71,12 @@ class CreateCommand extends FlutterCommand { ...@@ -75,16 +71,12 @@ class CreateCommand extends FlutterCommand {
String flutterPackagesDirectory = path.join(flutterRoot, 'packages'); String flutterPackagesDirectory = path.join(flutterRoot, 'packages');
String flutterPackagePath = path.join(flutterPackagesDirectory, 'flutter'); String flutterPackagePath = path.join(flutterPackagesDirectory, 'flutter');
if (!FileSystemEntity.isFileSync(path.join(flutterPackagePath, 'pubspec.yaml'))) { if (!FileSystemEntity.isFileSync(path.join(flutterPackagePath, 'pubspec.yaml')))
printError('Unable to find package:flutter in $flutterPackagePath'); throwToolExit('Unable to find package:flutter in $flutterPackagePath', exitCode: 2);
return 2;
}
String flutterDriverPackagePath = path.join(flutterRoot, 'packages', 'flutter_driver'); String flutterDriverPackagePath = path.join(flutterRoot, 'packages', 'flutter_driver');
if (!FileSystemEntity.isFileSync(path.join(flutterDriverPackagePath, 'pubspec.yaml'))) { if (!FileSystemEntity.isFileSync(path.join(flutterDriverPackagePath, 'pubspec.yaml')))
printError('Unable to find package:flutter_driver in $flutterDriverPackagePath'); throwToolExit('Unable to find package:flutter_driver in $flutterDriverPackagePath', exitCode: 2);
return 2;
}
Directory projectDir = new Directory(argResults.rest.first); Directory projectDir = new Directory(argResults.rest.first);
String dirPath = path.normalize(projectDir.absolute.path); String dirPath = path.normalize(projectDir.absolute.path);
...@@ -92,16 +84,12 @@ class CreateCommand extends FlutterCommand { ...@@ -92,16 +84,12 @@ class CreateCommand extends FlutterCommand {
String projectName = _normalizeProjectName(path.basename(dirPath)); String projectName = _normalizeProjectName(path.basename(dirPath));
String error =_validateProjectDir(dirPath, flutterRoot: flutterRoot); String error =_validateProjectDir(dirPath, flutterRoot: flutterRoot);
if (error != null) { if (error != null)
printError(error); throwToolExit(error);
return 1;
}
error = _validateProjectName(projectName); error = _validateProjectName(projectName);
if (error != null) { if (error != null)
printError(error); throwToolExit(error);
return 1;
}
int generatedCount = _renderTemplates( int generatedCount = _renderTemplates(
projectName, projectName,
...@@ -114,11 +102,8 @@ class CreateCommand extends FlutterCommand { ...@@ -114,11 +102,8 @@ class CreateCommand extends FlutterCommand {
printStatus(''); printStatus('');
if (argResults['pub']) { if (argResults['pub'])
int code = await pubGet(directory: dirPath); await pubGet(directory: dirPath);
if (code != 0)
return code;
}
printStatus(''); printStatus('');
...@@ -149,7 +134,6 @@ Your main program file is lib/main.dart in the $relativePath directory. ...@@ -149,7 +134,6 @@ Your main program file is lib/main.dart in the $relativePath directory.
"directory in order to launch your app."); "directory in order to launch your app.");
printStatus("Your main program file is: $relativePath/lib/main.dart"); printStatus("Your main program file is: $relativePath/lib/main.dart");
} }
return 0; return 0;
} }
......
...@@ -7,6 +7,7 @@ import 'dart:convert'; ...@@ -7,6 +7,7 @@ import 'dart:convert';
import 'dart:io'; import 'dart:io';
import '../android/android_device.dart'; import '../android/android_device.dart';
import '../base/common.dart';
import '../base/context.dart'; import '../base/context.dart';
import '../base/logger.dart'; import '../base/logger.dart';
import '../base/utils.dart'; import '../base/utils.dart';
...@@ -51,12 +52,15 @@ class DaemonCommand extends FlutterCommand { ...@@ -51,12 +52,15 @@ class DaemonCommand extends FlutterCommand {
Cache.releaseLockEarly(); Cache.releaseLockEarly();
return appContext.runInZone(() { return appContext.runInZone(() async {
Daemon daemon = new Daemon( Daemon daemon = new Daemon(
stdinCommandStream, stdoutCommandResponse, stdinCommandStream, stdoutCommandResponse,
daemonCommand: this, notifyingLogger: notifyingLogger); daemonCommand: this, notifyingLogger: notifyingLogger);
return daemon.onExit; int code = await daemon.onExit;
if (code != null)
throwToolExit(null, exitCode: code);
return 0;
}, onError: _handleError); }, onError: _handleError);
} }
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
import 'dart:async'; import 'dart:async';
import '../base/common.dart';
import '../base/utils.dart'; import '../base/utils.dart';
import '../device.dart'; import '../device.dart';
import '../globals.dart'; import '../globals.dart';
...@@ -19,9 +20,10 @@ class DevicesCommand extends FlutterCommand { ...@@ -19,9 +20,10 @@ class DevicesCommand extends FlutterCommand {
@override @override
Future<int> runCommand() async { Future<int> runCommand() async {
if (!doctor.canListAnything) { if (!doctor.canListAnything) {
printError("Unable to locate a development device; please run 'flutter doctor' for " throwToolExit(
"information about installing additional components."); "Unable to locate a development device; please run 'flutter doctor' for "
return 1; "information about installing additional components.",
exitCode: 1);
} }
List<Device> devices = await deviceManager.getAllConnectedDevices(); List<Device> devices = await deviceManager.getAllConnectedDevices();
...@@ -35,7 +37,6 @@ class DevicesCommand extends FlutterCommand { ...@@ -35,7 +37,6 @@ class DevicesCommand extends FlutterCommand {
printStatus('${devices.length} connected ${pluralize('device', devices.length)}:\n'); printStatus('${devices.length} connected ${pluralize('device', devices.length)}:\n');
Device.printDevices(devices); Device.printDevices(devices);
} }
return 0; return 0;
} }
} }
...@@ -7,6 +7,7 @@ import 'dart:convert'; ...@@ -7,6 +7,7 @@ import 'dart:convert';
import 'dart:io'; import 'dart:io';
import 'package:args/command_runner.dart'; import 'package:args/command_runner.dart';
import 'package:flutter_tools/src/base/common.dart';
import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/cache.dart';
import 'package:flutter_tools/src/commands/create.dart'; import 'package:flutter_tools/src/commands/create.dart';
import 'package:flutter_tools/src/dart/sdk.dart'; import 'package:flutter_tools/src/dart/sdk.dart';
...@@ -89,9 +90,13 @@ void main() { ...@@ -89,9 +90,13 @@ void main() {
CreateCommand command = new CreateCommand(); CreateCommand command = new CreateCommand();
CommandRunner runner = createTestCommandRunner(command); CommandRunner runner = createTestCommandRunner(command);
int code = await runner.run(<String>['create', temp.path, '--pub']); try {
expect(code, 2); await runner.run(<String>['create', temp.path, '--pub']);
expect(testLogger.errorText, contains('Try moving --pub')); fail('expected ToolExit exception');
} on ToolExit catch (e) {
expect(e.exitCode, 2);
expect(e.message, contains('Try moving --pub'));
}
}); });
// Verify that we fail with an error code when the file exists. // Verify that we fail with an error code when the file exists.
...@@ -101,8 +106,12 @@ void main() { ...@@ -101,8 +106,12 @@ void main() {
CommandRunner runner = createTestCommandRunner(command); CommandRunner runner = createTestCommandRunner(command);
File existingFile = new File("${temp.path.toString()}/bad"); File existingFile = new File("${temp.path.toString()}/bad");
if (!existingFile.existsSync()) existingFile.createSync(); if (!existingFile.existsSync()) existingFile.createSync();
int code = await runner.run(<String>['create', existingFile.path]); try {
expect(code, 1); await runner.run(<String>['create', existingFile.path]);
fail('expected ToolExit exception');
} on ToolExit catch (e) {
expect(e.message, contains('file exists'));
}
}); });
}); });
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment