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

Discussion: Bluebird promises, rejecting with non-error objects, "Warning: a promise was rejected with a non-error" in log #329

Closed
valichek opened this issue Apr 17, 2016 · 6 comments

Comments

@valichek
Copy link

Here is a reason:
https://github.com/aurelia/router/blob/master/src/navigation-plan.js#L34

export function _buildNavigationPlan(instruction: NavigationInstruction, forceLifecycleMinimum): Promise<Object> {
  ...
  if ('redirect' in config) {
    ...
    return Promise.reject(new Redirect(redirectLocation));
  }

Here is explanation of warning.
https://github.com/petkaantonov/bluebird/blob/master/docs/docs/warning-explanations.md#warning-a-promise-was-rejected-with-a-non-error

Due to a historic mistake in JavaScript, the throw statement is allowed to be used with any value, not just errors, and Promises/A+ choosing to inherit this mistake, it is possible to reject a promise with a value that is not an error.

An error is an object that is a instanceof Error. It will at minimum have the properties .stack and .message, which are an absolute must to have for any value that is being used in an automatic propagation mechanism such as exceptions and rejections. This is because errors are usually handled many levels above from where they actually originated from - the error object must have sufficient metadata about it so that its ultimate handler many levels above will have all the information needed for creating a useful high level error report.

Since all objects support having properties you might still wonder why exactly does it have to be an error object and not just any object. In addition to supporting properties, an equally important feature necessary for values that are automatically propagated is the stack trace property (.stack). A stack trace allows you easily find where an error originated from.

You should heed this warning because rejecting a promise with a non-error makes debugging extremely hard and costly. Additionally, if you reject with simple primitives such as undefined (commonly caused by simple calling reject()) you cannot handle errors at all because it's impossible to tell from undefined what exactly went wrong. All you can tell the user is that "something went wrong" and lose them forever.

@jwahyoung
Copy link
Contributor

@valichek I don't think this is something that we can honor as we use promises for flow control in this case. Further, our API is stabilizing and this would be a breaking change.

@EisenbergEffect I'm tagging this with the wontfix tag. Let me know if we should change this.

@shaunluttin
Copy link

Is the official recommendation from Aurelia simply to ignore these warnings?

@pranuel
Copy link

pranuel commented Apr 11, 2018

This warning is bugging us as well.
I think it is a bad idea to use exceptions (by rejecting a promise) to control the application flow.
Especially if this results in such a warning on the console.
We use Aurelia to build nice and solid apps for our customers - having warnings on the browser's console saying that the app misused promises doesn't really fit in that picture...
I vote for fixing this as well!

@Alexander-Taran
Copy link
Contributor

Alexander-Taran commented Apr 16, 2018

chrome
image
edge
image

@stevies
Copy link

stevies commented Sep 22, 2018

I think a similar issue got fixed in the latest release of the http client - https://github.com/aurelia/http-client/releases/tag/1.3.0

Can a similar fix be added to the router? This error polluting the debug console is very annoying and masks possible real errors.

@ricardograca
Copy link

@davismj What's the original issue this one is a duplicate of?

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

8 participants