Add custom exceptions with more information (#63)
diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c9c001..d469337 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md
@@ -1,3 +1,9 @@ +#### 4.2.1 + +* Added custom exception types `ProcessPackageException` and + `ProcessPackageExecutableNotFoundException` to provide extra + information from exception conditions. + #### 4.2.0 * Fix the signature of `ProcessManager.canRun` to be consistent with
diff --git a/lib/process.dart b/lib/process.dart index dfeb1d0..af513a0 100644 --- a/lib/process.dart +++ b/lib/process.dart
@@ -2,6 +2,7 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +export 'src/interface/exceptions.dart'; export 'src/interface/local_process_manager.dart'; export 'src/interface/process_manager.dart'; export 'src/interface/process_wrapper.dart';
diff --git a/lib/src/interface/common.dart b/lib/src/interface/common.dart index e0866ad..25d52f5 100644 --- a/lib/src/interface/common.dart +++ b/lib/src/interface/common.dart
@@ -7,6 +7,8 @@ import 'package:path/path.dart' show Context; import 'package:platform/platform.dart'; +import 'exceptions.dart'; + const Map<String, String> _osToPathStyle = <String, String>{ 'linux': 'posix', 'macos': 'posix', @@ -34,10 +36,10 @@ return executable; } -/// Searches the `PATH` for the executable that [command] is supposed to launch. +/// Searches the `PATH` for the executable that [executable] is supposed to launch. /// /// This first builds a list of candidate paths where the executable may reside. -/// If [command] is already an absolute path, then the `PATH` environment +/// If [executable] is already an absolute path, then the `PATH` environment /// variable will not be consulted, and the specified absolute path will be the /// only candidate that is considered. /// @@ -49,11 +51,11 @@ /// /// If [platform] is not specified, it will default to the current platform. String? getExecutablePath( - String command, + String executable, String? workingDirectory, { Platform platform = const LocalPlatform(), FileSystem fs = const LocalFileSystem(), - bool errorOnNull = false, + bool throwOnFailure = false, }) { assert(_osToPathStyle[platform.operatingSystem] == fs.path.style.name); try { @@ -71,19 +73,20 @@ String pathSeparator = platform.isWindows ? ';' : ':'; List<String> extensions = <String>[]; - if (platform.isWindows && context.extension(command).isEmpty) { + if (platform.isWindows && context.extension(executable).isEmpty) { extensions = platform.environment['PATHEXT']!.split(pathSeparator); } List<String> candidates = <String>[]; - if (command.contains(context.separator)) { - candidates = _getCandidatePaths( - command, <String>[workingDirectory], extensions, context); + List<String> searchPath; + if (executable.contains(context.separator)) { + // Deal with commands that specify a relative or absolute path differently. + searchPath = <String>[workingDirectory]; } else { - List<String> searchPath = - platform.environment['PATH']!.split(pathSeparator); - candidates = _getCandidatePaths(command, searchPath, extensions, context); + searchPath = platform.environment['PATH']!.split(pathSeparator); } + candidates = _getCandidatePaths(executable, searchPath, extensions, context); + final List<String> foundCandidates = <String>[]; for (String path in candidates) { final File candidate = fs.file(path); FileStat stat = candidate.statSync(); @@ -93,6 +96,9 @@ continue; } + // File exists, but we don't know if it's readable/executable yet. + foundCandidates.add(candidate.path); + const int isExecutable = 0x40; const int isReadable = 0x100; const int isExecutableAndReadable = isExecutable | isReadable; @@ -108,10 +114,24 @@ return path; } } - if (errorOnNull) { - throw ArgumentError('Failed to resolve $command to an executable.\n' - 'workingDirectory: $workingDirectory, ' - 'candidates: ${candidates.length}'); + if (throwOnFailure) { + if (foundCandidates.isNotEmpty) { + throw ProcessPackageExecutableNotFoundException( + executable, + message: + 'Found candidates, but lacked sufficient permissions to execute "$executable".', + workingDirectory: workingDirectory, + candidates: foundCandidates, + searchPath: searchPath, + ); + } else { + throw ProcessPackageExecutableNotFoundException( + executable, + message: 'Failed to find "$executable" in the search path.', + workingDirectory: workingDirectory, + searchPath: searchPath, + ); + } } return null; }
diff --git a/lib/src/interface/exceptions.dart b/lib/src/interface/exceptions.dart new file mode 100644 index 0000000..ee407ca --- /dev/null +++ b/lib/src/interface/exceptions.dart
@@ -0,0 +1,106 @@ +// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:io' show ProcessException; + +/// A specialized exception class for this package, so that it can throw +/// customized exceptions with more information. +class ProcessPackageException extends ProcessException { + /// Create a const ProcessPackageException. + /// + /// The [executable] should be the name of the executable to be run. + /// + /// The optional [workingDirectory] is the directory where the command + /// execution is attempted. + /// + /// The optional [arguments] is a list of the arguments to given to the + /// executable, already separated. + /// + /// The optional [message] is an additional message to be included in the + /// exception string when printed. + /// + /// The optional [errorCode] is the error code received when the executable + /// was run. Zero means it ran successfully, or that no error code was + /// available. + /// + /// See [ProcessException] for more information. + const ProcessPackageException( + String executable, { + List<String> arguments = const <String>[], + String message = "", + int errorCode = 0, + this.workingDirectory, + }) : super(executable, arguments, message, errorCode); + + /// Creates a [ProcessPackageException] from a [ProcessException]. + factory ProcessPackageException.fromProcessException( + ProcessException exception, { + String? workingDirectory, + }) { + return ProcessPackageException( + exception.executable, + arguments: exception.arguments, + message: exception.message, + errorCode: exception.errorCode, + workingDirectory: workingDirectory, + ); + } + + /// The optional working directory that the command was being executed in. + final String? workingDirectory; + + // Don't implement a toString() for this exception, since code may be + // depending upon the format of ProcessException.toString(). +} + +/// An exception for when an executable is not found that was expected to be found. +class ProcessPackageExecutableNotFoundException + extends ProcessPackageException { + /// Creates a const ProcessPackageExecutableNotFoundException + /// + /// The optional [candidates] are the files matching the expected executable + /// on the [searchPath]. + /// + /// The optional [searchPath] is the list of directories searched for the + /// expected executable. + /// + /// See [ProcessPackageException] for more information. + const ProcessPackageExecutableNotFoundException( + String executable, { + List<String> arguments = const <String>[], + String message = "", + int errorCode = 0, + String? workingDirectory, + this.candidates = const <String>[], + this.searchPath = const <String>[], + }) : super( + executable, + arguments: arguments, + message: message, + errorCode: errorCode, + workingDirectory: workingDirectory, + ); + + /// The list of non-viable executable candidates found. + final List<String> candidates; + + /// The search path used to find candidates. + final List<String> searchPath; + + @override + String toString() { + StringBuffer buffer = StringBuffer('$runtimeType: $message\n'); + // Don't add an extra space if there are no arguments. + final String args = arguments.isNotEmpty ? ' ${arguments.join(' ')}' : ''; + buffer.writeln(' Command: $executable$args'); + if (workingDirectory != null && workingDirectory!.isNotEmpty) { + buffer.writeln(' Working Directory: $workingDirectory'); + } + if (candidates.isNotEmpty) { + buffer.writeln(' Candidates:\n ${candidates.join('\n ')}'); + } + buffer.writeln(' Search Path:\n ${searchPath.join('\n ')}'); + return buffer.toString(); + } +}
diff --git a/lib/src/interface/local_process_manager.dart b/lib/src/interface/local_process_manager.dart index fe5041c..72e513a 100644 --- a/lib/src/interface/local_process_manager.dart +++ b/lib/src/interface/local_process_manager.dart
@@ -9,9 +9,13 @@ ProcessResult, ProcessSignal, ProcessStartMode, + ProcessException, systemEncoding; +import 'package:process/process.dart'; + import 'common.dart'; +import 'exceptions.dart'; import 'process_manager.dart'; /// Local implementation of the `ProcessManager` interface. @@ -36,19 +40,24 @@ bool runInShell = false, ProcessStartMode mode = ProcessStartMode.normal, }) { - return Process.start( - sanitizeExecutablePath(_getExecutable( - command, - workingDirectory, - runInShell, - )), - _getArguments(command), - workingDirectory: workingDirectory, - environment: environment, - includeParentEnvironment: includeParentEnvironment, - runInShell: runInShell, - mode: mode, - ); + try { + return Process.start( + sanitizeExecutablePath(_getExecutable( + command, + workingDirectory, + runInShell, + )), + _getArguments(command), + workingDirectory: workingDirectory, + environment: environment, + includeParentEnvironment: includeParentEnvironment, + runInShell: runInShell, + mode: mode, + ); + } on ProcessException catch (exception) { + throw ProcessPackageException.fromProcessException(exception, + workingDirectory: workingDirectory); + } } @override @@ -61,20 +70,25 @@ Encoding stdoutEncoding = systemEncoding, Encoding stderrEncoding = systemEncoding, }) { - return Process.run( - sanitizeExecutablePath(_getExecutable( - command, - workingDirectory, - runInShell, - )), - _getArguments(command), - workingDirectory: workingDirectory, - environment: environment, - includeParentEnvironment: includeParentEnvironment, - runInShell: runInShell, - stdoutEncoding: stdoutEncoding, - stderrEncoding: stderrEncoding, - ); + try { + return Process.run( + sanitizeExecutablePath(_getExecutable( + command, + workingDirectory, + runInShell, + )), + _getArguments(command), + workingDirectory: workingDirectory, + environment: environment, + includeParentEnvironment: includeParentEnvironment, + runInShell: runInShell, + stdoutEncoding: stdoutEncoding, + stderrEncoding: stderrEncoding, + ); + } on ProcessException catch (exception) { + throw ProcessPackageException.fromProcessException(exception, + workingDirectory: workingDirectory); + } } @override @@ -87,25 +101,30 @@ Encoding stdoutEncoding = systemEncoding, Encoding stderrEncoding = systemEncoding, }) { - return Process.runSync( - sanitizeExecutablePath(_getExecutable( - command, - workingDirectory, - runInShell, - )), - _getArguments(command), - workingDirectory: workingDirectory, - environment: environment, - includeParentEnvironment: includeParentEnvironment, - runInShell: runInShell, - stdoutEncoding: stdoutEncoding, - stderrEncoding: stderrEncoding, - ); + try { + return Process.runSync( + sanitizeExecutablePath(_getExecutable( + command, + workingDirectory, + runInShell, + )), + _getArguments(command), + workingDirectory: workingDirectory, + environment: environment, + includeParentEnvironment: includeParentEnvironment, + runInShell: runInShell, + stdoutEncoding: stdoutEncoding, + stderrEncoding: stderrEncoding, + ); + } on ProcessException catch (exception) { + throw ProcessPackageException.fromProcessException(exception, + workingDirectory: workingDirectory); + } } @override bool canRun(covariant String executable, {String? workingDirectory}) => - getExecutablePath(executable, workingDirectory, errorOnNull: false) != + getExecutablePath(executable, workingDirectory, throwOnFailure: false) != null; @override @@ -120,15 +139,14 @@ if (runInShell) { return commandName; } - String? exe = - getExecutablePath(commandName, workingDirectory, errorOnNull: true); - if (exe == null) { - throw ArgumentError('Cannot find executable for $commandName.'); - } - return exe; + return getExecutablePath( + commandName, + workingDirectory, + throwOnFailure: true, + )!; } -List<String> _getArguments(List<dynamic> command) => +List<String> _getArguments(List<Object> command) => // Adding a specific type to map in order to workaround dart issue // https://github.com/dart-lang/sdk/issues/32414 command
diff --git a/pubspec.yaml b/pubspec.yaml index f98baa6..685a754 100644 --- a/pubspec.yaml +++ b/pubspec.yaml
@@ -1,5 +1,5 @@ name: process -version: 4.2.0 +version: 4.2.1 description: A pluggable, mockable process invocation abstraction for Dart. homepage: https://github.com/google/process.dart
diff --git a/test/src/interface/common_test.dart b/test/src/interface/common_test.dart index f394a81..ae605db 100644 --- a/test/src/interface/common_test.dart +++ b/test/src/interface/common_test.dart
@@ -4,10 +4,10 @@ import 'dart:io' as io; import 'package:file/local.dart'; -import 'package:path/path.dart' as path; import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:platform/platform.dart'; +import 'package:process/process.dart'; import 'package:process/src/interface/common.dart'; import 'package:test/test.dart'; @@ -195,27 +195,34 @@ expect(executablePath, isNull); }); - test('not found with errorOnNull throws exception with match state', () { + test('not found with throwOnFailure throws exception with match state', + () { String command = 'foo.exe'; - dynamic error; + io.ProcessException error; try { getExecutablePath( command, workingDir.path, platform: platform, fs: fs, - errorOnNull: true, + throwOnFailure: true, ); fail('Expected to throw'); - } catch (err) { + } on io.ProcessException catch (err) { error = err; } - expect(error, isA<ArgumentError>()); + expect(error, isA<ProcessPackageExecutableNotFoundException>()); + ProcessPackageExecutableNotFoundException notFoundException = + error as ProcessPackageExecutableNotFoundException; + expect(notFoundException.candidates, isEmpty); + expect(notFoundException.workingDirectory, equals(workingDir.path)); expect( error.toString(), - contains( - 'workingDirectory: C:\\.tmp_rand0\\work_dir_rand0, candidates: 2')); + contains(' Working Directory: C:\\.tmp_rand0\\work_dir_rand0\n' + ' Search Path:\n' + ' C:\\.tmp_rand0\\dir1_rand0\n' + ' C:\\.tmp_rand0\\dir2_rand0\n')); }); test('when path has spaces', () { @@ -324,27 +331,34 @@ expect(executablePath, isNull); }); - test('not found with errorOnNull throws exception with match state', () { + test('not found with throwOnFailure throws exception with match state', + () { String command = 'foo'; - dynamic error; + io.ProcessException error; try { getExecutablePath( command, workingDir.path, platform: platform, fs: fs, - errorOnNull: true, + throwOnFailure: true, ); fail('Expected to throw'); - } catch (err) { + } on io.ProcessException catch (err) { error = err; } - expect(error, isA<ArgumentError>()); + expect(error, isA<ProcessPackageExecutableNotFoundException>()); + ProcessPackageExecutableNotFoundException notFoundException = + error as ProcessPackageExecutableNotFoundException; + expect(notFoundException.candidates, isEmpty); + expect(notFoundException.workingDirectory, equals(workingDir.path)); expect( error.toString(), - contains( - 'workingDirectory: /.tmp_rand0/work_dir_rand0, candidates: 2')); + contains(' Working Directory: /.tmp_rand0/work_dir_rand0\n' + ' Search Path:\n' + ' /.tmp_rand0/dir1_rand0\n' + ' /.tmp_rand0/dir2_rand0\n')); }); test('when path has spaces', () { @@ -356,6 +370,9 @@ }); }); group('Real Filesystem', () { + // These tests don't use the memory filesystem because Dart can't modify file + // executable permissions, so we have to create them with actual commands. + late Platform platform; late Directory tmpDir; late Directory pathDir1; @@ -402,8 +419,6 @@ tmpDir.deleteSync(recursive: true); }); - // This doesn't use the memory filesystem because Dart can't modify file - // executable permissions, so we have to create them with actual commands. test('Only returns executables in PATH', () { if (localPlatform.isWindows) { // Windows doesn't check for executable-ness, and we can't run 'chmod' @@ -435,6 +450,114 @@ // user. _expectSamePath(executablePath, command5.absolute.path); }); + + test( + 'Test that finding non-executable paths throws with proper information', + () { + if (localPlatform.isWindows) { + // Windows doesn't check for executable-ness, and we can't run 'chmod' + // on Windows anyhow. + return; + } + + // Make the second command in the path executable, but not the first. + // No executable permissions + io.Process.runSync("chmod", <String>["0644", "--", command1.path]); + // Only group executable permissions + io.Process.runSync("chmod", <String>["0645", "--", command2.path]); + // Only other executable permissions + io.Process.runSync("chmod", <String>["0654", "--", command3.path]); + // All executable permissions, but not readable + io.Process.runSync("chmod", <String>["0311", "--", command4.path]); + + io.ProcessException error; + try { + getExecutablePath( + 'command', + tmpDir.path, + platform: platform, + fs: fs, + throwOnFailure: true, + ); + fail('Expected to throw'); + } on io.ProcessException catch (err) { + error = err; + } + + expect(error, isA<ProcessPackageExecutableNotFoundException>()); + ProcessPackageExecutableNotFoundException notFoundException = + error as ProcessPackageExecutableNotFoundException; + expect( + notFoundException.candidates, + equals(<String>[ + '${tmpDir.path}/path1/command', + '${tmpDir.path}/path2/command', + '${tmpDir.path}/path3/command', + '${tmpDir.path}/path4/command', + '${tmpDir.path}/path5/command', + ])); + expect( + error.toString(), + equals( + 'ProcessPackageExecutableNotFoundException: Found candidates, but lacked sufficient permissions to execute "command".\n' + ' Command: command\n' + ' Working Directory: ${tmpDir.path}\n' + ' Candidates:\n' + ' ${tmpDir.path}/path1/command\n' + ' ${tmpDir.path}/path2/command\n' + ' ${tmpDir.path}/path3/command\n' + ' ${tmpDir.path}/path4/command\n' + ' ${tmpDir.path}/path5/command\n' + ' Search Path:\n' + ' ${tmpDir.path}/path1\n' + ' ${tmpDir.path}/path2\n' + ' ${tmpDir.path}/path3\n' + ' ${tmpDir.path}/path4\n' + ' ${tmpDir.path}/path5\n', + ), + ); + }); + + test('Test that finding no executable paths throws with proper information', + () { + if (localPlatform.isWindows) { + // Windows doesn't check for executable-ness, and we can't run 'chmod' + // on Windows anyhow. + return; + } + + io.ProcessException error; + try { + getExecutablePath( + 'non-existent-command', + tmpDir.path, + platform: platform, + fs: fs, + throwOnFailure: true, + ); + fail('Expected to throw'); + } on io.ProcessException catch (err) { + error = err; + } + + expect(error, isA<ProcessPackageExecutableNotFoundException>()); + ProcessPackageExecutableNotFoundException notFoundException = + error as ProcessPackageExecutableNotFoundException; + expect(notFoundException.candidates, isEmpty); + expect( + error.toString(), + equals( + 'ProcessPackageExecutableNotFoundException: Failed to find "non-existent-command" in the search path.\n' + ' Command: non-existent-command\n' + ' Working Directory: ${tmpDir.path}\n' + ' Search Path:\n' + ' ${tmpDir.path}/path1\n' + ' ${tmpDir.path}/path2\n' + ' ${tmpDir.path}/path3\n' + ' ${tmpDir.path}/path4\n' + ' ${tmpDir.path}/path5\n'), + ); + }); }); }