6

I am turning my comments on wordpress into an ajax driven system.

All is good so far, until I bumped into an issue with the .catch() method firing straight after the .then() method.

Here is my code...

Ajax engine

commentAPI: function(action, encoded, userID) {
    let self = this;

    return new Promise(function (resolve, reject) {
       //console.log("ajax call to " + self.ajaxURL + " with action " + action);

        jQuery.ajax({               
            url: self.ajaxURL,
            type: 'post',
            data: 'action='+action+'&data='+encoded,
            dataType: 'json',
            success: function(data, code, jqXHR) { resolve(data); },
            fail: function(jqXHR, status, err) { console.log('ajax error = ' + err ); reject(err); },
            beforeSend: function() {} //display loading gif
        });
    });
}, 

The method handling the comment form submission

handleReplyFormSubmit: function(form) {
    let self = this;

    this.removeErrorHtml(form);

    // Serialize form to name=value string
    const formdata = jQuery(form).serialize();

    // Validate inputs
    // * Wordpress doing this for now and providing error resonse 

    // Encoode data for easy sending
    const encodedJSON = btoa( formdata );

    this.commentAPI('dt_submitAjaxComment', encodedJSON).then(function(response){
        console.log('firing then');

        if( response.error == true ) {
            self.printFormError(form, response.errorMsg);
        }

        else { 
            let html = response.commentHTML;
            console.log('html returned' + html)
            jQuery(form).append(html);
            Jquery(form).remove();
        }

    }).catch(function(err) {            
        console.log('firing catch');

        if( err !== undefined && err.length > 0 ) { 
            self.printFormError(form, err);
        }

        else { 
            self.printFormError(form, 'Unkown error');
        }
    });

    return false;
},

The code is doing what it is supposed to, but the catch method is being fired too, which is making error handling frustrating...

Output of code. Notice the console.log('firing catch') is being fired

Notice how this is being fired

console.log('firing catch')

But this isn't ( within the ajax fail function )

console.log('ajax error = ' + err );

Am I doing something wrong?

Dale Woods
  • 784
  • 1
  • 13
  • 31

1 Answers1

10

Promises

It is common that a then fires, and then a later catch: it means that some error was encountered in your then handler code, so the catch fires. Catch handlers will fire:

  • If there is an error in the asynchronous operation, so the Promise is rejected.
  • If there is an error in any previous then handler.

So, the following code:

Promise.resolve()
.then( () => {
  console.log('this will be called');
  throw new Error('bum');
  console.log('this wont be logged');
})
.catch(err => {
  console.log('this will be logged too');
  console.log(err); // bum related error
});

Will yield logs both from then and catch handlers.

Your code

Inside your then handler there is this code:

    else { 
        let html = response.commentHTML;
        console.log('html returned' + html)
        jQuery(form).append(html);
        Jquery(form).remove();
    }

Notice how the last line have Jquery instead of jQuery, which is a typo. I bet this is the error causing the catch to fire.

On top of that

Modern verions of jQuery just return a promise from $.ajax(), so there is no need to wrap it into another promise.

This code:

commentAPI: function(action, encoded, userID) {
  let self = this;

  return new Promise(function (resolve, reject) {
  //console.log("ajax call to " + self.ajaxURL + " with action " + action);

    jQuery.ajax({               
        url: self.ajaxURL,
        type: 'post',
        data: 'action='+action+'&data='+encoded,
        dataType: 'json',
        success: function(data, code, jqXHR) { resolve(data); },
        fail: function(jqXHR, status, err) { console.log('ajax error = ' + err ); reject(err); },
        beforeSend: function() {} //display loading gif
    });
  });
},

Should just be:

commentAPI: function(action, encoded, userID) {
    return jQuery.ajax({
        url: this.ajaxURL,
        type: 'post',
        data: 'action='+action+'&data='+encoded,
        dataType: 'json',
        beforeSend: function() {} //display loading gif
    });
},

So you can handle success and failure in commentApi then and catch handlers, instead of passing success and fail callbacks to resolve or reject a wrapping Promise.

The ajax success params

The success callback param takes three parameters. However, Promises usually just take one.

But, jQuery does pass the same three arguments to the then handler, so in case you need access to them, you can still use them in the handler:

this.commentAPI('dt_submitAjaxComment', encodedJSON).then(function(data, code, jqXhr){
 // the three arguments will be accessible here.
...
}
Sergeon
  • 6,638
  • 2
  • 23
  • 43
  • Thanks, I did see the typo before your response, but it did fix it. Why was the error just 'undefined'? Should I check for another type of error object in the catch also? – Dale Woods Aug 04 '18 at 14:14
  • Also when reducing the commentAPI as you suggested, what params are passed to the promise resolve/reject callbacks? Same as the default which I've listed in my ajax function? – Dale Woods Aug 04 '18 at 14:15
  • 2
    "Could just be" should read "Should just be" to avoid the [explicit promise construction antipattern](https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it) – charlietfl Aug 04 '18 at 14:17
  • You don't need to pass params, you call `then` and `catch` right after the ajax call. I'll update my answer later with an example. – Sergeon Aug 04 '18 at 14:17
  • You might want to [use `Promise.resolve(…)`](https://stackoverflow.com/a/31327725/1048572), though – Bergi Aug 04 '18 at 14:40