Calling token API

Question

I'm calling an API to request token, and making sure i'm not leaving any corners for errors and handle the result appropriately.

It's a web service POST call in general, The code gets credentials, formats them in the URL and parses the result.

The objective here is I'm looking if code overall is good when it comes to REST call, if any corner I'm missing for an error somewhere.

function Authentication() {
}
Authentication.prototype.requestToken = function(credentials) {
 var self = this;
 return new Promise(function(resolve, reject){
 https.request({
 method: 'POST',
 hostname: 'hostname.com',
 path: '/path-to-api' +
 '&client_id=' + credentials.clientId +
 '&client_secret=' + credentials.clientSecret +
 '&device_id=' + credentials.deviceId +
 '&device_token=' + credentials.deviceToken,
 headers: {
 'accept': 'application/json;charset=UTF-8',
 'content-type': 'application/x-www-form-urlencoded'
 }
 }, function(response) {
 var buffers = [];
 response.on('data', buffers.push.bind(buffers));
 response.on('end', function(){
 try {
 self.token = JSON.parse(Buffer.concat(buffers).toString());
 response.statusCode == 200 ? resolve(self.token) : reject(self.token);
 } catch (error) {
 reject(error);
 }
 });
 }).on('error', reject).end();
 });
};
Authentication.prototype.token;

• Mast: Please tell us more about what your code does and why you wrote it. Code lives in a context and a good review takes this context into account.
• Developer: @Mast I've added some details, hope it helps. The idea is to know if the http call is comprehensive covering all corners of errors and results.
• Mast: Much better already.

Answer 1

A few minor quibbles:

• You should always have a try-catch inside a new Promise, otherwise exceptions are swallowed.
• Query parameters should be escaped
• "new Promise" should be a rare sight. Never use it directly. Always wrap the thing you want to use with a promise interface (in this case, it's the https library)

And a major quibble:

• This code is stuck in the past. You can simplify greatly if you embrace newer versions of javascript, and maybe add a couple of packages

For example:

const rp = require('request-promise')
class Authentication{
 constructor(){
 this.token = null
 }
 async requestToken(credentials){
 const token = await rp({
 method: 'POST',
 uri: 'https://hostname.com/path-to-api',
 json: true,
 qs: {
 client_id: credentials.clientId,
 client_secret: credentials.clientSecret,
 device_id: credentials.deviceId,
 device_token: credentials.deviceToken,
 },
 headers: {
 'accept': 'application/json;charset=UTF-8',
 'content-type': 'application/x-www-form-urlencoded'
 }
 })
 this.token = token
 return token
 }
}

• Developer: thanks but I don't see any error handlings, it's never a smooth ride, what if things goes wrong? my code was around all possibilites of errors, from json parsing, to http errors and returning it to the caller
• Developer: I didn't get you on You should always have a try-catch inside a new Promise, otherwise exceptions are swallowed. please explain more. I think my code try catch is inside.
• Magnus Jeffs Tovslid: Your code didn't handle any errors, it just turned them into rejections. My version also does this. About try catch inside a new promise. Anything you don't reject yourself will be swallowed. For example, if credentials is null, you will get a swallowed exception.
• Developer: in your version, I'll have a try catch outside or catch expression at the caller level of requestToken, appreciate if you can add that part just for a comprehensive deal.