fix(transformer): Don't hang on bad urls and log better errors

closes angular#2605

Author: jakemac53

modules/angular2/src/transform/common/xhr_impl.dart

@@ -16,9 +16,14 @@ class XhrImpl implements XHR {
  Future<String> get(String url) async {
  var assetId = uriToAssetId(_entryPoint, url, logger, null /* span */,
  errorOnAbsolute: false);
+ if (assetId == null) {
+ logger.error(
+ 'Uri $url not supported from $_entryPoint, could not build AssetId');
+ return null;
+ }
  var templateExists = await _reader.hasInput(assetId);
  if (!templateExists) {
- logger.error('Could not read template at uri $url from $_entryPoint');
+ logger.error('Could not read asset at uri $url from $_entryPoint');
  return null;
  }
  return await _reader.readAsString(assetId);

modules/angular2/src/transform/directive_processor/visitors.dart

@@ -1,5 +1,6 @@
 library angular2.transform.directive_processor.visitors;
 
+import 'dart:async';
 import 'package:analyzer/analyzer.dart';
 import 'package:analyzer/src/generated/java_core.dart';
 import 'package:angular2/src/render/xhr.dart' show XHR;
@@ -263,7 +264,7 @@ class AnnotationsTransformVisitor extends ToSourceVisitor {
  var url = node.expression.accept(_evaluator);
  if (url is String) {
  writer.print("template: r'''");
- writer.asyncPrint(_xhr.get(url));
+ writer.asyncPrint(_readOrEmptyString(url));
  writer.print("'''");
  return null;
  } else {
@@ -276,7 +277,7 @@ class AnnotationsTransformVisitor extends ToSourceVisitor {
  for (var url in urls) {
  if (url is String) {
  writer.print("r'''");
- writer.asyncPrint(_xhr.get(url));
+ writer.asyncPrint(_readOrEmptyString(url));
  writer.print("''', ");
  } else {
  logger.warning('style url is not a String ${url}');
@@ -287,4 +288,14 @@ class AnnotationsTransformVisitor extends ToSourceVisitor {
  }
  return super.visitNamedExpression(node);
  }
+
+ /// Attempts to read the content from {@link url}, if it returns null then
+ /// just return the empty string.
+ Future<String> _readOrEmptyString(String url) async {
+ var content = await _xhr.get(url);
+ if (content == null) {
+ content = '';
+ }
+ return content;
+ }
 }

modules/angular2/test/transform/directive_processor/all_tests.dart

@@ -4,6 +4,8 @@ import 'package:barback/barback.dart';
 import 'package:angular2/src/transform/directive_processor/rewriter.dart';
 import 'package:angular2/src/transform/common/annotation_matcher.dart';
 import 'package:angular2/src/transform/common/asset_reader.dart';
+import 'package:angular2/src/transform/common/logging.dart' as log;
+import 'package:code_transformers/messages/build_logger.dart';
 import 'package:dart_style/dart_style.dart';
 import 'package:guinness/guinness.dart';
 import 'package:path/path.dart' as path;
@@ -67,12 +69,29 @@ void allTests() {
 
  _testNgDeps('should not include superclasses in `interfaces`.',
  'superclass_files/soup.dart');
+
+ _testNgDeps(
+ 'should not throw/hang on invalid urls', 'invalid_url_files/hello.dart',
+ expectedLogs: [
+ 'ERROR: Uri /bad/absolute/url.html not supported from angular2|test/'
+ 'transform/directive_processor/invalid_url_files/hello.dart, could not '
+ 'build AssetId',
+ 'ERROR: Could not read asset at uri package:invalid/package.css from '
+ 'angular2|test/transform/directive_processor/invalid_url_files/'
+ 'hello.dart',
+ 'ERROR: Could not read asset at uri bad_relative_url.css from angular2|'
+ 'test/transform/directive_processor/invalid_url_files/hello.dart'
+ ]);
 }
 
 void _testNgDeps(String name, String inputPath,
  {List<AnnotationDescriptor> customDescriptors: const [], AssetId assetId,
- AssetReader reader}) {
+ AssetReader reader, List<String> expectedLogs}) {
  it(name, () async {
+ if (expectedLogs != null) {
+ log.setLogger(new RecordingLogger());
+ }
+
  var inputId = _assetIdForPath(inputPath);
  if (reader == null) {
  reader = new TestAssetReader();
@@ -93,8 +112,35 @@ void _testNgDeps(String name, String inputPath,
  expect(formatter.format(output))
  .toEqual((await reader.readAsString(expectedId)).replaceAll('\r\n', '\n'));
  }
+
+ if (expectedLogs != null) {
+ expect((log.logger as RecordingLogger).logs, expectedLogs);
+ }
  });
 }
 
 AssetId _assetIdForPath(String path) =>
  new AssetId('angular2', 'test/transform/directive_processor/$path');
+
+class RecordingLogger implements BuildLogger {
+ @override
+ final String detailsUri = '';
+ @override
+ final bool convertErrorsToWarnings = false;
+
+ List<String> logs = [];
+
+ void _record(prefix, msg) => logs.add('$prefix: $msg');
+
+ void info(msg, {AssetId asset, SourceSpan span}) => _record('INFO', msg);
+
+ void fine(msg, {AssetId asset, SourceSpan span}) => _record('FINE', msg);
+
+ void warning(msg, {AssetId asset, SourceSpan span}) => _record('WARN', msg);
+
+ void error(msg, {AssetId asset, SourceSpan span}) => _record('ERROR', msg);
+
+ Future writeOutput() => throw new UnimplementedError();
+ Future addLogFilesFromAsset(AssetId id, [int nextNumber = 1]) =>
+ throw new UnimplementedError();
+}

modules/angular2/test/transform/directive_processor/invalid_url_files/expected/hello.ng_deps.dart

@@ -0,0 +1,20 @@
+library test.transform.directive_processor.url_expression_files.hello.ng_deps.dart;
+
+import 'hello.dart';
+import 'package:angular2/angular2.dart'
+ show bootstrap, Component, Directive, View, NgElement;
+
+var _visited = false;
+void initReflector(reflector) {
+ if (_visited) return;
+ _visited = true;
+ reflector
+ ..registerType(HelloCmp, {
+ 'factory': () => new HelloCmp(),
+ 'parameters': const [],
+ 'annotations': const [
+ const Component(selector: 'hello-app'),
+ const View(template: r'''''', styles: const [r'''''', r'''''',])
+ ]
+ });
+}

modules/angular2/test/transform/directive_processor/invalid_url_files/hello.dart

@@ -0,0 +1,13 @@
+library test.transform.directive_processor.url_expression_files.hello;
+
+import 'package:angular2/angular2.dart'
+show bootstrap, Component, Directive, View, NgElement;
+
+@Component(selector: 'hello-app')
+@View(
+ templateUrl: '/bad/absolute/url.html',
+ styleUrls: const [
+ 'package:invalid/package.css',
+ 'bad_relative_url.css'
+ ])
+class HelloCmp {}