1

I am putting catches at the end, but they are returning empty object in one particular instance at least. Is a catch necessary for anything unbeknownst, or is it just screwing me up?

$( document).ready(function(){
    app.callAPI()//a chainable a RSVP wrapper around a jquery call, with its own success() fail() passing forward to the wrapper, so it will either be a resolved or rejected thenable to which is now going to be chained 
        .then(
            function(env) {
                //set the property you needed now
                app.someSpecialEnvObj = env;
            },
            function(rejectMessage){
                console.log('call.API() cant set some special env object..');
                console.log(rejectMessage);
            }
        )
        .catch(
        function(rejectMessage){
            if(rejectMessage){
                //a just incase, DOES IT HAVE VALUE, for somebody that may have not done their homework in the parent calls?
                console.log('you have some kind of legitimate error, maybe even in callAPI() that is not part of any problems inside them.  you may have forgotton handle something at an early state, your so lucky this is here!)
            } else {
                console.log('can this, and or will this ever run.  i.e., is there any value to it, when the necessity to already be logging is being handled in each and every then already, guaranteeing that we WONT be missing ANYTHING')
            }
        }
    );
});

Is it wrong? or is there some kind of use for it, even when I still use an error/reject handler on all usages of .then(resolve, reject) methods in all parent chained then-ables?

EDIT: Better code example, I hope. I think I might be still be using some kind of anti-pattern in the naming, I rejectMessage in my e.g., it's the jqXhr object right?

So maybe I should be naming them exactly that or what? i.e. jqXhr? By the way, the reason I like to reject it on the spot inside each then(), if there was an error, is because this way I can copiously log each individual call, if there was a problem specifically there, that way I don't have to track anything down. Micro-logging, because I can.

Promises are helping opening up the world of debugging this way.

Here's the three examples I have tried. I prefer method1, and method2, and by no means am I going back to method3, which is where I started off in the promise land.

//method 1
app.rsvpAjax = function (){
    var async,
        promise = new window.RSVP.Promise(function(resolve, reject){
            async = $.extend( true, {},app.ajax, {
                success: function(returnData) {
                    resolve(returnData);
                },
                error: function(jqXhr, textStatus, errorThrown){
                    console.log('async error');
                    console.log({jqXhr:  jqXhr, textStatus: textStatus, errorThrown: errorThrown});
                    reject({ jqXhr: jqXhr, textStatus: textStatus, errorThrown: errorThrown}); //source of promise catch data believe
                }
            });
            $.ajax(async); //make the call using jquery's ajax, passing it our reconstructed object, each and every time
        });
    return promise;
};

app.callAPI = function () {
    var promise =app.rsvpAjax();
    if ( !app.ajax.url ) {
        console.log("need ajax url");
        promise.reject(); //throw (reject now)
    }
    return promise;
};

//method 2
app.ajaxPromise = function(){
    var  promise,  url = app.ajax.url;
    var coreObj = { //our XMLHttpRequestwrapper object
        ajax : function (method, url, args) {  // Method that performs the ajax request
            promise = window.RSVP.Promise( function (resolve, reject) {    // Creating a promise
                var client = new XMLHttpRequest(),  // Instantiates the XMLHttpRequest
                    uri = url;
                uri = url;
                if (args && (method === 'POST' || method === 'PUT')) {
                    uri += '?';
                    var argcount = 0;
                    for (var key in args) {
                        if (args.hasOwnProperty(key)) {
                            if (argcount++) {
                                uri += '&';
                            }
                            uri += encodeURIComponent(key) + '=' + encodeURIComponent(args[key]);
                        }
                    }
                }
                client.open(method, uri);
                client.send();
                client.onload = function () {
                    if (this.status == 200) {
                        resolve(this.response);   // Performs the function "resolve" when this.status is equal to 200
                    }
                    else {
                        reject(this.statusText); // Performs the function "reject" when this.status is different than 200
                    }
                };

                client.onerror = function () {
                    reject(this.statusText);
                };
            });
            return promise;   // Return the promise
        }
    };
    // Adapter pattern
    return {
        'get' : function(args) {
            return coreObj.ajax('GET', url, args);
        },
        'post' : function(args) {
            return coreObj.ajax('POST', url, args);
        },
        'put' : function(args) {
            return coreObj.ajax('PUT', url, args);
        },
        'delete' : function(args) {
            return coreObj.ajax('DELETE', url, args);
        }
    };
};

app.callAPI = function () {
    var async, callback;
    async =app.ajaxPromise() ; //app.ajaxPromise() is what creates the RSVP PROMISE HERE<
    if(app.ajax.type === 'GET'){async = async.get();}
    else if(app.ajax.type === 'POST') {async = async.post();}
    else if(app.ajax.type === 'PUT'){async = async.put();}
    else if(app.ajax.type === 'DELETE'){ async = async.delete();}
    callback = {
        success: function (data) {
            return JSON.parse(data);
        },
        error: function (reason) {
            console.log('something went wrong here');
            console.log(reason);
        }
    };
    async = async.then(callback.success)
        .catch(callback.error);
    return async;
};

//method 3 using old school jquery deferreds
app.callAPI = function () {
    //use $.Deferred instead of RSVP
    async = $.ajax( app.ajax) //run the ajax call now
        .then(
        function (asyncData) { //call has been completed, do something now
            return asyncData;  //modify data if needed, then return, sweet success
        },
        function(rejectMessage) {  //call failed miserably, log this thing
            console.log('Unexpected error inside the callApi.  There was a fail in the $.Deferred ajax call');
            return rejectMessage;
        }
    );
    return async;
};

I also run this somewhere onready as another backup.

window.RSVP.on('error', function(error) {
    window.console.assert(false, error);
    var response;
    if(error.jqXhr){
        response = error.jqXhr;
    } else {
        //response = error;
        response = 'is this working yet?';
    }
    console.log('rsvp_on_error_report')
    console.log(response);
});

Edit error examples

//one weird error I can't understand, an empty string("")?
{
  "jqXhr": {
    "responseText": {
      "readyState": 0,
      "responseText": "",
      "status": 0,
      "statusText": "error"
    },
    "statusText": "error",
    "status": 0
  },
  "textStatus": "error",
  "errorThrown": "\"\""
}
//another wierd one, but this one comes from a different stream,  the RSVP.on('error') function
{
  "readyState": 0,
  "responseText": "",
  "status": 0,
  "statusText": "error"
}
halfer
  • 19,824
  • 17
  • 99
  • 186
blamb
  • 4,220
  • 4
  • 32
  • 50
  • sorry, you didnt give me a chance to finish my question... now what on the edits?? – blamb Jun 17 '15 at 02:03
  • 2
    First, using `resolve` and `reject` as arguments for your `then` handlers is a bit confusing, since these names are usually used as arguments to the function passed to promise constructors. The values passed to the `then` handlers are the value of the promise, or the reason for its failure, so it's probably better to name them accordingly. Anyway, no, you do not need `catch` (or the failure handler to `then`) everywhere. Why would you? The whole point of promises is that failures will flow onward and upward. –  Jun 17 '15 at 02:09
  • Uh, are you using RSVP promises or ES6 promises now? Please tag your question accordingly. – Bergi Jun 17 '15 at 02:45
  • 1
    I wonder how you expect the error reason to be an empty object? – Bergi Jun 17 '15 at 02:47
  • 1
    What do you mean by "*use a reject in all thenables*"? Some non-pseudo code would help to identify your problem, do you have any actual code? – Bergi Jun 17 '15 at 02:49
  • 1
    Have a look at the [answers over here](http://stackoverflow.com/q/28761365/1048572) on how to properly use `catch` with promises – Bergi Jun 17 '15 at 03:01
  • man, thats across the board.. where do i start. i like @torazaburo's the best, hes right, its so that i can convery(to myself) that thsi is whats in the error message. so ill try better at that. @Bergi, Tildeo RSVP, which is Ecma6, isnt it?. and because i was having problems with it being empty all the time, and i wondered if i even needed it. soby adding that condition, it ensures that if it ever contained any error data it will log. i mean (i.e. param 2, the reject method) in all `.then()`'s sorry, more properly stated i guess. – blamb Jun 17 '15 at 03:30
  • I have seen all the best use cases, none of them seem to addres this. i know if the 2nd method in a then is never ran if the first is ran, i know not to use throws, i just want to know, what if something like just setting up the promise in teh first place fails somehow, or if something "unbeknownst" happens, should i have a catch at the end, even if i use the 2nd method in all thens? Im afraid of missing any information at the end that may be haltering my development. – blamb Jun 17 '15 at 03:32
  • Ok, let me make some code for you, a better example. its hard because i have such a deep method base im pulling from here, in my core library, extending this and that off jquery ajax here and rsvp there, my wrapper around that, then all the calls wrapped as methods inside my app object. , not to mention its NDA, so it takes up alot of time for me to not obfuscate things. however, i started with this, and then i remembered all those times i got complaints from SO patrons that im too windy. So i thought i would save you the wind. – blamb Jun 17 '15 at 03:38
  • If you want it, ill add some different code though, because i do want to help you help me. thanks.. – blamb Jun 17 '15 at 03:38
  • @Bergi i updated my code example, i think i have cleared up a bit on what was pretty confusing. let me know. thanks. – blamb Jun 17 '15 at 04:22
  • "*a chainable a RSVP wrapper around a jquery call, with its own success() fail() passing forward to the wrapper*" - that sounds scary. Given that jQuery returns thenables, `Promise.resolve(jQuery.ajax(…))` is enough, no custom wrapper required. – Bergi Jun 17 '15 at 04:48
  • no no no no, im not going back to jQuery Deferreds, what are you talking about? I use RSVP in my app. the only thing i use jquery for is the ajax call itself to my internal api. How do you suggest replacing Deferred, and still have access to async ajax call, with out having to include a special library other than rsvp and jquery? – blamb Jun 17 '15 at 05:08
  • 1
    @BrianThomas: I assumed that by "jQuery call" you meant `$.ajax` which does return a jQuery deferred? You can easily lift that to a proper promise by calling `RSVP.Promise.resolve`, without any "forwarding wrappers". – Bergi Jun 17 '15 at 05:10
  • Ok, maybe thats what im doing. But yes, i do mean $.ajax(), now i know i need to share this piece of code, because i think its generating this mysterious empty object on one particular call. – blamb Jun 17 '15 at 05:11
  • 1
    @BrianThomas: Yes, that's quite likely - one of the common pitfalls of the [explicit construction antipattern](http://stackoverflow.com/q/23803743/1048572) – Bergi Jun 17 '15 at 05:12
  • 1
    @BrianThomas: Hm, that seems to be fine actually, if you need those `textStatus` and `errorThrown` parameters. Now the only thing thing that would trigger your `.catch()` handler without a message would be if one of the callbacks does something like `throw null` – Bergi Jun 17 '15 at 05:28
  • ok, the code is fully updated, i think werer talking about method 1 right now, in my e.g. im notirious for forgetting returns in promises also, so did i forget any returns in any of my first two examples? i think i returned the rejection in some places, and didnt in others. – blamb Jun 17 '15 at 05:39
  • @Bergi When you say throw null in your last message, you mean a real `throw;` function call which may be broken? or some other kind of throw equivalent, like a reject. i kept thinking it was my callApi() returning an empty json object with no data, from a bad doctrine update, completely empty. do you think its not that, i.e. is that ruled out already, without knowing more details on that? – blamb Jun 17 '15 at 06:16
  • 1
    @BrianThomas: I judged that from the first snippet you posted only - something seemed to `throw` nothing. But now, seeing your `callAPI` implementation, it seems to have been a bug in there indeed, stemming from the `.reject()`. – Bergi Jun 17 '15 at 07:05
  • Yeah, i think thats why im posting all these questions. lol. So you mean in this reject? `reject({ jqXhr: jqXhr, textStatus: textStatus, errorThrown: errorThrown}); `? or which. i know you mentioned If i need all those, so your saying possible something unecessary might be passing through there? I think the reason i used them all was to see what i can get out of them. If i dont see any useful messages, ill remove them. – blamb Jun 17 '15 at 07:12
  • ok, i think after reading that article you linked, and thinking about this, i'm trying to get the promise to do too much, i am using it for more than just promise error capture, i was using it for capturing any logic errors in all those extra then reject locations... I suppose thats my current error generation point, and why its coming back mainly empty. not sure though. Ill rewrite it a bit to make sure i only use then rejects when necessary (when is that, lol), and just a global catch at the end, along wtih my rsvp on error trap. think thats the right way to go? – blamb Jun 17 '15 at 07:28
  • any chance for a point for the question? it looks good, but net got voted, and stalled at 0. – blamb May 31 '16 at 17:56

3 Answers3

3

I am putting catches at the end

That's the typical position for them - you handle all errors that were occurring somewhere in the chain. It's important not to forget to handle errors at all, and having a catch-all in the end is the recommended practise.

even if I use onreject handlers in all .then(…) calls?

That's a bit odd. Usually all errors are handled in a central location (the catch in the end), but of course if you want you can handle them anywhere and then continue with the chain.

Just make sure to understand the difference between an onreject handler in a then and in a catch, and you can use them freely. Still, the catch in the end is recommended to catch errors in the then callbacks themselves.

they are returning empty object in one particular instance atleast.

Then the promise screwed up - it should never reject without a reason. Seems to be caused be the

if ( !app.ajax.url ) {
    console.log("need ajax url");
    promise.reject();
}

in your code that should have been a

if (!app.ajax.url)
    return Promise.reject("need ajax url");

Is a catch necessary for anything unbeknownst?

Not really. The problem is that catch is usually a catch-all, even catching unexpected exceptions. So if you can distinguish them, what would you do with the unexpected ones?

Usually you'd set up some kind of global unhandled rejection handler for those, so that you do not have to make sure to handle them manually at the end of every promise chain.

Community
  • 1
  • 1
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Why is it a bit odd to use reject callbacks in all thens, i guess its only needed if you want to error out whatever it is you need to do in that `then()` in the first place. if so, then i think i see what you mean.. ? – blamb Jun 17 '15 at 05:50
  • 1
    Yes, it is only needed if you want to handle that specific error. Depending on the nature of your handler, [this can be even wrong](https://github.com/petkaantonov/bluebird/wiki/Promise-anti-patterns#the-thensuccess-fail-anti-pattern), but at least I wouldn't expect to see it in *all* `then` callbacks. – Bergi Jun 17 '15 at 05:55
  • ok, i was going overboard, because i thought maybe it was part of how you were supposed squeak more error messages out of it to build a better picture of whats going on where there is an error, mainly during development obviously. – blamb Jun 17 '15 at 06:00
  • 2
    Not necessarily overboard. I currently work on the basis that mid-chain error handling is justifiable/necessary for - (a) mid-chain error recovery - eg injection of a default value, or to use *progress so far*; (b) injection of custom error messages; (c) specialised flow control, eg a [catch chain](http://stackoverflow.com/questions/30704891/#30707580) which allows break-on-success instead of a chain's natural break-on-error behaviour. – Roamer-1888 Jun 17 '15 at 08:35
  • yes, im heavily hitting it with custom error messages, i have a debug method i built thats a glorified console logger, guaranteed not to fowl up production... :-) and im putting them right in where you are referring to, mid chain, everywhere, but im removing them now as i am seeing better streams coming in from the reject method, which are shoving straight into my db, and incrementing an updated count :-) SO thanks for the justification there. I think ill use those debug messages until i can get all my rejection jqXhr errors going nicely in the right direction. – blamb Jun 17 '15 at 21:10
  • @Roamer-1888 on the progress so far, tell me more.. i dont use any progress methods yet.. OH, and about breaks, i cant figure out how to break it yet, how do you break it properly? Thats been a more difficult part to grasp. im guessing its just a combination of how you set up your rejections, and adding some kind of real `"throw'OOPS!';"` in there? I do need to understand the break, so i can correct all the loss of information im getting here.. – blamb Jun 17 '15 at 21:12
  • "Progress so far" is all about some sequence of async calls that contribute to some cumulative goal. See my answer [here](http://stackoverflow.com/questions/30856549/30880634#30880634), in particular the clause `if (accumulator.length) { return accumulator; }` which ensures that any data that were successfully received prior to an error are not simply discarded once that error has occurred. This is a "partial recovery" stratagem and not the same as chaining `.progress()`. Maybe I should use a different term to avoid confusion. – Roamer-1888 Jun 18 '15 at 00:45
  • "Breaks" is all about exploiting fulfilment/rejection for flow control within a promise chain. The "natural" behaviour is for a chain to progress down the success path as long as successes keep happening, and to skip to some distant fail/catch handler on error, which is pretty simple to grasp. Click the "catch chain" link in my comment above and you will see that the converse can be engineered - namely that a promise chain can be made to progress down its fail path as long as failures keep happening, and to skip to some distant success handler on success. – Roamer-1888 Jun 18 '15 at 00:47
2

I think the general question deserves a simple answer without the example.

The pedantic technical answer is 'no', because .catch(e) is equivalent to .then(null, e).

However (and this is a big "however"), unless you actually pass in null, you'll be passing in something that can fail when it runs (like a coding error), and you'll need a subsequent catch to catch that since the sibling rejection handler by design wont catch it:

.then(onSuccess, onFailure); // onFailure wont catch onSuccess failing!!

If this is the tail of a chain, then (even coding) errors in onSuccess are swallowed up forever. So don't do that.

So the real answer is yes, unless you're returning the chain, in which case no.

All chains should be terminated, but if your code is only a part of a larger chain that the caller will add to after your call returns, then it is up to the caller to terminate it properly (unless your function is designed to never fail).

The rule I follow is: All chains must either be returned or terminated (with a catch).

jib
  • 40,579
  • 17
  • 100
  • 158
  • Ok, so yeah, i basically did exactly that, i am terminating them wtih a catch, no caller is using them after myself, and i removed any catches on anything that return promises, and the last one that presents it to the view, runs that promise chain, and has a catch, once only, at the end of the chain only. :-) I also removed any unnecessary rejects in my then statements as formerly suggested. I only use them when its a promise im passing. also, since this is primarily for development work, its important for the catch. i think this is exactly what your saying. This is feeling better now :-) – blamb Jun 17 '15 at 21:01
  • Also, reading this stuff on terminate , it makes more sense http://making.change.org/post/69613524472/promises-and-error-handling which one do you like better, his former way to terminate using done()(can you even do that? i thought done was for jquery deferred only), or his alternate solution using catch()? – blamb Jun 17 '15 at 21:30
  • `done` is not part of ES6 promises, but is equivalent to `.catch(e => setTimeout(() => { throw e; }));` as [described in this answer](http://stackoverflow.com/questions/26667598/will-javascript-es6-promise-support-done-api). Or just `.catch(e => console.log(e.toString())` works too. The former throws real errors to web console with clickable file and line number links, which I like. As long as you output the error somewhere, that's the important part. – jib Jun 18 '15 at 01:35
  • Right, but did you read the article, the part where he seems to be mentioning 'done', in the terminate section? I was already thinking what your saying.. – blamb Jun 18 '15 at 01:42
1

If I remember correctly, catch will fire when your promise is rejected. Since you have the fail callback attached, your catch will not fire unless you call reject function in either your fail or success callback. In other words, catch block is catching rejection in your then method.

user1605909
  • 106
  • 1
  • 4
  • Right, the catch should not fire, your right, you seem to understand where im at right now. Something is making it fire though, in one of my particular calls, so ponder for a moment that it could be something making it fire, hmmm... could it be a parent method somewhere up the chain in the case? If so, then this has real value, shedding some light on maybe problems up the chain, that werent caught earlier.. :-p im wondering if thats what happening, just that the bas'terd' is empty in that case.. and im confused now anyway trying to figure if i even need it, or if im causing it... – blamb Jun 17 '15 at 04:46
  • ahh i think i changed your response a bit? so your saying the catch is maybe catching the rejection in one of the thens in the chain above, and it just so happens to be an empty object? if so, then this means im on the right track by using it just incase. i just cant see ANY other trace to figure out what it is. So as long as i understand the fundamentals correctly here, then im not worried about it. – blamb Jun 17 '15 at 04:50
  • Dude, this is like totally a f'in bad loop on acid trip of dimensions in promise land, of chained thenables, lol. so from another angle, im thinking on this differently what you say, the catch isnt for anything to do with the reject callbacks in the thens, is it? if so, then maybe I have missed something. I think i a catch can just be used alternately, in no way does it backup the then fail callbacks as i thought you just suggested, reading into it different, i believe. so can you clarify? – blamb Jun 17 '15 at 04:53
  • 1
    Yes, I believe it is something up the chain. – user1605909 Jun 17 '15 at 04:53
  • ok, then this is good news to me... so is it sounding like it might justify the use of a catch sort of a JUST IN CASE for things like this, when your seriously chaining things? I mean in my case, i will from here forward use a reject callback in all thens, where practical, but ill still have that catch there now i think. will this work guys? – blamb Jun 17 '15 at 04:54
  • Its probably the way i built my wrapper, thats a can of worms. i need to finish it before i go asking for help on that one, lol. I think i need to pull it in here. – blamb Jun 17 '15 at 04:56
  • 1
    that's my understanding. if any rejection happens in the chain isn't handled by fail callback, it will be caught by catch. And the other reason, you want to use catch is you can reject promise in success callback. – user1605909 Jun 17 '15 at 04:57
  • True, ok so im on the right track i think, and it sounds like your saying definitely use a catch always at the end unless you are interesting in some errors inevitable going unnoticed? 'ANy more opinions on this? I think promises are in an early stage right now and need the think tank approach like this. im definitely open to anything i missed in here, and maybe i should share my wrapper to see what it might be. – blamb Jun 17 '15 at 05:01