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

'undefined is not a function' when promise function not defined #11330

Closed
olore opened this Issue Mar 16, 2015 · 10 comments

Comments

Projects
None yet
7 participants
@olore
Copy link

commented Mar 16, 2015

Using 1.4.0-beta.5, the following stack trace seems to be the result of an error() function not being defined during promise rejection. The rejection still bubbles up, however the "undefined is not a function" is logged in the console.

http://plnkr.co/edit/JtaNeNEcxFLPH5ByuHI8

TypeError: undefined is not a function
at angular.js:9564
at processQueue (angular.js:14018)
at angular.js:14034
at Scope.$get.Scope.$eval (angular.js:15236)
at Scope.$get.Scope.$digest (angular.js:15051)
at Scope.$get.Scope.$apply (angular.js:15341)
at done (angular.js:9847)
at completeRequest (angular.js:10037)
at XMLHttpRequest.requestLoaded (angular.js:9978)
@jbedard

This comment has been minimized.

Copy link
Contributor

commented Mar 16, 2015

That is because the httpError variable is undefined until the assignment takes place (which is after calling .error. That is the same as doing $http(...).error(undefined).

See function hoisting vs var hoisting.

@petebacondarwin

This comment has been minimized.

Copy link
Member

commented Mar 16, 2015

@jbedard is right. Closing as not a valid issue

@IgorMinar

This comment has been minimized.

Copy link
Member

commented Mar 16, 2015

The problem is that the error is not nice at all and since it's thrown usually asynchronously with respect to where the callback was registered, it's really hard to debug.

Could we just silently ignore, if the callback is not a function? Would this break Promises A+ spec?

a little bit more context: https://twitter.com/olore/status/577277479506837504

@petebacondarwin

This comment has been minimized.

Copy link
Member

commented Mar 16, 2015

OK, so this is hard to debug but it is a JS development error not a failure in Angular. Wherever there is code that takes a callback, you have to have defined the callback before you pass it to the code.

Silently ignoring if the callback is not a function will make this even more difficult to debug! There won't even be an error but the error callback will silently not be called if the HTTP request errors.

I think we could throw an error if you call .error() with a thing that is not a function. For example in this case the code is effectively calling .error(undefined), which is not what the error() method expects.

@petebacondarwin

This comment has been minimized.

Copy link
Member

commented Mar 16, 2015

I created a PR with this in mind. @olore and @IgorMinar is this an acceptable fix?

Now if you fail to pass in a valid function to either error or success you get an instant error.

petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Mar 16, 2015

@stoltene2

This comment has been minimized.

Copy link

commented Mar 16, 2015

I think this will give more clarity to the situation.

@pkozlowski-opensource

This comment has been minimized.

Copy link
Member

commented Mar 16, 2015

So, if we want to keep those callbacks close to promises callbacks we should ignore non-function arguments: https://promisesaplus.com/#point-23

@petebacondarwin

This comment has been minimized.

Copy link
Member

commented Mar 16, 2015

The only reason the promise spec allows us to ignore params is because they are all optional but I am pretty sure that passing no parameters is always an error. In the case of error and success there is only one parameter, so it never makes sense to fail to pass the parameter.

@pkozlowski-opensource

This comment has been minimized.

Copy link
Member

commented Mar 16, 2015

Well, the spec says "if it is not a function", so I'm not sure it is only about optional params

@Narretz Narretz added this to the Backlog milestone Mar 17, 2015

petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Mar 17, 2015

petebacondarwin added a commit that referenced this issue Mar 17, 2015

@olore

This comment has been minimized.

Copy link
Author

commented Mar 28, 2015

Guys thanks for seeing this one through.
@IgorMinar appreciate you opening this one back up and describing our twitter convo more clearly

netman92 added a commit to netman92/angular.js that referenced this issue Aug 8, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.