Promise chaining problems


(Hugo Almeida) #1

I’ve started to write a plugin that extends the PostView to support translating a post content, right now using the Bing api.

I’m having a problem, not really related to Discourse but related to promise chaining. I was under the impression that if you return a promise inside the then it will chain it, so i must be doing something wrong.

The post view code:

Discourse.PostView.reopen({
  actions: {
    translate: function(post) {
      Discourse.Translator.translate(post.cooked).then(function(translation) {
        alert(translation);
      });
    }
  }
});

The translation request code:

Discourse.Translator = {
  token: null,

  getToken: function() {
    var translator = this;
    return Discourse.ajax('/translator/token', { type: 'POST' }).then(function(res){
      translator.token = res.access_token;
    });
  },

  translate: function(content) {
    var translator = this;
    return this.getToken().then(function() {
      return Discourse.ajax('http://api.microsofttranslator.com/V2/Ajax.svc/Translate', { 
        type: 'POST',
        dataType: 'jsonp',
        data: {
          appid: "Bearer " + translator.token,
          to: "fr",
          text: content,
          contentType: 'text/html'
        }
      }).then(function(res) {
        return res;
      });
    });
  }
};

This code will result in an uncaught promise, the Discourse.ajax in translate function.


(Robin Ward) #2

If it possible your Discourse.ajax call to bing is failing?

Promises should also handle failure conditions using fail. If you attach a fail to your ajax you should see what went wrong.

Additionally, you don’t need to to:

.then(function(res) {
        return res;
      });

That’s the default behavior.


(Hugo Almeida) #3

Thanks it helped me debug, the Discourse.ajax was indeed failing but not at the response level, it returned 200 and the expected contents.

The problem was fixed when i added jsonp: 'oncomplete' to the options of translate ajax request. Although i don’t really understand why.


(Robin Ward) #4

jQuery’s AJAX can be a bit weird if the response could not be parsed. The oncomplete probably told it what it needed to parse it properly.


(Hugo Almeida) #5

After reading a bit about JSONP the endpoint needs to be provided the callback function for wrapping (the Padding in JSONP) the json.

its very minor but I think this is ‘oncomplete’ in 99% of the cases since Discourse already wraps ajax calls to be in promises. My suggestion would be to include jsonp: 'oncomplete' by default.

Counter point to above suggestion is i don’t really understand why this works then:
(note: this is the only place i could find a jsonp request in Discourse, so my suggestion is not that important)

https://github.com/discourse/discourse/blob/master/app/assets/javascripts/admin/models/github_commit.js#L27-L41

maybe $.ajax already defines a few entry points for frequently used names of callbacks.


(Robin Ward) #6

Oh I see, yes the JSONP callback definitely needs to be specified in order to work. That Discourse.Ajax one does it by saying callback=callback which is how most jQuery calls tend to look for JSONP.

Since that’s the only other place we specify it, I am hesitant to add the setting everywhere.