superagent: [fixed]The .then() function is not compatible with Promises

The faux .then() function added in 1.3 completely misses the point of promises and breaks even in simple cases.

request.get(someCorrectUrl)
    .then(function(){return 1;})
    .then(console.log.bind(console));

Should print 1, but throws an unhandled exception.

request.get('fail').then(function(){});

TypeError: reject is not a function

request.get('fail')
    .then(undefined, function(){throw Error();})
    .catch(function(){})

.then(…).catch is not a function

Please don’t try to reimplement promises — just enable use of a correct library.

I suggest:

Request.prototype.then = function(resolve, reject) {
  var self = this;
  return new Promise(function(resolve, reject){
    self.end(function(err, res){
      if (err) reject(err); else resolve(res);
    });
  })
  .then(resolve, reject);
};

The code above assumes that promises are either supported natively (already the case in majority of browsers and even Node 0.12) or polyfilled by the user (which is a common practice, e.g. ES6 transpilers do it).

About this issue

  • Original URL
  • State: closed
  • Created 9 years ago
  • Comments: 33 (17 by maintainers)

Commits related to this issue

Most upvoted comments

This is fixed in 2.0.0

No, .end() is the old world. Don’t use it. .then() is a replacement for .end().

Looks like version 2.0 is the answer.

A thenable without chaining is no different than supplying a callback as an argument to the method. It serves no purpose other than to fool the developer into thinking it’s a fully-implemented thenable. If you don’t want to adhere to the promises/A+ spec, then remove the then function.