Unverified Commit 549de844 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter_tools] add a mechanism to turn off immediate tool exit (#66787)

Instead of always exiting the tool, provide a mechanism to turn off this behavior for non-critical functionality like configuration and analytics settings.

Fixes #66786 Fixes #4674
parent 5e651b96
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
import '../convert.dart'; import '../convert.dart';
import 'error_handling_io.dart';
import 'file_system.dart'; import 'file_system.dart';
import 'logger.dart'; import 'logger.dart';
import 'platform.dart'; import 'platform.dart';
...@@ -25,7 +26,7 @@ class Config { ...@@ -25,7 +26,7 @@ class Config {
_userHomePath(platform), _userHomePath(platform),
name, name,
)); ));
return Config._(file, logger); return Config.createForTesting(file, logger);
} }
/// Constructs a new [Config] object from a file called [name] in /// Constructs a new [Config] object from a file called [name] in
...@@ -34,14 +35,18 @@ class Config { ...@@ -34,14 +35,18 @@ class Config {
String name, { String name, {
@required Directory directory, @required Directory directory,
@required Logger logger, @required Logger logger,
}) => Config._(directory.childFile(name), logger); }) => Config.createForTesting(directory.childFile(name), logger);
Config._(File file, Logger logger) : _file = file, _logger = logger { /// Test only access to the Config constructor.
@visibleForTesting
Config.createForTesting(File file, Logger logger) : _file = file, _logger = logger {
if (!_file.existsSync()) { if (!_file.existsSync()) {
return; return;
} }
try { try {
_values = castStringKeyedMap(json.decode(_file.readAsStringSync())); ErrorHandlingFileSystem.noExitOnFailure(() {
_values = castStringKeyedMap(json.decode(_file.readAsStringSync()));
});
} on FormatException { } on FormatException {
_logger _logger
..printError('Failed to decode preferences in ${_file.path}.') ..printError('Failed to decode preferences in ${_file.path}.')
...@@ -50,6 +55,13 @@ class Config { ...@@ -50,6 +55,13 @@ class Config {
'with the "flutter config" command.', 'with the "flutter config" command.',
); );
_file.deleteSync(); _file.deleteSync();
} on Exception catch (err) {
_logger
..printError('Could not read preferences in ${file.path}.\n$err')
..printError(
'You may need to resolve the error above and reapply any previously '
'saved configuration with the "flutter config" command.',
);
} }
} }
......
...@@ -45,6 +45,26 @@ class ErrorHandlingFileSystem extends ForwardingFileSystem { ...@@ -45,6 +45,26 @@ class ErrorHandlingFileSystem extends ForwardingFileSystem {
final Platform _platform; final Platform _platform;
/// Allow any file system operations executed within the closure to fail with any
/// operating system error, rethrowing an [Exception] instead of a [ToolExit].
///
/// This should not be used with async file system operation.
///
/// This can be used to bypass the [ErrorHandlingFileSystem] permission exit
/// checks for situations where failure is acceptable, such as the flutter
/// persistent settings cache.
static void noExitOnFailure(void Function() operation) {
final bool previousValue = ErrorHandlingFileSystem._noExitOnFailure;
try {
ErrorHandlingFileSystem._noExitOnFailure = true;
operation();
} finally {
ErrorHandlingFileSystem._noExitOnFailure = previousValue;
}
}
static bool _noExitOnFailure = false;
@override @override
Directory get currentDirectory => directory(delegate.currentDirectory); Directory get currentDirectory => directory(delegate.currentDirectory);
...@@ -147,6 +167,15 @@ class ErrorHandlingFile ...@@ -147,6 +167,15 @@ class ErrorHandlingFile
); );
} }
@override
String readAsStringSync({Encoding encoding = utf8}) {
return _runSync<String>(
() => delegate.readAsStringSync(),
platform: _platform,
failureMessage: 'Flutter failed to read a file at "${delegate.path}"',
);
}
@override @override
void writeAsBytesSync( void writeAsBytesSync(
List<int> bytes, { List<int> bytes, {
...@@ -542,26 +571,26 @@ void _handlePosixException(Exception e, String message, int errorCode) { ...@@ -542,26 +571,26 @@ void _handlePosixException(Exception e, String message, int errorCode) {
const int enospc = 28; const int enospc = 28;
const int eacces = 13; const int eacces = 13;
// Catch errors and bail when: // Catch errors and bail when:
String errorMessage;
switch (errorCode) { switch (errorCode) {
case enospc: case enospc:
throwToolExit( errorMessage =
'$message. The target device is full.' '$message. The target device is full.'
'\n$e\n' '\n$e\n'
'Free up space and try again.', 'Free up space and try again.';
);
break; break;
case eperm: case eperm:
case eacces: case eacces:
throwToolExit( errorMessage =
'$message. The flutter tool cannot access the file or directory.\n' '$message. The flutter tool cannot access the file or directory.\n'
'Please ensure that the SDK and/or project is installed in a location ' 'Please ensure that the SDK and/or project is installed in a location '
'that has read/write permissions for the current user.' 'that has read/write permissions for the current user.';
);
break; break;
default: default:
// Caller must rethrow the exception. // Caller must rethrow the exception.
break; break;
} }
_throwFileSystemException(errorMessage);
} }
void _handleWindowsException(Exception e, String message, int errorCode) { void _handleWindowsException(Exception e, String message, int errorCode) {
...@@ -571,31 +600,40 @@ void _handleWindowsException(Exception e, String message, int errorCode) { ...@@ -571,31 +600,40 @@ void _handleWindowsException(Exception e, String message, int errorCode) {
const int kUserMappedSectionOpened = 1224; const int kUserMappedSectionOpened = 1224;
const int kAccessDenied = 5; const int kAccessDenied = 5;
// Catch errors and bail when: // Catch errors and bail when:
String errorMessage;
switch (errorCode) { switch (errorCode) {
case kAccessDenied: case kAccessDenied:
throwToolExit( errorMessage =
'$message. The flutter tool cannot access the file.\n' '$message. The flutter tool cannot access the file.\n'
'Please ensure that the SDK and/or project is installed in a location ' 'Please ensure that the SDK and/or project is installed in a location '
'that has read/write permissions for the current user.' 'that has read/write permissions for the current user.';
);
break; break;
case kDeviceFull: case kDeviceFull:
throwToolExit( errorMessage =
'$message. The target device is full.' '$message. The target device is full.'
'\n$e\n' '\n$e\n'
'Free up space and try again.', 'Free up space and try again.';
);
break; break;
case kUserMappedSectionOpened: case kUserMappedSectionOpened:
throwToolExit( errorMessage =
'$message. The file is being used by another program.' '$message. The file is being used by another program.'
'\n$e\n' '\n$e\n'
'Do you have an antivirus program running? ' 'Do you have an antivirus program running? '
'Try disabling your antivirus program and try again.', 'Try disabling your antivirus program and try again.';
);
break; break;
default: default:
// Caller must rethrow the exception. // Caller must rethrow the exception.
break; break;
} }
_throwFileSystemException(errorMessage);
}
void _throwFileSystemException(String errorMessage) {
if (errorMessage == null) {
return;
}
if (ErrorHandlingFileSystem._noExitOnFailure) {
throw Exception(errorMessage);
}
throwToolExit(errorMessage);
} }
...@@ -7,9 +7,19 @@ ...@@ -7,9 +7,19 @@
import 'dart:convert' hide utf8; import 'dart:convert' hide utf8;
import 'dart:convert' as cnv show utf8, Utf8Decoder; import 'dart:convert' as cnv show utf8, Utf8Decoder;
import 'package:meta/meta.dart';
import 'base/common.dart'; import 'base/common.dart';
export 'dart:convert' hide utf8, Utf8Codec, Utf8Decoder; export 'dart:convert' hide utf8, Utf8Codec, Utf8Decoder;
/// The original utf8 encoding for testing overrides only.
///
/// Attempting to use the flutter tool utf8 decoder will surface an analyzer
/// warning that overrides cannot change the default value of a named
/// parameter.
@visibleForTesting
const Encoding utf8ForTesting = cnv.utf8;
/// A [Codec] which reports malformed bytes when decoding. /// A [Codec] which reports malformed bytes when decoding.
/// ///
/// Occasionally people end up in a situation where we try to decode bytes /// Occasionally people end up in a situation where we try to decode bytes
......
...@@ -12,6 +12,7 @@ import 'package:intl/intl.dart'; ...@@ -12,6 +12,7 @@ import 'package:intl/intl.dart';
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
import 'package:usage/usage_io.dart'; import 'package:usage/usage_io.dart';
import '../base/error_handling_io.dart';
import '../base/file_system.dart'; import '../base/file_system.dart';
import '../base/io.dart'; import '../base/io.dart';
import '../base/logger.dart'; import '../base/logger.dart';
......
...@@ -221,14 +221,16 @@ class _DefaultUsage implements Usage { ...@@ -221,14 +221,16 @@ class _DefaultUsage implements Usage {
_analytics = LogToFileAnalytics(logFilePath); _analytics = LogToFileAnalytics(logFilePath);
} else { } else {
try { try {
_analytics = analyticsIOFactory( ErrorHandlingFileSystem.noExitOnFailure(() {
_kFlutterUA, _analytics = analyticsIOFactory(
settingsName, _kFlutterUA,
version, settingsName,
documentDirectory: configDirOverride != null version,
? globals.fs.directory(configDirOverride) documentDirectory: configDirOverride != null
: null, ? globals.fs.directory(configDirOverride)
); : null,
);
});
} on Exception catch (e) { } on Exception catch (e) {
globals.printTrace('Failed to initialize analytics reporting: $e'); globals.printTrace('Failed to initialize analytics reporting: $e');
suppressAnalytics = true; suppressAnalytics = true;
......
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
import 'package:file/file.dart'; import 'package:file/file.dart';
import 'package:flutter_tools/src/base/common.dart';
import 'package:flutter_tools/src/base/error_handling_io.dart'; import 'package:flutter_tools/src/base/error_handling_io.dart';
import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/io.dart';
...@@ -71,6 +72,18 @@ void setupWriteMocks({ ...@@ -71,6 +72,18 @@ void setupWriteMocks({
)).thenThrow(FileSystemException('', '', OSError('', errorCode))); )).thenThrow(FileSystemException('', '', OSError('', errorCode)));
} }
void setupReadMocks({
FileSystem mockFileSystem,
ErrorHandlingFileSystem fs,
int errorCode,
}) {
final MockFile mockFile = MockFile();
when(mockFileSystem.file(any)).thenReturn(mockFile);
when(mockFile.readAsStringSync(
encoding: anyNamed('encoding'),
)).thenThrow(FileSystemException('', '', OSError('', errorCode)));
}
void setupDirectoryMocks({ void setupDirectoryMocks({
FileSystem mockFileSystem, FileSystem mockFileSystem,
ErrorHandlingFileSystem fs, ErrorHandlingFileSystem fs,
...@@ -114,6 +127,30 @@ void main() { ...@@ -114,6 +127,30 @@ void main() {
when(mockFileSystem.path).thenReturn(MockPathContext()); when(mockFileSystem.path).thenReturn(MockPathContext());
}); });
testWithoutContext('bypasses error handling when withAllowedFailure is used', () {
setupWriteMocks(
mockFileSystem: mockFileSystem,
fs: fs,
errorCode: kUserPermissionDenied,
);
final File file = fs.file('file');
expect(() => ErrorHandlingFileSystem.noExitOnFailure(
() => file.writeAsStringSync('')), throwsA(isA<Exception>()));
// nesting does not unconditionally re-enable errors.
expect(() {
ErrorHandlingFileSystem.noExitOnFailure(() {
ErrorHandlingFileSystem.noExitOnFailure(() { });
file.writeAsStringSync('');
});
}, throwsA(isA<Exception>()));
// Check that state does not leak.
expect(() => file.writeAsStringSync(''), throwsA(isA<ToolExit>()));
});
testWithoutContext('when access is denied', () async { testWithoutContext('when access is denied', () async {
setupWriteMocks( setupWriteMocks(
mockFileSystem: mockFileSystem, mockFileSystem: mockFileSystem,
...@@ -219,6 +256,20 @@ void main() { ...@@ -219,6 +256,20 @@ void main() {
expect(() => directory.existsSync(), expect(() => directory.existsSync(),
throwsToolExit(message: expectedMessage)); throwsToolExit(message: expectedMessage));
}); });
testWithoutContext('When reading from a file without permission', () {
setupReadMocks(
mockFileSystem: mockFileSystem,
fs: fs,
errorCode: kUserPermissionDenied,
);
final File file = fs.file('file');
const String expectedMessage = 'Flutter failed to read a file at';
expect(() => file.readAsStringSync(),
throwsToolExit(message: expectedMessage));
});
}); });
group('throws ToolExit on Linux', () { group('throws ToolExit on Linux', () {
...@@ -437,6 +488,20 @@ void main() { ...@@ -437,6 +488,20 @@ void main() {
expect(() => directory.existsSync(), expect(() => directory.existsSync(),
throwsToolExit(message: expectedMessage)); throwsToolExit(message: expectedMessage));
}); });
testWithoutContext('When reading from a file without permission', () {
setupReadMocks(
mockFileSystem: mockFileSystem,
fs: fs,
errorCode: eacces,
);
final File file = fs.file('file');
const String expectedMessage = 'Flutter failed to read a file at';
expect(() => file.readAsStringSync(),
throwsToolExit(message: expectedMessage));
});
}); });
testWithoutContext('Caches path context correctly', () { testWithoutContext('Caches path context correctly', () {
......
...@@ -4,10 +4,12 @@ ...@@ -4,10 +4,12 @@
import 'package:file/memory.dart'; import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/config.dart'; import 'package:flutter_tools/src/base/config.dart';
import 'package:flutter_tools/src/base/error_handling_io.dart';
import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/base/terminal.dart'; import 'package:flutter_tools/src/convert.dart';
import 'package:test/fake.dart';
import '../src/common.dart'; import '../src/common.dart';
...@@ -62,13 +64,7 @@ void main() { ...@@ -62,13 +64,7 @@ void main() {
}); });
testWithoutContext('Config parse error', () { testWithoutContext('Config parse error', () {
final BufferLogger bufferLogger = BufferLogger( final BufferLogger bufferLogger = BufferLogger.test();
terminal: AnsiTerminal(
stdio: null,
platform: const LocalPlatform(),
),
outputPreferences: OutputPreferences.test(),
);
final File file = memoryFileSystem.file('example') final File file = memoryFileSystem.file('example')
..writeAsStringSync('{"hello":"bar'); ..writeAsStringSync('{"hello":"bar');
config = Config( config = Config(
...@@ -81,4 +77,50 @@ void main() { ...@@ -81,4 +77,50 @@ void main() {
expect(file.existsSync(), false); expect(file.existsSync(), false);
expect(bufferLogger.errorText, contains('Failed to decode preferences')); expect(bufferLogger.errorText, contains('Failed to decode preferences'));
}); });
testWithoutContext('Config does not error on missing file', () {
final BufferLogger bufferLogger = BufferLogger.test();
final File file = memoryFileSystem.file('example');
config = Config(
'example',
fileSystem: memoryFileSystem,
logger: bufferLogger,
platform: fakePlatform,
);
expect(file.existsSync(), false);
expect(bufferLogger.errorText, isEmpty);
});
testWithoutContext('Config does not error on a normally fatal file system exception', () {
final BufferLogger bufferLogger = BufferLogger.test();
final File file = ErrorHandlingFile(
platform: FakePlatform(operatingSystem: 'linux'),
fileSystem: MemoryFileSystem.test(),
delegate: FakeFile('testfile'),
);
config = Config.createForTesting(file, bufferLogger);
expect(bufferLogger.errorText, contains('Could not read preferences in testfile'));
// Also contains original error message:
expect(bufferLogger.errorText, contains('The flutter tool cannot access the file or directory'));
});
}
class FakeFile extends Fake implements File {
FakeFile(this.path);
@override
final String path;
@override
bool existsSync() {
return true;
}
@override
String readAsStringSync({Encoding encoding = utf8ForTesting}) {
throw const FileSystemException('', '', OSError('', 13)); // EACCES error on linux
}
} }
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