Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

feat($q): add shorthand for defining promise success or error handlers #3476

Closed
wants to merge 2 commits into from

Conversation

bolasblack
Copy link
Contributor

Now we can instead this

promise.then(null, errorHandler)

like this

promise.catch(errorhandler)

Closes #2048

@petebacondarwin
Copy link
Contributor

@bolasblack - thanks for the PR.

I notice that you have a commit that modifies the build on travis. bower is currently installed locally using the version specified in package.json and is called as part of the grunt build. Are you aware of this? I am not sure that this fix is necessarily the solution to our recent flakiness in travis builds.

Also the done() function has a slightly different meaning in Q - it is about causing exceptions to be exposed. In any case this method as it stands is pointless as the second parameter to then() is optional so you can do:

promise.then(successHandler);

@bolasblack
Copy link
Contributor Author

@petebacondarwin i think you are right, so i removed the done method, and i found that i missed bower in package.json too, so i amend the commit, it will not install bower globally now.

Thanks for your reminding.

@petebacondarwin
Copy link
Contributor

Great. Thanks

@pkozlowski-opensource
Copy link
Member

@bolasblack bolasblack@f53f64a shouldn't be part of this PR, right?

@pkozlowski-opensource
Copy link
Member

Cleaned up commit with updated docs here:
matsko@265b6c3

@bolasblack
Copy link
Contributor Author

@pkozlowski-opensource if this commit bolasblack/angular.js@f53f64a not exist, the PR will test failed.

@IgorMinar
Copy link
Contributor

fail is deprecated in Q and provides catch instead. So does the new DOM Promise spec

@bolasblack
Copy link
Contributor Author

@IgorMinar it's ok now~

@bolasblack
Copy link
Contributor Author

@IgorMinar I amended the commit :)

@bolasblack
Copy link
Contributor Author

@IgorMinar i don't know what happen, i don't kown why the test failed...

WARN [proxy]: failed to proxy /build/docs/index-nocache.html ([object Object])
java.net.SocketException: Unexpected end of file from server
    at sun.net.www.http.HttpClient.parseHTTPHeader(HttpClient.java:718)
    at sun.net.www.http.HttpClient.parseHTTP(HttpClient.java:579)
    at sun.net.www.http.HttpClient.parseHTTPHeader(HttpClient.java:715)
    at sun.net.www.http.HttpClient.parseHTTP(HttpClient.java:579)
    at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1322)
    at java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:468)
    at com.saucelabs.sauceconnect.proxy.ProxyHandler.proxyPlainTextRequest(ProxyHandler.scala:422)
    at com.saucelabs.sauceconnect.proxy.ProxyHandler.handle(ProxyHandler.scala:113)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:114)
    at org.eclipse.jetty.server.Server.handle(Server.java:352)
    at org.eclipse.jetty.server.HttpConnection.handleRequest(HttpConnection.java:596)
    at org.eclipse.jetty.server.HttpConnection$RequestHandler.headerComplete(HttpConnection.java:1052)
    at org.eclipse.jetty.http.HttpParser.parseNext(HttpParser.java:590)
    at org.eclipse.jetty.http.HttpParser.parseAvailable(HttpParser.java:212)
    at org.eclipse.jetty.server.HttpConnection.handle(HttpConnection.java:426)
    at org.eclipse.jetty.io.nio.SelectChannelEndPoint.handle(SelectChannelEndPoint.java:510)
    at org.eclipse.jetty.io.nio.SelectChannelEndPoint.access$000(SelectChannelEndPoint.java:34)
    at org.eclipse.jetty.io.nio.SelectChannelEndPoint$1.run(SelectChannelEndPoint.java:40)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$2.run(QueuedThreadPool.java:450)
    at java.lang.Thread.run(Thread.java:724)
Chrome 27.0.1453 (Windows XP) angular+jqlite api/ngSanitize.filter:linky should linkify the snippet with urls FAILED
    select binding 'snippet | linky'
    /home/travis/build/angular/angular.js/build/docs/docs-scenario.js:73:14: Selector #linky-filter did not match any elements.

@lrlopez
Copy link
Contributor

lrlopez commented Aug 9, 2013

May be there is nothing wrong on your code. Travis behavior is being flaky for some time. You can retrigger Travis testing by amending the commit and pushing again to your repository with the -f option to force the update.

By the way, it may be advisable to rebase the PR and take the chore(bower) commit away.

Now we can instead this

    promise.then(null, errorHandler)

with this

    promise.catch(errorhandler)

Closes angular#2048
@bolasblack
Copy link
Contributor Author

@lrlopez thanks for your help, i trid your method, but i'm afraid it did not work for me...

@lrlopez
Copy link
Contributor

lrlopez commented Aug 9, 2013

Sorry to hear that... Seems that E2E tests via Travis are broken in current master. I've found another PR unrelated to yours with the same error.

@IgorMinar IgorMinar closed this in a207665 Aug 9, 2013
@IgorMinar
Copy link
Contributor

travis is totally borked right now.. looks like a node.js issue, but we can't reliably reproduce it outside of travis :-(

I merged the $q commit after some minor modifications. thanks!

@bolasblack bolasblack deleted the shorthandForPromise branch August 9, 2013 16:54
@bolasblack
Copy link
Contributor Author

@IgorMinar thanks for merging this.

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

Successfully merging this pull request may close these issues.

Shorthand for defining promise error handlers - $q.fail()
5 participants