Skip to content

Commit

Permalink
[BUGFIX LTS] Improve error for owner methods called after destroy
Browse files Browse the repository at this point in the history
The previous error handling gives you a stack trace and a notice, but
does not tell you what was being asked for with the lookup. This makes
the traces much less helpful, especially for minified/prod builds.
  • Loading branch information
chriskrycho committed Feb 21, 2023
1 parent 7e48201 commit 8b45f3f
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
6 changes: 4 additions & 2 deletions packages/@ember/-internals/container/lib/container.ts
Expand Up @@ -152,7 +152,7 @@ export default class Container {
options?: RegisterOptions
): InternalFactory<object> | object | undefined {
if (this.isDestroyed) {
throw new Error(`Cannot call \`.lookup\` after the owner has been destroyed`);
throw new Error(`Cannot call \`.lookup('${fullName}')\` after the owner has been destroyed`);
}
assert('fullName must be a proper full name', this.registry.isValidFullName(fullName));
return lookup(this, this.registry.normalize(fullName), options);
Expand Down Expand Up @@ -217,7 +217,9 @@ export default class Container {
*/
factoryFor(fullName: FullName): InternalFactoryManager<object> | undefined {
if (this.isDestroyed) {
throw new Error(`Cannot call \`.factoryFor\` after the owner has been destroyed`);
throw new Error(
`Cannot call \`.factoryFor('${fullName}')\` after the owner has been destroyed`
);
}
let normalizedName = this.registry.normalize(fullName);

Expand Down
4 changes: 2 additions & 2 deletions packages/@ember/-internals/container/tests/container_test.js
Expand Up @@ -608,7 +608,7 @@ moduleFor(

assert.throws(() => {
container.lookup('service:foo');
}, /Cannot call `.lookup` after the owner has been destroyed/);
}, /Cannot call `.lookup('service:foo')` after the owner has been destroyed/);
}

[`@test assert when calling factoryFor after destroy on a container`](assert) {
Expand All @@ -626,7 +626,7 @@ moduleFor(

assert.throws(() => {
container.factoryFor('service:foo');
}, /Cannot call `.factoryFor` after the owner has been destroyed/);
}, /Cannot call `.factoryFor('service:foo')` after the owner has been destroyed/);
}

// this is skipped until templates and the glimmer environment do not require `OWNER` to be
Expand Down

0 comments on commit 8b45f3f

Please sign in to comment.