Do I always need catch() at the end even if I use a reject callback in all then-ables?

5.1k views Asked by At

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"
}
3

There are 3 answers

8
Bergi On BEST ANSWER

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.

8
user1605909 On

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.

4
jib On

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).