Commit 0edc4d2a authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Make hot mode a little less aggressive about catching errors. (#8743)

It was resulting in weird situations where the tool would dump an
error message and stack but not quit, or would fail hard but then just
hang.

Instead, specifically catch errors you expect. As an example of this,
there's one error we expect from the DartDependencySetBuilder, so we
catch that one, turn it into a dedicated exception class, then in the
caller catch that specific exception.
parent 8a365886
...@@ -257,7 +257,7 @@ Future<File> _createLocalCrashReport(List<String> args, dynamic error, Chain cha ...@@ -257,7 +257,7 @@ Future<File> _createLocalCrashReport(List<String> args, dynamic error, Chain cha
buffer.writeln('flutter ${args.join(' ')}\n'); buffer.writeln('flutter ${args.join(' ')}\n');
buffer.writeln('## exception\n'); buffer.writeln('## exception\n');
buffer.writeln('$error\n'); buffer.writeln('${error.runtimeType}: $error\n');
buffer.writeln('```\n${chain.terse}```\n'); buffer.writeln('```\n${chain.terse}```\n');
buffer.writeln('## flutter doctor\n'); buffer.writeln('## flutter doctor\n');
......
...@@ -2,16 +2,16 @@ ...@@ -2,16 +2,16 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'package:analyzer/analyzer.dart'; import 'package:analyzer/analyzer.dart' as analyzer;
import '../base/file_system.dart'; import '../base/file_system.dart';
import '../dart/package_map.dart'; import '../dart/package_map.dart';
class DartDependencySetBuilder { class DartDependencySetBuilder {
DartDependencySetBuilder(String mainScriptPath, String packagesFilePath) : DartDependencySetBuilder(String mainScriptPath, String packagesFilePath) :
this._mainScriptPath = fs.path.canonicalize(mainScriptPath), _mainScriptPath = fs.path.canonicalize(mainScriptPath),
this._mainScriptUri = fs.path.toUri(mainScriptPath), _mainScriptUri = fs.path.toUri(mainScriptPath),
this._packagesFilePath = fs.path.canonicalize(packagesFilePath); _packagesFilePath = fs.path.canonicalize(packagesFilePath);
final String _mainScriptPath; final String _mainScriptPath;
final String _packagesFilePath; final String _packagesFilePath;
...@@ -25,19 +25,40 @@ class DartDependencySetBuilder { ...@@ -25,19 +25,40 @@ class DartDependencySetBuilder {
while (toProcess.isNotEmpty) { while (toProcess.isNotEmpty) {
final Uri currentUri = toProcess.removeLast(); final Uri currentUri = toProcess.removeLast();
final CompilationUnit unit = _parse(currentUri.toFilePath()); final analyzer.CompilationUnit unit = _parse(currentUri.toFilePath());
for (Directive directive in unit.directives) { for (analyzer.Directive directive in unit.directives) {
if (!(directive is UriBasedDirective)) if (!(directive is analyzer.UriBasedDirective))
continue; continue;
final UriBasedDirective uriBasedDirective = directive; final analyzer.UriBasedDirective uriBasedDirective = directive;
final String uriAsString = uriBasedDirective.uri.stringValue; final String uriAsString = uriBasedDirective.uri.stringValue;
Uri resolvedUri = resolveRelativeUri(currentUri, Uri.parse(uriAsString)); Uri resolvedUri = analyzer.resolveRelativeUri(currentUri, Uri.parse(uriAsString));
if (resolvedUri.scheme.startsWith('dart')) if (resolvedUri.scheme.startsWith('dart'))
continue; continue;
if (resolvedUri.scheme == 'package') if (resolvedUri.scheme == 'package') {
resolvedUri = packageMap.uriForPackage(resolvedUri); final Uri newResolvedUri = packageMap.uriForPackage(resolvedUri);
if (newResolvedUri == null) {
throw new DartDependencyException(
'The following Dart file:\n'
' ${currentUri.toFilePath()}\n'
'...refers, in an import, to the following library:\n'
' $resolvedUri\n'
'That library is in a package that is not known. Maybe you forgot to '
'mention it in your pubspec.yaml file?'
);
}
resolvedUri = newResolvedUri;
}
final String path = fs.path.canonicalize(resolvedUri.toFilePath()); final String path = fs.path.canonicalize(resolvedUri.toFilePath());
if (!dependencies.contains(path)) { if (!dependencies.contains(path)) {
if (!fs.isFileSync(path)) {
throw new DartDependencyException(
'The following Dart file:\n'
' ${currentUri.toFilePath()}\n'
'...refers, in an import, to the following library:\n'
' $path\n'
'Unfortunately, that library does not appear to exist on your file system.'
);
}
dependencies.add(path); dependencies.add(path);
toProcess.add(resolvedUri); toProcess.add(resolvedUri);
} }
...@@ -46,5 +67,42 @@ class DartDependencySetBuilder { ...@@ -46,5 +67,42 @@ class DartDependencySetBuilder {
return dependencies.toSet(); return dependencies.toSet();
} }
CompilationUnit _parse(String path) => parseDirectives(fs.file(path).readAsStringSync(), name: path); analyzer.CompilationUnit _parse(String path) {
String body;
try {
body = fs.file(path).readAsStringSync();
} on FileSystemException catch (error) {
throw new DartDependencyException(
'Could not read "$path" when determining Dart dependencies.',
error,
);
}
try {
return analyzer.parseDirectives(body, name: path);
} on analyzer.AnalyzerError catch (error) {
throw new DartDependencyException(
'When trying to parse this Dart file to find its dependencies:\n'
' $path\n'
'...the analyzer failed with the following error:\n'
' ${error.toString().trimRight()}',
error,
);
} on analyzer.AnalyzerErrorGroup catch (error) {
throw new DartDependencyException(
'When trying to parse this Dart file to find its dependencies:\n'
' $path\n'
'...the analyzer failed with the following error:\n'
' ${error.toString().trimRight()}',
error,
);
}
}
} }
class DartDependencyException implements Exception {
DartDependencyException(this.message, [this.parent]);
final String message;
final Exception parent;
@override
String toString() => message;
}
\ No newline at end of file
...@@ -48,6 +48,8 @@ class PackageMap { ...@@ -48,6 +48,8 @@ class PackageMap {
final List<String> pathSegments = packageUri.pathSegments.toList(); final List<String> pathSegments = packageUri.pathSegments.toList();
final String packageName = pathSegments.removeAt(0); final String packageName = pathSegments.removeAt(0);
final Uri packageBase = map[packageName]; final Uri packageBase = map[packageName];
if (packageBase == null)
return null;
final String packageRelativePath = fs.path.joinAll(pathSegments); final String packageRelativePath = fs.path.joinAll(pathSegments);
return packageBase.resolveUri(fs.path.toUri(packageRelativePath)); return packageBase.resolveUri(fs.path.toUri(packageRelativePath));
} }
......
...@@ -5,10 +5,8 @@ ...@@ -5,10 +5,8 @@
import 'dart:async'; import 'dart:async';
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
import 'package:stack_trace/stack_trace.dart';
import 'application_package.dart'; import 'application_package.dart';
import 'base/common.dart';
import 'base/context.dart'; import 'base/context.dart';
import 'base/file_system.dart'; import 'base/file_system.dart';
import 'base/logger.dart'; import 'base/logger.dart';
...@@ -66,29 +64,6 @@ class HotRunner extends ResidentRunner { ...@@ -66,29 +64,6 @@ class HotRunner extends ResidentRunner {
bool _runningFromSnapshot = true; bool _runningFromSnapshot = true;
String kernelFilePath; String kernelFilePath;
@override
Future<int> run({
Completer<DebugConnectionInfo> connectionInfoCompleter,
Completer<Null> appStartedCompleter,
String route,
bool shouldBuild: true
}) {
// Don't let uncaught errors kill the process.
return Chain.capture(() {
return _run(
connectionInfoCompleter: connectionInfoCompleter,
appStartedCompleter: appStartedCompleter,
route: route,
shouldBuild: shouldBuild
);
}, onError: (dynamic error, StackTrace stackTrace) {
// Actually exit on ToolExit.
if (error is ToolExit)
throw error;
printError('Exception from flutter run: $error', stackTrace);
});
}
bool _refreshDartDependencies() { bool _refreshDartDependencies() {
if (!hotRunnerConfig.computeDartDependencies) { if (!hotRunnerConfig.computeDartDependencies) {
// Disabled. // Disabled.
...@@ -102,15 +77,18 @@ class HotRunner extends ResidentRunner { ...@@ -102,15 +77,18 @@ class HotRunner extends ResidentRunner {
new DartDependencySetBuilder(mainPath, packagesFilePath); new DartDependencySetBuilder(mainPath, packagesFilePath);
try { try {
_dartDependencies = new Set<String>.from(dartDependencySetBuilder.build()); _dartDependencies = new Set<String>.from(dartDependencySetBuilder.build());
} catch (error) { } on DartDependencyException catch (error) {
printStatus('Error detected in application source code:', emphasis: true); printError(
printError('$error'); 'Your application could not be compiled, because its dependencies could not be established.\n'
'$error'
);
return false; return false;
} }
return true; return true;
} }
Future<int> _run({ @override
Future<int> run({
Completer<DebugConnectionInfo> connectionInfoCompleter, Completer<DebugConnectionInfo> connectionInfoCompleter,
Completer<Null> appStartedCompleter, Completer<Null> appStartedCompleter,
String route, String route,
......
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'package:analyzer/analyzer.dart';
import 'package:flutter_tools/src/dart/dependencies.dart'; import 'package:flutter_tools/src/dart/dependencies.dart';
import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/file_system.dart';
import 'package:test/test.dart'; import 'package:test/test.dart';
...@@ -40,9 +39,38 @@ void main() { ...@@ -40,9 +39,38 @@ void main() {
new DartDependencySetBuilder(mainPath, packagesPath); new DartDependencySetBuilder(mainPath, packagesPath);
try { try {
builder.build(); builder.build();
fail('expect an assertion to be thrown.'); fail('expect an exception to be thrown.');
} on AnalyzerErrorGroup catch (e) { } on DartDependencyException catch (error) {
expect(e.toString(), contains('foo.dart: Expected a string literal')); expect(error.toString(), contains('foo.dart: Expected a string literal'));
}
});
testUsingContext('bad_path', () {
final String testPath = fs.path.join(dataPath, 'bad_path');
final String mainPath = fs.path.join(testPath, 'main.dart');
final String packagesPath = fs.path.join(testPath, '.packages');
final DartDependencySetBuilder builder =
new DartDependencySetBuilder(mainPath, packagesPath);
try {
builder.build();
fail('expect an exception to be thrown.');
} on DartDependencyException catch (error) {
expect(error.toString(), contains('amaze${fs.path.separator}and${fs.path.separator}astonish.dart'));
}
});
testUsingContext('bad_package', () {
final String testPath = fs.path.join(dataPath, 'bad_package');
final String mainPath = fs.path.join(testPath, 'main.dart');
final String packagesPath = fs.path.join(testPath, '.packages');
final DartDependencySetBuilder builder =
new DartDependencySetBuilder(mainPath, packagesPath);
try {
builder.build();
fail('expect an exception to be thrown.');
} on DartDependencyException catch (error) {
expect(error.toString(), contains('rochambeau'));
expect(error.toString(), contains('pubspec.yaml'));
} }
}); });
}); });
......
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'package:rochambeau/you_have_your_orders_now_go_man_go.dart';
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'amaze/and/astonish.dart';
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