Skip to content

Commit 6549418

Browse files
committed
refactor(core): include DI path into cyclic dependency error message (#50902)
This commit updates the logic to better handle a situation when there is a cyclic DI dependency detected. Previously, the error message used to contain the name of the token that triggered the problem. With this change, the DI resolution path would also be included, so that it's easier to find and resolve the cycle. PR Close #50902
1 parent 66a85ab commit 6549418

26 files changed

+544
-140
lines changed

packages/core/src/di/injector_compatibility.ts

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ export function injectInjectorOnly<T>(
105105
return injectRootLimpMode(token, undefined, flags);
106106
} else {
107107
const options = convertToInjectOptions(flags);
108+
// TODO: improve the typings here.
109+
// `token` can be a multi: true provider definition, which is considered as a Token but not represented in the typings
108110
const value = currentInjector.retrieve(token as PrimitivesInjectionToken<T>, options) as T;
109111
ngDevMode && emitInjectEvent(token as Type<unknown>, value, flags);
110112
if (isNotFound(value)) {
@@ -382,22 +384,6 @@ export function getInjectFlag(token: any): number | undefined {
382384
return token[DI_DECORATOR_FLAG];
383385
}
384386

385-
export function catchInjectorError(
386-
e: any,
387-
token: any,
388-
injectorErrorName: string,
389-
source: string | null,
390-
): never {
391-
const tokenPath: any[] = e[NG_TEMP_TOKEN_PATH];
392-
if (token[SOURCE]) {
393-
tokenPath.unshift(token[SOURCE]);
394-
}
395-
e.message = formatError('\n' + e.message, tokenPath, injectorErrorName, source);
396-
e[NG_TOKEN_PATH] = tokenPath;
397-
e[NG_TEMP_TOKEN_PATH] = null;
398-
throw e;
399-
}
400-
401387
export function formatError(
402388
text: string,
403389
obj: any,

packages/core/src/di/null_injector.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,22 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {NotFoundError} from '@angular/core/primitives/di';
9+
import {RuntimeErrorCode} from '../errors';
10+
import {createRuntimeError} from '../render3/errors_di';
1011
import {stringify} from '../util/stringify';
12+
1113
import type {Injector} from './injector';
1214
import {THROW_IF_NOT_FOUND} from './injector_compatibility';
1315

1416
export class NullInjector implements Injector {
1517
get(token: any, notFoundValue: any = THROW_IF_NOT_FOUND): any {
1618
if (notFoundValue === THROW_IF_NOT_FOUND) {
17-
const error = new NotFoundError(`NullInjectorError: No provider for ${stringify(token)}!`);
19+
const message = ngDevMode ? `No provider found for \`${stringify(token)}\`.` : '';
20+
const error = createRuntimeError(message, RuntimeErrorCode.PROVIDER_NOT_FOUND);
21+
22+
// Note: This is the name used by the primitives to identify a not found error.
23+
error.name = 'ɵNotFound';
24+
1825
throw error;
1926
}
2027
return notFoundValue;

packages/core/src/di/provider_collection.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {RuntimeError, RuntimeErrorCode} from '../errors';
1010
import {Type} from '../interface/type';
1111
import {getComponentDef} from '../render3/def_getters';
1212
import {getFactoryDef} from '../render3/definition_factory';
13-
import {throwCyclicDependencyError, throwInvalidProviderError} from '../render3/errors_di';
13+
import {cyclicDependencyErrorWithDetails, throwInvalidProviderError} from '../render3/errors_di';
1414
import {stringifyForError} from '../render3/util/stringify_utils';
1515
import {deepForEach} from '../util/array_utils';
1616
import {EMPTY_ARRAY} from '../util/empty';
@@ -270,8 +270,8 @@ export function walkProviderTree(
270270
// Check for circular dependencies.
271271
if (ngDevMode && parents.indexOf(defType) !== -1) {
272272
const defName = stringify(defType);
273-
const path = parents.map(stringify);
274-
throwCyclicDependencyError(defName, path);
273+
const path = parents.map(stringify).concat(defName);
274+
throw cyclicDependencyErrorWithDetails(defName, path);
275275
}
276276

277277
// Check for multiple imports of the same module

packages/core/src/di/r3_injector.ts

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ import {
2121
} from '../render3/debug/injector_profiler';
2222
import {FactoryFn, getFactoryDef} from '../render3/definition_factory';
2323
import {
24-
throwCyclicDependencyError,
24+
augmentRuntimeError,
25+
cyclicDependencyError,
26+
getRuntimeErrorCode,
27+
prependTokenToDependencyPath,
2528
throwInvalidProviderError,
2629
throwMixedMultiProviderError,
2730
} from '../render3/errors_di';
@@ -37,10 +40,8 @@ import {InjectionToken} from './injection_token';
3740
import type {Injector} from './injector';
3841
import {
3942
BackwardsCompatibleInjector,
40-
catchInjectorError,
4143
convertToBitFlags,
4244
injectArgs,
43-
NG_TEMP_TOKEN_PATH,
4445
setCurrentInjector,
4546
THROW_IF_NOT_FOUND,
4647
ɵɵinject,
@@ -379,20 +380,33 @@ export class R3Injector extends EnvironmentInjector implements PrimitivesInjecto
379380
? null
380381
: notFoundValue;
381382
return nextInjector.get(token, notFoundValue);
382-
} catch (e: any) {
383-
if (isNotFound(e)) {
384-
// @ts-ignore
385-
const path: any[] = (e[NG_TEMP_TOKEN_PATH] = e[NG_TEMP_TOKEN_PATH] || []);
386-
path.unshift(stringify(token));
383+
} catch (error: any) {
384+
// If there was a cyclic dependency error or a token was not found,
385+
// an error is thrown at the level where the problem was detected.
386+
// The error propagates up the call stack and the code below appends
387+
// the current token into the path. As a result, the full path is assembled
388+
// at the very top of the call stack, so the final error message can be
389+
// formatted to include that path.
390+
const errorCode = getRuntimeErrorCode(error);
391+
if (
392+
errorCode === RuntimeErrorCode.CYCLIC_DI_DEPENDENCY ||
393+
errorCode === RuntimeErrorCode.PROVIDER_NOT_FOUND
394+
) {
395+
if (!ngDevMode) {
396+
throw new RuntimeError(errorCode, null);
397+
}
398+
399+
prependTokenToDependencyPath(error, token);
400+
387401
if (previousInjector) {
388402
// We still have a parent injector, keep throwing
389-
throw e;
403+
throw error;
390404
} else {
391405
// Format & throw the final error message when we don't have any previous injector
392-
return catchInjectorError(e, token, 'R3InjectorError', this.source);
406+
throw augmentRuntimeError(error, this.source);
393407
}
394408
} else {
395-
throw e;
409+
throw error;
396410
}
397411
} finally {
398412
// Lastly, restore the previous injection context.
@@ -501,7 +515,7 @@ export class R3Injector extends EnvironmentInjector implements PrimitivesInjecto
501515
const prevConsumer = setActiveConsumer(null);
502516
try {
503517
if (record.value === CIRCULAR) {
504-
throwCyclicDependencyError(stringify(token));
518+
throw cyclicDependencyError(stringify(token));
505519
} else if (record.value === NOT_YET) {
506520
record.value = CIRCULAR;
507521

packages/core/src/render3/di.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@ import {
2626
setInjectorProfilerContext,
2727
} from './debug/injector_profiler';
2828
import {getFactoryDef} from './definition_factory';
29-
import {throwCyclicDependencyError, throwProviderNotFoundError} from './errors_di';
29+
import {
30+
cyclicDependencyError,
31+
cyclicDependencyErrorWithDetails,
32+
throwProviderNotFoundError,
33+
} from './errors_di';
3034
import {NG_ELEMENT_ID, NG_FACTORY_DEF} from './fields';
3135
import {registerPreOrderHooks} from './hooks';
3236
import {AttributeMarker} from './interfaces/attribute_marker';
@@ -716,6 +720,11 @@ export function locateDirectiveOrProvider<T>(
716720
return null;
717721
}
718722

723+
/**
724+
* Used in ngDevMode to keep the injection path in case of cycles in DI.
725+
*/
726+
let injectionPath: string[] = [];
727+
719728
/**
720729
* Retrieve or instantiate the injectable from the `LView` at particular `index`.
721730
*
@@ -734,8 +743,14 @@ export function getNodeInjectable(
734743
const tData = tView.data;
735744
if (value instanceof NodeInjectorFactory) {
736745
const factory: NodeInjectorFactory = value;
746+
ngDevMode && injectionPath.push(factory.name ?? 'unknown');
737747
if (factory.resolving) {
738-
throwCyclicDependencyError(stringifyForError(tData[index]));
748+
const token = stringifyForError(tData[index]);
749+
if (ngDevMode) {
750+
throw cyclicDependencyErrorWithDetails(token, injectionPath);
751+
} else {
752+
throw cyclicDependencyError(token);
753+
}
739754
}
740755
const previousIncludeViewProviders = setIncludeViewProviders(factory.canSeeViewProviders);
741756
factory.resolving = true;
@@ -788,6 +803,7 @@ export function getNodeInjectable(
788803
setIncludeViewProviders(previousIncludeViewProviders);
789804
factory.resolving = false;
790805
leaveDI();
806+
ngDevMode && (injectionPath = []);
791807
}
792808
}
793809
return value;

packages/core/src/render3/di_setup.ts

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9+
import {InjectionToken} from '../di';
910
import {resolveForwardRef} from '../di/forward_ref';
1011
import {InternalInjectFlags} from '../di/interface/injector';
1112
import {ClassProvider, Provider} from '../di/interface/provider';
@@ -116,8 +117,12 @@ function resolveProvider(
116117
tNode.providerIndexes >> TNodeProviderIndexes.CptViewProvidersCountShift;
117118

118119
if (isTypeProvider(provider) || !provider.multi) {
119-
// Single provider case: the factory is created and pushed immediately
120-
const factory = new NodeInjectorFactory(providerFactory, isViewProvider, ɵɵdirectiveInject);
120+
const factory = new NodeInjectorFactory(
121+
providerFactory,
122+
isViewProvider,
123+
ɵɵdirectiveInject,
124+
ngDevMode ? providerName(provider) : null,
125+
);
121126
const existingFactoryIndex = indexOf(
122127
token,
123128
tInjectables,
@@ -205,6 +210,7 @@ function resolveProvider(
205210
isViewProvider,
206211
isComponent,
207212
providerFactory,
213+
provider,
208214
);
209215
if (!isViewProvider && doesViewProvidersFactoryExist) {
210216
lInjectablesBlueprint[existingViewProvidersFactoryIndex].providerFactory = factory;
@@ -394,11 +400,38 @@ function multiFactory(
394400
isViewProvider: boolean,
395401
isComponent: boolean,
396402
f: () => any,
403+
provider: Provider,
397404
): NodeInjectorFactory {
398-
const factory = new NodeInjectorFactory(factoryFn, isViewProvider, ɵɵdirectiveInject);
405+
const factory = new NodeInjectorFactory(
406+
factoryFn,
407+
isViewProvider,
408+
ɵɵdirectiveInject,
409+
ngDevMode ? providerName(provider) : null,
410+
);
399411
factory.multi = [];
400412
factory.index = index;
401413
factory.componentProviders = 0;
402414
multiFactoryAdd(factory, f, isComponent && !isViewProvider);
403415
return factory;
404416
}
417+
418+
function providerName(provider: Provider): string | null {
419+
if (Array.isArray(provider)) {
420+
return null;
421+
}
422+
if (isTypeProvider(provider)) {
423+
return provider.name;
424+
} else if (isClassProvider(provider)) {
425+
if (provider.provide instanceof InjectionToken) {
426+
return `('${provider.provide.toString()}':${provider.useClass.name})`;
427+
}
428+
429+
return provider.useClass.name;
430+
} else if (provider.provide instanceof InjectionToken) {
431+
return provider.provide.toString();
432+
} else if (typeof provider.provide === 'string') {
433+
return provider.provide;
434+
} else {
435+
return null;
436+
}
437+
}

packages/core/src/render3/errors_di.ts

Lines changed: 102 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,27 @@
88

99
import type {ProviderToken} from '../di';
1010
import {isEnvironmentProviders} from '../di/interface/provider';
11-
import {RuntimeError, RuntimeErrorCode} from '../errors';
11+
import {formatRuntimeError, RuntimeError, RuntimeErrorCode} from '../errors';
1212
import {Type} from '../interface/type';
13+
import {assertDefined} from '../util/assert';
14+
import {getClosureSafeProperty} from '../util/property';
1315
import {stringify} from '../util/stringify';
1416

1517
import {stringifyForError} from './util/stringify_utils';
1618

17-
/** Called when directives inject each other (creating a circular dependency) */
18-
export function throwCyclicDependencyError(token: string, path?: string[]): never {
19-
throw new RuntimeError(
20-
RuntimeErrorCode.CYCLIC_DI_DEPENDENCY,
21-
ngDevMode
22-
? `Circular dependency in DI detected for ${token}${path ? `. Dependency path: ${path.join(' > ')} > ${token}` : ''}`
23-
: token,
24-
);
19+
const NG_RUNTIME_ERROR_CODE = getClosureSafeProperty({'ngErrorCode': getClosureSafeProperty});
20+
const NG_RUNTIME_ERROR_MESSAGE = getClosureSafeProperty({'ngErrorMessage': getClosureSafeProperty});
21+
const NG_TOKEN_PATH = getClosureSafeProperty({'ngTokenPath': getClosureSafeProperty});
22+
23+
/** Creates a circular dependency runtime error. */
24+
export function cyclicDependencyError(token: string, path?: string[]): Error {
25+
const message = ngDevMode ? `Circular dependency detected for \`${token}\`.` : '';
26+
return createRuntimeError(message, RuntimeErrorCode.CYCLIC_DI_DEPENDENCY, path);
27+
}
28+
29+
/** Creates a circular dependency runtime error including a dependency path in the error message. */
30+
export function cyclicDependencyErrorWithDetails(token: string, path: string[]): Error {
31+
return augmentRuntimeError(cyclicDependencyError(token, path), null);
2532
}
2633

2734
export function throwMixedMultiProviderError() {
@@ -67,3 +74,89 @@ export function throwProviderNotFoundError(
6774
`No provider for ${stringifyForError(token)} found${injectorName ? ` in ${injectorName}` : ''}`;
6875
throw new RuntimeError(RuntimeErrorCode.PROVIDER_NOT_FOUND, errorMessage);
6976
}
77+
78+
/**
79+
* Given an Error instance and the current token - update the monkey-patched
80+
* dependency path info to include that token.
81+
*
82+
* @param error Current instance of the Error class.
83+
* @param token Extra token that should be appended.
84+
*/
85+
export function prependTokenToDependencyPath(
86+
error: any,
87+
token: ProviderToken<unknown> | {multi: true; provide: ProviderToken<unknown>},
88+
): void {
89+
error[NG_TOKEN_PATH] ??= [];
90+
// Append current token to the current token path. Since the error
91+
// is bubbling up, add the token in front of other tokens.
92+
const currentPath = error[NG_TOKEN_PATH];
93+
// Do not append the same token multiple times.
94+
let pathStr: string;
95+
if (typeof token === 'object' && 'multi' in token && token?.multi === true) {
96+
assertDefined(token.provide, 'Token with multi: true should have a provide property');
97+
pathStr = stringifyForError(token.provide);
98+
} else {
99+
pathStr = stringifyForError(token);
100+
}
101+
102+
if (currentPath[0] !== pathStr) {
103+
(error[NG_TOKEN_PATH] as string[]).unshift(pathStr);
104+
}
105+
}
106+
107+
/**
108+
* Modifies an Error instance with an updated error message
109+
* based on the accumulated dependency path.
110+
*
111+
* @param error Current instance of the Error class.
112+
* @param source Extra info about the injector which started
113+
* the resolution process, which eventually failed.
114+
*/
115+
export function augmentRuntimeError(error: any, source: string | null): Error {
116+
const tokenPath: string[] = error[NG_TOKEN_PATH];
117+
const errorCode = error[NG_RUNTIME_ERROR_CODE];
118+
const message = error[NG_RUNTIME_ERROR_MESSAGE] || error.message;
119+
error.message = formatErrorMessage(message, errorCode, tokenPath, source);
120+
return error;
121+
}
122+
123+
/**
124+
* Creates an initial RuntimeError instance when a problem is detected.
125+
* Monkey-patches extra info in the RuntimeError instance, so that it can
126+
* be reused later, before throwing the final error.
127+
*/
128+
export function createRuntimeError(message: string, code: number, path?: string[]): Error {
129+
// Cast to `any`, so that extra info can be monkey-patched onto this instance.
130+
const error = new RuntimeError(code, message) as any;
131+
132+
// Monkey-patch a runtime error code and a path onto an Error instance.
133+
error[NG_RUNTIME_ERROR_CODE] = code;
134+
error[NG_RUNTIME_ERROR_MESSAGE] = message;
135+
if (path) {
136+
error[NG_TOKEN_PATH] = path;
137+
}
138+
return error;
139+
}
140+
141+
/**
142+
* Reads monkey-patched error code from the given Error instance.
143+
*/
144+
export function getRuntimeErrorCode(error: any): number | undefined {
145+
return error[NG_RUNTIME_ERROR_CODE];
146+
}
147+
148+
function formatErrorMessage(
149+
text: string,
150+
code: number,
151+
path: string[] = [],
152+
source: string | null = null,
153+
): string {
154+
let pathDetails = '';
155+
// If the path is empty or contains only one element (self) -
156+
// do not append additional info the error message.
157+
if (path && path.length > 1) {
158+
pathDetails = ` Path: ${path.join(' -> ')}.`;
159+
}
160+
const sourceDetails = source ? ` Source: ${source}.` : '';
161+
return formatRuntimeError(code, `${text}${sourceDetails}${pathDetails}`);
162+
}

0 commit comments

Comments
 (0)