Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors during model hooks should emit to Ember.onerror #18533

Closed
rwjblue opened this issue Oct 30, 2019 · 4 comments
Closed

Errors during model hooks should emit to Ember.onerror #18533

rwjblue opened this issue Oct 30, 2019 · 4 comments

Comments

@rwjblue
Copy link
Member

rwjblue commented Oct 30, 2019

Currently, when a model hook rejects we bubble up the route hierarchy looking for any route that implements an error action. If we get to the top (above application route) we have some default behavior for the error action:

// Attempt to find an appropriate error route or substate to enter.
error(routeInfos: PrivateRouteInfo[], error: Error, transition: Transition) {
let router: any = this;
let routeInfoWithError = routeInfos[routeInfos.length - 1];
forEachRouteAbove(routeInfos, (route: Route, routeInfo: PrivateRouteInfo) => {
// We don't check the leaf most routeInfo since that would
// technically be below where we're at in the route hierarchy.
if (routeInfo !== routeInfoWithError) {
// Check for the existence of an 'error' route.
let errorRouteName = findRouteStateName(route, 'error');
if (errorRouteName) {
router._markErrorAsHandled(error);
router.intermediateTransitionTo(errorRouteName, error);
return false;
}
}
// Check for an 'error' substate route
let errorSubstateName = findRouteSubstateName(route, 'error');
if (errorSubstateName) {
router._markErrorAsHandled(error);
router.intermediateTransitionTo(errorSubstateName, error);
return false;
}
return true;
});
logError(error, `Error while processing route: ${transition.targetName}`);
},

Which invokes logError, defined here:

function logError(_error: any, initialMessage: string) {
let errorArgs = [];
let error;
if (_error && typeof _error === 'object' && typeof _error.errorThrown === 'object') {
error = _error.errorThrown;
} else {
error = _error;
}
if (initialMessage) {
errorArgs.push(initialMessage);
}
if (error) {
if (error.message) {
errorArgs.push(error.message);
}
if (error.stack) {
errorArgs.push(error.stack);
}
if (typeof error === 'string') {
errorArgs.push(error);
}
}
console.error(...errorArgs); //eslint-disable-line no-console
}

As you can see, logError is a small layer on top of console.error. Unfortunately, this means that any errors during model hooks will not be sent to Ember.onerror (unless you also implement error on your application route).

We should fix this by checking if there is an Ember.onerror and invoking it with the error, falling back to logError.

@xg-wang
Copy link
Contributor

xg-wang commented Nov 6, 2019

The final error handling after logError is at L1610 promise chain. There if the error is not "handled" by any

  • route error actions
  • <route>.error state
  • <route>-error substate
    It is part of RSVP Promise rejection, which goes into Ember.onerror. The wrapper around console.error always executes.

function didBeginTransition(transition: Transition, router: EmberRouter) {
let routerState = new RouterState(router, router._routerMicrolib, transition[STATE_SYMBOL]!);
if (!router.currentState) {
router.set('currentState', routerState);
}
router.set('targetState', routerState);
transition.promise = transition.catch((error: any) => {
if (router._isErrorHandled(error)) {
router._clearHandledError(error);
} else {
throw error;
}
}, 'Transition Error');
}

@rwjblue are you suggesting the error should be thrown in the case that it's handled, but the action that handled the error specified to bubble it up?

@houfeng0923
Copy link

hi @rwjblue , i have other question about onerror:

when use async and catch a reject promise(rsvp, not native), always an error in console.
unless i use native promise instead .

https://ember-twiddle.com/8c7e71eeab8de3ca2a48345bc1058eb9?openFiles=routes.application%5C.js%2C

in ember, if rsvp Promise has no catch handler, it look has a default handler in development ? i have to use Ember.onerror to suppress it .

@rwjblue
Copy link
Member Author

rwjblue commented Jul 22, 2020

@houfeng0923 - emberjs/ember-qunit#592 is a discussion about using native promises for these kinds of things

@rwjblue rwjblue closed this as completed Jul 22, 2020
@Turbo87
Copy link
Member

Turbo87 commented Sep 21, 2020

@rwjblue was the original issue resolved? I might be doing something wrong, but I'm still seeing the error in the logs of my test suite 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants