This repository was archived by the owner on Feb 24, 2018. It is now read-only.
This repository was archived by the owner on Feb 24, 2018. It is now read-only.
Async APIs should be returning Promises #88
Description
The current callbacks are a mess of node-style and callback objects, and are often immediately invoked, a problem for re-entrancy. I propose all callback accepting methods are updated to return a Promise and to make the callback argument optional.
With all the current lib dependencies, adding a dependency for global Promise or polyfill should not be a big deal.
There is a growing expectation that async methods return a Promise, they compose far better, and the proposed (and available now with babel) async / await language feature make them as simple to use as synchronous methods:
// callback style, still works function getUserEmailCallback(cognitoUser, callback) { cognitoUser.getUserAttributes(function(err, result) { if (err) { return callback(err, null); } var attr = result.filter(function (x) { return x.getName() === 'email'; })[0]; if (!attr) { return callback(new Error('User missing "email" attribute?'), null); } callback(null, attr.getValue()); }); } // promise style function getUserEmailPromise(cognitoUser) { return cognitoUser.getUserAttributes().then(function (result) { var attr = result.filter(function (x) { return x.getName() === 'email'; })[0]; if (!attr) { throw new Error('User missing "email" attribute?'); } return attr.getValue(); }); } // promise style used with async function language feature async function getUserEmailAsync(cognitoUser) { var result = await cognitoUser.getUserAttributes(); var attr = result.filter(function (x) { return x.getName() === 'email'; })[0]; if (!attr) { throw new Error('User missing "email" attribute?'); } return attr.getValue(); }With callback object styles, I currently only propose making onSuccess and onFailure optional, other callback properties can still make sense (but probably should be broken into multiple calls later):
function provideEmailVerificationCallbackl(cognitoUser, verificationCode, callback) { cognitoUser.getAttributeVerificationCode('email', { onSuccess: function (result) { callback(null, result); }, onFailure: function(err) { callback(err, null); }, inputVerificationCode() { cognitoUser.verifyAttribute('email', verificationCode, this); } }); } function provideEmailVerificationPromise(cognitoUser, verificationCode) { return cognitoUser.getAttributeVerificationCode('email', { inputVerificationCode() { cognitoUser.verifyAttribute('email', verificationCode, this); } }); }OLD PRE-ES6 CODE Strawman implementations (untested)
A minimal (hand-written) diff for hybrid node style and Promise returning:
CognitoUser.prototype.deleteUser = function deleteUser(callback) { + var promise = new Promise(function (resolve, reject) { if (this.signInUserSession != null && this.signInUserSession.isValid()) { this.client.deleteUser({ AccessToken : this.signInUserSession.getAccessToken().getJwtToken() }, function (err, data) { if (err) { - return callback(err, null); + return reject(err); } else { - return callback(null, 'SUCCESS'); + return resolve('SUCCESS'); } }); } else { - return callback(new Error('User is not authenticated'), null); + throw new Error('User is not authenticated'); } + }); + if (callback) { + promise.then(function (data) { callback(null, data); }, function (err) { callback(err, null); }); + } + return promise; };Simply wrapping with polygoat:
- CognitoUser.prototype.deleteUser = function deleteUser(callback) { + CognitoUser.prototype.deleteUser = function deleteUser(userCallback) { + return polygoat(function (callback) { if (this.signInUserSession != null && this.signInUserSession.isValid()) { this.client.deleteUser({ AccessToken : this.signInUserSession.getAccessToken().getJwtToken() }, function (err, data) { if (err) { return callback(err, null); } else { return callback(null, 'SUCCESS'); } }); } else { return callback(new Error('User is not authenticated'), null); } + }, userCallback); };Rewriting using AWS SDK promise support and hybrid-izer wrapper:
CognitoUser.prototype.deleteUser = toNodePromiseHybrid(function deleteUser() { if (this.signInUserSession != null && this.signInUserSession.isValid()) { return this.client.deleteUser({ AccessToken : this.signInUserSession.getAccessToken().getJwtToken() }).promise(); } else { throw new Error('User is not authenticated'); } }); function toNodePromiseHybrid(method) { return function () { var args = Array.prototype.slice(arguments); var callback = null; if (args.length && args[args.length - 1] instanceof Function) { callback = args.pop(); } var promise = method.apply(this, args); if (callback) { promise.then(function (data) { callback(null, data); }, function (err) { callback(err, null); }); } return promise; }; }