feat(Core): More work on InjectionError.enableBetterErrors.

This flag is *enabled* by default. When enabled, you can expect *some* better
error messages, but the better messages are *not* lazy, early testing by key customers don't show any noticeable performance regressions in DDC.

See a potentially blocking issue:
#434 (comment)

... where we've decided to evaluate code cost before going forward on that one.

PiperOrigin-RevId: 184883497

_tests/test/di/directive_test.dart

@@ -8,6 +8,8 @@ import 'directive_test.template.dart' as ng_generated;
 
 /// Verifies whether injection through directives/components is correct.
 void main() {
+  // TODO(matanl): Remove once this is the default.
+  InjectionError.enableBetterErrors = true;
   ng_generated.initReflector();
 
   tearDown(disposeAnyRunningTest);
@@ -143,6 +145,63 @@ void main() {
     final fixture = await testBed.create();
     expect(fixture.assertOnlyInstance.childView.example, ['A', 'B', 'C']);
   });
+
+  test('should throw a readable error message on a 1-node failure', () {
+    final testBed = new NgTestBed<WillFailInjecting1Node>();
+    expect(
+      () => testBed.create(),
+      throwsA(
+        predicate(
+          (e) => '$e'.endsWith('No provider found for $MissingService'),
+        ),
+      ),
+    );
+  });
+
+  test('should throw a readable error message on a 2-node failure', () {
+    // NOTE: In an ideal scenario, this would throw a better error, i.e.
+    //   InjectsMissingService -> MissingService
+    //
+    // ... but this would require enter() and leave() wrapping around the
+    // successful cases in AppView-local injection (and changes to the
+    // generated code).
+    //
+    // If we end up doing this, we should modify the test accordingly.
+    final testBed = new NgTestBed<WillFailInjecting2Node>();
+    expect(
+      () => testBed.create(),
+      throwsA(
+        predicate(
+          (e) => '$e'.endsWith('No provider found for $MissingService'),
+        ),
+      ),
+      reason: 'AppView does not trace local injections',
+    );
+  });
+
+  test('should throw a readable erro message on a 2-node/parent failure', () {
+    // Passes, unlike the missing error case, because the parent injector, in
+    // this case a ReflectiveInjector, *does* trace the individual calls.
+    final testBed = new NgTestBed<WillFailInjecting2NodeParent>().addProviders([
+      new Provider(
+        InjectsMissingService,
+        useFactory: (Object willNotBeCalled) => null,
+        deps: const [
+          MissingService,
+        ],
+      )
+    ]);
+    expect(
+      () => testBed.create(),
+      throwsA(
+        predicate(
+          (e) => '$e'.contains(''
+              'No provider found for $MissingService: '
+              '$InjectsMissingService -> $MissingService.'),
+        ),
+      ),
+    );
+  });
 }
 
 @Component(
@@ -511,3 +570,38 @@ class ChildThatInjectsTypedToken {
 
   ChildThatInjectsTypedToken(@Inject(listOfStringToken) this.example);
 }
+
+class MissingService {}
+
+@Component(
+  selector: 'will-fail-injecting-1-node',
+  template: '',
+)
+class WillFailInjecting1Node {
+  WillFailInjecting1Node(MissingService _);
+}
+
+class InjectsMissingService {
+  InjectsMissingService(MissingService _);
+}
+
+@Component(
+  selector: 'will-fail-injecting-2-node',
+  providers: const [
+    const Provider(
+      InjectsMissingService,
+    ),
+  ],
+  template: '',
+)
+class WillFailInjecting2Node {
+  WillFailInjecting2Node(InjectsMissingService _);
+}
+
+@Component(
+  selector: 'will-fail-injecting-2-node',
+  template: '',
+)
+class WillFailInjecting2NodeParent {
+  WillFailInjecting2NodeParent(InjectsMissingService _);
+}

_tests/test/di/injector_test.dart

@@ -5,7 +5,6 @@ import 'package:angular/src/di/injector/hierarchical.dart';
 import 'package:angular/src/di/injector/injector.dart';
 import 'package:test/test.dart';
 import 'package:angular/src/di/reflector.dart' as reflector;
-import 'package:angular_test/angular_test.dart';
 import 'package:_tests/matchers.dart';
 
 import 'injector_test.template.dart' as ng;
@@ -68,6 +67,32 @@ void main() {
         );
       });
 
+      test('should throw a readable message with injection fails', () {
+        // Anything but injector.get(Injector) will fail here.
+        final injector = new Injector.empty();
+        expect(
+          () => injector.get(ExampleService),
+          throwsA(
+            predicate(
+              (e) => '$e'.endsWith('No provider found for $ExampleService'),
+            ),
+          ),
+        );
+      });
+
+      test('should throw a readable message even with a parent injector', () {
+        final parent = new Injector.empty();
+        final child = new Injector.empty(parent);
+        expect(
+          () => child.get(ExampleService),
+          throwsA(
+            predicate(
+              (e) => '$e'.endsWith('No provider found for $ExampleService'),
+            ),
+          ),
+        );
+      });
+
       test('should use orElse if provided', () {
         i = new Injector.empty();
         expect(i.get(ExampleService, 123), 123);
@@ -117,6 +142,18 @@ void main() {
       test('should return itself if Injector is passed', () {
         expect(i.get(Injector), i);
       });
+
+      test('should throw a readable error message on a failure', () {
+        final injector = new Injector.map({});
+        expect(
+          () => injector.get(ExampleService),
+          throwsA(
+            predicate(
+              (e) => '$e'.endsWith('No provider found for $ExampleService'),
+            ),
+          ),
+        );
+      });
     });
 
     group('.slowReflective', () {
@@ -311,19 +348,67 @@ void main() {
         expect(injector.get(unnamedTokenOfString), 2);
       });
 
-      test('should throw the provider token that failed', () {
+      test('should throw a readable error message on a 1-node failure', () {
+        final injector = new Injector.slowReflective([]);
+        expect(
+          () => injector.get(ExampleService),
+          throwsA(
+            predicate(
+              (e) => '$e'.endsWith('No provider found for $ExampleService'),
+            ),
+          ),
+        );
+      });
+
+      test('should throw a readable error message on a 2-node failure', () {
         final injector = new Injector.slowReflective([
           new Provider(
             ExampleService,
-            useFactory: (ExampleService2 e) => null,
+            useFactory: (Null willNeverBeCalled) => null,
             deps: const [ExampleService2],
           ),
         ]);
         expect(
           () => injector.get(ExampleService),
-          throwsInAngular(
+          throwsA(
             predicate(
-              (e) => '$e'.contains('No provider found for ExampleService2'),
+              (e) => '$e'.contains(''
+                  'No provider found for $ExampleService2: '
+                  '$ExampleService -> $ExampleService2.'),
+            ),
+          ),
+        );
+      });
+
+      test('should throw a readable error message on a 3-node failure', () {
+        // ExampleService -->
+        //   ExampleService2 -->
+        //     ExampleService3 + ExampleService4
+        //
+        // ... where ExampleService4 is missing.
+        final injector = new Injector.slowReflective([
+          new Provider(
+            ExampleService,
+            useFactory: (Null willNeverBeCalled) => null,
+            deps: const [ExampleService2],
+          ),
+          new Provider(
+            ExampleService2,
+            useFactory: (Null willNeverBeCalled) => null,
+            deps: const [ExampleService3, ExampleService4],
+          ),
+          new Provider(
+            ExampleService3,
+            useValue: new ExampleService3(),
+          ),
+        ]);
+        expect(
+          () => injector.get(ExampleService),
+          throwsA(
+            predicate(
+              (e) => '$e'.contains(''
+                  'No provider found for $ExampleService4: '
+                  '$ExampleService -> $ExampleService2 -> $ExampleService4.'),
             ),
           ),
         );
@@ -385,6 +470,43 @@ void main() {
         expect(injector.get(unnamedTokenOfDynamic), 5);
         expect(injector.get(unnamedTokenOfString), 6);
       });
+
+      test('should throw a readable error message on a 1-node failure', () {
+        expect(
+          () => injector.get(MissingService),
+          throwsA(
+            predicate(
+              (e) => '$e'.endsWith('No provider found for $MissingService'),
+            ),
+          ),
+        );
+      });
+
+      test('should throw a readable error message on a 2-node failure', () {
+        expect(
+          () => injector.get(ExampleService3),
+          throwsA(
+            predicate(
+              (e) => '$e'.contains(''
+                  'No provider found for $MissingService: '
+                  '$ExampleService3 -> $MissingService.'),
+            ),
+          ),
+        );
+      });
+
+      test('should throw a readable error message on a 3-node failure', () {
+        expect(
+          () => injector.get(ExampleService4),
+          throwsA(
+            predicate(
+              (e) => '$e'.contains(''
+                  'No provider found for $MissingService: '
+                  '$ExampleService4 -> $ExampleService3 -> $MissingService.'),
+            ),
+          ),
+        );
+      });
     });
   });
 }
@@ -411,6 +533,12 @@ class ExampleService {
 
 class ExampleService2 implements ExampleService {}
 
+class ExampleService3 {}
+
+class ExampleService4 {}
+
+class MissingService {}
+
 const stringToken = const OpaqueToken('stringToken');
 const numberToken = const OpaqueToken('numberToken');
 const booleanToken = const OpaqueToken('booleanToken');
@@ -427,6 +555,9 @@ const typedTokenOfListString = const OpaqueToken<List<String>>('typedToken');
 const unnamedTokenOfDynamic = const OpaqueToken();
 const unnamedTokenOfString = const OpaqueToken<String>();
 
+Null willNeverBeCalled1(Object _) => null;
+Null willNeverBeCalled2(Object _, Object __) => null;
+
 @Injector.generate(const [
   const Provider(ExampleService, useClass: ExampleService2),
   const Provider(ExampleService2),
@@ -435,6 +566,21 @@ const unnamedTokenOfString = const OpaqueToken<String>();
   const Provider(booleanToken, useValue: true),
   const Provider(simpleConstToken, useValue: const ExampleService()),
 
+  // Example of a runtime failure; MissingService
+  const Provider(
+    ExampleService3,
+    useFactory: willNeverBeCalled1,
+    deps: const [MissingService],
+  ),
+
+  // Example of a runtime failure; ExampleService3 -> MissingService.
+  const Provider(
+    ExampleService4,
+    useFactory: willNeverBeCalled2,
+    // Will find ExampleService2, ExampleService3 will fail (see above).
+    deps: const [ExampleService2, ExampleService3],
+  ),
+
   // TODO(matanl): Switch to ValueProvider.forToken once supported.
   const Provider<String>(multiStringToken, useValue: 'A'),
   const Provider<String>(multiStringToken, useValue: 'B'),

angular/CHANGELOG.md

@@ -2,8 +2,8 @@
 
 * Added `InjectionError` and `NoProviderError`, which _may_ be thrown during
   dependency injection when `InjectionError.enableBetterErrors` is set to
-  `true`. This is an experiment, and we not complete this feature (and it could
-  be rolled back entirely).
+  `true`. This is an experiment, and we may not complete this feature (and it
+  could be rolled back entirely).
 
 ### Bug fixes
 

angular/lib/src/core/linker/app_view.dart

@@ -6,6 +6,7 @@ import 'package:meta/meta.dart';
 import 'package:angular/src/core/app_view_consts.dart';
 import 'package:angular/src/core/change_detection/change_detection.dart'
     show ChangeDetectorRef, ChangeDetectionStrategy, ChangeDetectorState;
+import 'package:angular/src/di/errors.dart' as di_errors;
 import 'package:angular/src/di/injector/element.dart';
 import 'package:angular/src/di/injector/injector.dart'
     show throwIfNotFound, Injector;
@@ -296,6 +297,7 @@ abstract class AppView<T> {
   }
 
   dynamic injectorGet(token, int nodeIndex, [notFoundValue = throwIfNotFound]) {
+    di_errors.debugInjectorEnter(token);
     var result = _UndefinedInjectorResult;
     AppView view = this;
     while (identical(result, _UndefinedInjectorResult)) {
@@ -312,6 +314,7 @@ abstract class AppView<T> {
       nodeIndex = view.viewData.parentIndex;
       view = view.parentView;
     }
+    di_errors.debugInjectorLeave(token);
     return result;
   }