From ffea8351374ecaa45b2128c1bb55575a923f3e23 Mon Sep 17 00:00:00 2001
From: "nweiz@google.com" <nweiz@google.com>
Date: Thu, 26 Jun 2014 23:02:35 +0000
Subject: [PATCH] Report all transformer load errors before exiting.
Previously, pub would exit immediately after the first load error was
reported. Now it will wait to see if any other transformers have
errors and report those as well.
R=rnystrom@google.com
BUG=14552
Review URL: https://codereview.chromium.org//354053002
git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge@37770 260f80e4-7a28-3924-810f-c04153c831b5
---
bin/pub.dart | 33 +---------
lib/src/barback/load_all_transformers.dart | 10 +--
lib/src/exceptions.dart | 9 +++
lib/src/log.dart | 41 +++++++++++++
lib/src/utils.dart | 16 +++++
...transformers_reject_their_config_test.dart | 61 +++++++++++++++++++
6 files changed, 136 insertions(+), 34 deletions(-)
create mode 100644 test/transformer/multiple_transformers_reject_their_config_test.dart
diff --git a/bin/pub.dart b/bin/pub.dart
index 0664ff56..ab95fc6e 100644
--- a/bin/pub.dart
+++ b/bin/pub.dart
@@ -8,7 +8,6 @@ import 'dart:io';
import 'package:args/args.dart';
import 'package:http/http.dart' as http;
import 'package:path/path.dart' as path;
-import 'package:source_maps/source_maps.dart';
import 'package:stack_trace/stack_trace.dart';
import '../lib/src/command.dart';
@@ -88,35 +87,7 @@ void runPub(String cacheDir, ArgResults options, List<String> arguments) {
captureErrors(() => invokeCommand(cacheDir, options),
captureStackChains: captureStackChains).catchError((error, Chain chain) {
- // This is basically the top-level exception handler so that we don't
- // spew a stack trace on our users.
- if (error is SpanException) {
- log.error(error.toString(useColors: canUseSpecialChars));
- } else {
- log.error(getErrorMessage(error));
- }
- log.fine("Exception type: ${error.runtimeType}");
-
- if (log.json.enabled) {
- if (error is UsageException) {
- // Don't print usage info in JSON output.
- log.json.error(error.message);
- } else {
- log.json.error(error);
- }
- }
-
- if (options['trace'] || !isUserFacingException(error)) {
- log.error(chain.terse);
- } else {
- log.fine(chain.terse);
- }
-
- if (error is WrappedException && error.innerError != null) {
- var message = "Wrapped exception: ${error.innerError}";
- if (error.innerChain != null) message = "$message\n${error.innerChain}";
- log.fine(message);
- }
+ log.exception(error, chain);
if (options['trace']) {
log.dumpTranscript();
@@ -141,6 +112,8 @@ and include the results in a bug report on http://dartbug.com/new.
/// Returns the appropriate exit code for [exception], falling back on 1 if no
/// appropriate exit code could be found.
int chooseExitCode(exception) {
+ while (exception is WrappedException) exception = exception.innerError;
+
if (exception is HttpException || exception is http.ClientException ||
exception is SocketException || exception is PubHttpException) {
return exit_codes.UNAVAILABLE;
diff --git a/lib/src/barback/load_all_transformers.dart b/lib/src/barback/load_all_transformers.dart
index 0c85fea9..5956aeab 100644
--- a/lib/src/barback/load_all_transformers.dart
+++ b/lib/src/barback/load_all_transformers.dart
@@ -246,9 +246,11 @@ class _TransformerLoader {
/// If any library hasn't yet been loaded via [load], it will be ignored.
Future<List<Set<Transformer>>> transformersForPhases(
Iterable<Set<TransformerConfig>> phases) {
- return Future.wait(phases.map((phase) =>
- Future.wait(phase.map(transformersFor)).then(unionAll)))
- // Return a growable list so that callers can add phases.
- .then((phases) => phases.toList());
+ return Future.wait(phases.map((phase) {
+ return waitAndPrintErrors(phase.map(transformersFor)).then(unionAll);
+ })).then((phases) {
+ // Return a growable list so that callers can add phases.
+ return phases.toList();
+ });
}
}
diff --git a/lib/src/exceptions.dart b/lib/src/exceptions.dart
index e67ef64d..af09a0c3 100644
--- a/lib/src/exceptions.dart
+++ b/lib/src/exceptions.dart
@@ -39,6 +39,15 @@ class WrappedException extends ApplicationException {
super(message);
}
+/// A class for exceptions that shouldn't be printed at the top level.
+///
+/// This is usually used when an exception has already been printed using
+/// [log.exception].
+class SilentException extends WrappedException {
+ SilentException(innerError, [StackTrace innerTrace])
+ : super(innerError.toString(), innerError, innerTrace);
+}
+
/// A class for command usage exceptions.
class UsageException extends ApplicationException {
/// The command usage information.
diff --git a/lib/src/log.dart b/lib/src/log.dart
index eaf87320..d34c612d 100644
--- a/lib/src/log.dart
+++ b/lib/src/log.dart
@@ -10,8 +10,10 @@ import 'dart:convert';
import 'dart:io';
import 'package:path/path.dart' as p;
+import 'package:source_maps/source_maps.dart';
import 'package:stack_trace/stack_trace.dart';
+import 'exceptions.dart';
import 'io.dart';
import 'progress.dart';
import 'transcript.dart';
@@ -305,6 +307,45 @@ void processResult(String executable, PubProcessResult result) {
io(buffer.toString().trim());
}
+/// Logs an exception.
+void exception(exception, [StackTrace trace]) {
+ if (exception is SilentException) return;
+
+ var chain = trace == null ? new Chain.current() : new Chain.forTrace(trace);
+
+ // This is basically the top-level exception handler so that we don't
+ // spew a stack trace on our users.
+ if (exception is SpanException) {
+ error(exception.toString(useColors: canUseSpecialChars));
+ } else {
+ error(getErrorMessage(exception));
+ }
+ fine("Exception type: ${exception.runtimeType}");
+
+ if (json.enabled) {
+ if (exception is UsageException) {
+ // Don't print usage info in JSON output.
+ json.error(exception.message);
+ } else {
+ json.error(exception);
+ }
+ }
+
+ if (!isUserFacingException(exception)) {
+ error(chain.terse);
+ } else {
+ fine(chain.terse);
+ }
+
+ if (exception is WrappedException && exception.innerError != null) {
+ var message = "Wrapped exception: ${exception.innerError}";
+ if (exception.innerChain != null) {
+ message = "$message\n${exception.innerChain}";
+ }
+ fine(message);
+ }
+}
+
/// Enables recording of log entries.
void recordTranscript() {
_transcript = new Transcript<Entry>(_MAX_TRANSCRIPT);
diff --git a/lib/src/utils.dart b/lib/src/utils.dart
index 24c9ecfc..d3d2c4aa 100644
--- a/lib/src/utils.dart
+++ b/lib/src/utils.dart
@@ -16,6 +16,7 @@ import 'package:path/path.dart' as path;
import "package:stack_trace/stack_trace.dart";
import 'exceptions.dart';
+import 'log.dart' as log;
export '../../asset/dart/utils.dart';
@@ -124,6 +125,21 @@ Future captureErrors(Future callback(), {bool captureStackChains: false}) {
return completer.future;
}
+/// Like [Future.wait], but prints all errors from the futures as they occur and
+/// only returns once all Futures have completed, successfully or not.
+///
+/// This will wrap the first error thrown in a [SilentException] and rethrow it.
+Future waitAndPrintErrors(Iterable<Future> futures) {
+ return Future.wait(futures.map((future) {
+ return future.catchError((error, stackTrace) {
+ log.exception(error, stackTrace);
+ throw error;
+ });
+ })).catchError((error, stackTrace) {
+ throw new SilentException(error, stackTrace);
+ });
+}
+
/// Returns a [StreamTransformer] that will call [onDone] when the stream
/// completes.
///
diff --git a/test/transformer/multiple_transformers_reject_their_config_test.dart b/test/transformer/multiple_transformers_reject_their_config_test.dart
new file mode 100644
index 00000000..8461b5c6
--- /dev/null
+++ b/test/transformer/multiple_transformers_reject_their_config_test.dart
@@ -0,0 +1,61 @@
+// Copyright (c) 2014, the Dart project authors. Please see the AUTHORS d.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.
+
+library pub_tests;
+
+import 'package:scheduled_test/scheduled_stream.dart';
+import 'package:scheduled_test/scheduled_test.dart';
+
+import '../descriptor.dart' as d;
+import '../test_pub.dart';
+import '../serve/utils.dart';
+
+const REJECT_CONFIG_TRANSFORMER = """
+import 'dart:async';
+
+import 'package:barback/barback.dart';
+
+class RejectConfigTransformer extends Transformer {
+ RejectConfigTransformer.asPlugin(BarbackSettings settings) {
+ throw "I hate these settings!";
+ }
+
+ Future<bool> isPrimary(_) => new Future.value(true);
+ Future apply(Transform transform) {}
+}
+""";
+
+main() {
+ initConfig();
+
+ withBarbackVersions("any", () {
+ integration("multiple transformers in the same phase reject their "
+ "configurations", () {
+ d.dir(appPath, [
+ d.pubspec({
+ "name": "myapp",
+ "transformers": [[
+ {"myapp/src/transformer": {'foo': 'bar'}},
+ {"myapp/src/transformer": {'baz': 'bang'}},
+ {"myapp/src/transformer": {'qux': 'fblthp'}}
+ ]]
+ }),
+ d.dir("lib", [d.dir("src", [
+ d.file("transformer.dart", REJECT_CONFIG_TRANSFORMER)
+ ])])
+ ]).create();
+
+ createLockFile('myapp', pkg: ['barback']);
+
+ // We should see three instances of the error message, once for each
+ // use of the transformer.
+ var pub = startPubServe();
+ for (var i = 0; i < 3; i++) {
+ pub.stderr.expect(consumeThrough(endsWith('Error loading transformer: '
+ 'I hate these settings!')));
+ }
+ pub.shouldExit(1);
+ });
+ });
+}
\ No newline at end of file
--
GitLab