Skip to content
This repository has been archived by the owner on Sep 16, 2022. It is now read-only.

Commit

Permalink
feat(Core): Start work on a MissingProviderError, with better error m…
Browse files Browse the repository at this point in the history
…essages.

Added a simple test (passes), and a complex test (skipped, does not pass). In
dev-mode we will start tracking the chain of failed lookups and give a better
error message, i.e. "No provider found for B: A -> B".

Work towards #434.

PiperOrigin-RevId: 184594151
  • Loading branch information
matanlurey committed Feb 5, 2018
1 parent ca84a28 commit d97324b
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 14 deletions.
13 changes: 13 additions & 0 deletions _tests/lib/matchers.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,20 @@
import 'dart:html';

import 'package:angular/src/di/errors.dart';
import 'package:angular/src/facade/lang.dart';
import 'package:test/test.dart';

/// Matches a missing provider error thrown at runtime.
final Matcher throwsMissingProviderError = (() {
if (assertionsEnabled()) {
return _throwsMissingProviderError;
}
return throwsArgumentError;
})();

final _isMissingProviderError = const isInstanceOf<MissingProviderError>();
final _throwsMissingProviderError = throwsA(_isMissingProviderError);

/// Matches textual content of an element including children.
Matcher hasTextContent(expected) => new _HasTextContent(expected);

Expand Down
46 changes: 37 additions & 9 deletions _tests/test/di/injector_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ 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;

Expand Down Expand Up @@ -43,11 +44,26 @@ void main() {

test('should throw by default', () {
i = new Injector.empty();
expect(() => i.get(ExampleService), throwsArgumentError);
expect(() => i.inject(ExampleService), throwsArgumentError);
expect(() => i.injectFromSelf(ExampleService), throwsArgumentError);
expect(() => i.injectFromAncestry(ExampleService), throwsArgumentError);
expect(() => i.injectFromParent(ExampleService), throwsArgumentError);
expect(
() => i.get(ExampleService),
throwsMissingProviderError,
);
expect(
() => i.inject(ExampleService),
throwsMissingProviderError,
);
expect(
() => i.injectFromSelf(ExampleService),
throwsMissingProviderError,
);
expect(
() => i.injectFromAncestry(ExampleService),
throwsMissingProviderError,
);
expect(
() => i.injectFromParent(ExampleService),
throwsMissingProviderError,
);
});

test('should use orElse if provided', () {
Expand All @@ -64,7 +80,10 @@ void main() {
i = new Injector.empty(parent);
expect(i.get(ExampleService), 123);
expect(i.inject(ExampleService), 123);
expect(() => i.injectFromSelf(ExampleService), throwsArgumentError);
expect(
() => i.injectFromSelf(ExampleService),
throwsMissingProviderError,
);
expect(i.injectFromAncestry(ExampleService), 123);
expect(i.injectFromParent(ExampleService), 123);
});
Expand All @@ -83,8 +102,14 @@ void main() {
expect(i.get(ExampleService), 123);
expect(i.inject(ExampleService), 123);
expect(i.injectFromSelf(ExampleService), 123);
expect(() => i.injectFromAncestry(ExampleService), throwsArgumentError);
expect(() => i.injectFromParent(ExampleService), throwsArgumentError);
expect(
() => i.injectFromAncestry(ExampleService),
throwsMissingProviderError,
);
expect(
() => i.injectFromParent(ExampleService),
throwsMissingProviderError,
);
});

test('should return itself if Injector is passed', () {
Expand Down Expand Up @@ -193,7 +218,10 @@ void main() {

test('should thrown when a provider was not found', () {
i = new Injector.slowReflective([]);
expect(() => i.get(#ABC), throwsArgumentError);
expect(
() => i.get(#ABC),
throwsMissingProviderError,
);
});

test('should support resolveAndCreateChild', () {
Expand Down
36 changes: 36 additions & 0 deletions _tests/test/di/missing_provider_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
@TestOn('browser')
import 'package:angular/angular.dart';
import 'package:angular/src/di/injector/injector.dart';
import 'package:test/test.dart';

import 'missing_provider_test.template.dart' as ng;

void main() {
ng.initReflector();

test('should throw an error for a single missing token', () {
final nullInjector = const Injector.empty();
expect(
() => nullInjector.get(A),
throwsA(predicate((e) => '$e'.contains('No provider found for $A'))),
);
});

test('should throw a better error for a path of missing tokens', () {
final injectorWithAB = new Injector.slowReflective([A]);
expect(
() => injectorWithAB.get(A),
throwsA(
predicate((e) => '$e'.contains('No provider found for $B: $A -> $B')),
),
);
}, skip: 'Not yet supported: We need to track the provider chain.');
}

@Injectable()
class A {
A(B b);
}

@Injectable()
class B {}
9 changes: 5 additions & 4 deletions _tests/test/regression/755_reflective_meta_fail_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
@TestOn('browser')

import 'package:test/test.dart';
import 'package:_tests/matchers.dart';
import 'package:angular/angular.dart';

import '755_reflective_meta_fail_test.template.dart' as ng_generated;
Expand All @@ -15,10 +16,10 @@ void main() {
const Provider(ServiceInjectingToken, useClass: ServiceInjectingToken),
// Intentionally omit a binding for "stringToken".
]);

// Used to return an Object representing the secret "notFound" instead of
// throwing ArgumentError, which was the expected behavior.
expect(() => injector.get(ServiceInjectingToken), throwsArgumentError);
expect(
() => injector.get(ServiceInjectingToken),
throwsMissingProviderError,
);
});
}

Expand Down
46 changes: 46 additions & 0 deletions angular/lib/src/di/errors.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import 'package:angular/src/facade/lang.dart';

import 'injector/injector.dart';

/// A list representing tokens not found in development mode.
///
/// In production mode, this is always `null`, and should be ignored.
List<Object> devModeTokensNotResolvable;

/// An error thrown when a token is not found when using an [Injector].
///
/// **NOTE**: This error is only guaranteed to be thrown in development mode
/// (i.e. when `assert` is enabled). Otherwise a generic error is thrown with
/// less information (but better performance and lower code-size).
class MissingProviderError extends Error {
/// Creates an error representing a miss on [token] looked up on [injector].
static Error create(Injector injector, Object token) {
if (assertionsEnabled()) {
_findMissingTokenPath(injector, token);
return new MissingProviderError._(devModeTokensNotResolvable);
}
return new ArgumentError('No provider found for $token.');
}

static void _findMissingTokenPath(Injector injector, Object token) {
// TODO(matanl): Actually implement the rest of the missing path.
// i.e. Leaf -> Parent -> Root, not just Leaf.
devModeTokensNotResolvable = [token];
}

/// A list of tokens that was attempted, in order, that resulted in an error.
///
/// For example: `[Leaf, Parent, Root]`, where `Root` was the root service
/// that was missing, looked up via a dependency on `Parent`, which was a
/// dependency of `Leaf`, the actual token requested via `get(Leaf)`.
final List<Object> tokens;

MissingProviderError._(this.tokens);

@override
String toString() =>
// Example: No provider found for Leaf: Leaf -> Parent -> Root.
tokens.length == 1
? 'No provider found for ${tokens.first}'
: 'No provider found for ${tokens.first}: ' + tokens.join(' -> ');
}
6 changes: 5 additions & 1 deletion angular/lib/src/di/injector/injector.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import 'package:meta/meta.dart';

import '../errors.dart';

import 'empty.dart';
import 'hierarchical.dart';
import 'map.dart';
Expand All @@ -17,8 +19,10 @@ typedef T OrElseInject<T>(Injector injector, Object token);
/// **INTERNAL ONLY**: Sentinel value for determining a missing DI instance.
const Object throwIfNotFound = const Object();

/// **INTERNAL ONLY**: Throws an error that [token] was not found.
@alwaysThrows
Null throwsNotFound(Injector injector, Object token) {
throw new ArgumentError('No provider found for $token.');
throw MissingProviderError.create(injector, token);
}

/// Support for imperatively loading dependency injected services at runtime.
Expand Down

0 comments on commit d97324b

Please sign in to comment.