change jwt.sign to return errors on callback instead of throwing errors

jfromaniello · jfromaniello · commit 1e46c5a42aa3 · 2016-05-19T12:47:24.000-03:00
diff --git a/sign.js b/sign.js
@@ -48,13 +48,20 @@ module.exports = function(payload, secretOrPrivateKey, options, callback) {
     typ: typeof payload === 'object' ? 'JWT' : undefined
   }, options.header);
 
+  function failure (err) {
+    if (callback) {
+      return callback(err);
+    }
+    throw err;
+  }
+
   if (typeof payload === 'undefined') {
-    throw new Error('payload is required');
+    return failure(new Error('payload is required'));
   } else if (typeof payload === 'object') {
     var payload_validation_result = registered_claims_schema.validate(payload);
 
     if (payload_validation_result.error) {
-      throw payload_validation_result.error;
+      return failure(payload_validation_result.error);
     }
 
     payload = xtend(payload);
@@ -64,22 +71,22 @@ module.exports = function(payload, secretOrPrivateKey, options, callback) {
     });
 
     if (invalid_options.length > 0) {
-      throw new Error('invalid ' + invalid_options.join(',') + ' option for ' + (typeof payload ) + ' payload' );
+      return failure(new Error('invalid ' + invalid_options.join(',') + ' option for ' + (typeof payload ) + ' payload' ));
     }
   }
 
   if (typeof payload.exp !== 'undefined' && typeof options.expiresIn !== 'undefined') {
-    throw new Error('Bad "options.expiresIn" option the payload already has an "exp" property.');
+    return failure(new Error('Bad "options.expiresIn" option the payload already has an "exp" property.'));
   }
 
   if (typeof payload.nbf !== 'undefined' && typeof options.notBefore !== 'undefined') {
-    throw new Error('Bad "options.notBefore" option the payload already has an "nbf" property.');
+    return failure(new Error('Bad "options.notBefore" option the payload already has an "nbf" property.'));
   }
 
   var validation_result = sign_options_schema.validate(options);
 
   if (validation_result.error) {
-    throw validation_result.error;
+   return failure(validation_result.error);
   }
 
   var timestamp = payload.iat || Math.floor(Date.now() / 1000);
@@ -93,22 +100,22 @@ module.exports = function(payload, secretOrPrivateKey, options, callback) {
   if (typeof options.notBefore !== 'undefined') {
     payload.nbf = timespan(options.notBefore);
     if (typeof payload.nbf === 'undefined') {
-      throw new Error('"notBefore" should be a number of seconds or string representing a timespan eg: "1d", "20h", 60');
+      return failure(new Error('"notBefore" should be a number of seconds or string representing a timespan eg: "1d", "20h", 60'));
     }
   }
 
   if (typeof options.expiresIn !== 'undefined' && typeof payload === 'object') {
     payload.exp = timespan(options.expiresIn);
     if (typeof payload.exp === 'undefined') {
-      throw new Error('"expiresIn" should be a number of seconds or string representing a timespan eg: "1d", "20h", 60');
+      return failure(new Error('"expiresIn" should be a number of seconds or string representing a timespan eg: "1d", "20h", 60'));
     }
   }
 
   Object.keys(options_to_payload).forEach(function (key) {
     var claim = options_to_payload[key];
     if (typeof options[key] !== 'undefined') {
       if (typeof payload[claim] !== 'undefined') {
-        throw new Error('Bad "options.' + key + '" option. The payload already has an "' + claim + '" property.');
+        return failure(new Error('Bad "options.' + key + '" option. The payload already has an "' + claim + '" property.'));
       }
       payload[claim] = options[key];
     }
diff --git a/test/async_sign.tests.js b/test/async_sign.tests.js
@@ -17,12 +17,28 @@ describe('signing a token asynchronously', function() {
       });
     });
 
-    it('should throw error', function(done) {
+    it('should return error when secret is not a cert for RS256', function(done) {
       //this throw an error because the secret is not a cert and RS256 requires a cert.
       jwt.sign({ foo: 'bar' }, secret, { algorithm: 'RS256' }, function (err) {
         expect(err).to.be.ok();
         done();
       });
     });
+
+    it('should return error on wrong arguments', function(done) {
+      //this throw an error because the secret is not a cert and RS256 requires a cert.
+      jwt.sign({ foo: 'bar' }, secret, { notBefore: {} }, function (err) {
+        expect(err).to.be.ok();
+        done();
+      });
+    });
+
+    it('should return error on wrong arguments (2)', function(done) {
+      jwt.sign('string', 'secret', {noTimestamp: true}, function (err) {
+        expect(err).to.be.ok();
+        expect(err).to.be.instanceof(Error);
+        done();
+      });
+    });
   });
 });

Original file line number	Diff line number	Diff line change
`@@ -48,13 +48,20 @@ module.exports = function(payload, secretOrPrivateKey, options, callback) {`
`48`	`48`	`typ: typeof payload === 'object' ? 'JWT' : undefined`
`49`	`49`	`}, options.header);`
`50`	`50`
	`51`	`+ function failure (err) {`
	`52`	`+ if (callback) {`
	`53`	`+ return callback(err);`
	`54`	`+ }`
	`55`	`+ throw err;`
	`56`	`+ }`
	`57`	`+`
`51`	`58`	`if (typeof payload === 'undefined') {`
`52`		`- throw new Error('payload is required');`
	`59`	`+ return failure(new Error('payload is required'));`
`53`	`60`	`} else if (typeof payload === 'object') {`
`54`	`61`	`var payload_validation_result = registered_claims_schema.validate(payload);`
`55`	`62`
`56`	`63`	`if (payload_validation_result.error) {`
`57`		`- throw payload_validation_result.error;`
	`64`	`+ return failure(payload_validation_result.error);`
`58`	`65`	`}`
`59`	`66`
`60`	`67`	`payload = xtend(payload);`
`@@ -64,22 +71,22 @@ module.exports = function(payload, secretOrPrivateKey, options, callback) {`
`64`	`71`	`});`
`65`	`72`
`66`	`73`	`if (invalid_options.length > 0) {`
`67`		`- throw new Error('invalid ' + invalid_options.join(',') + ' option for ' + (typeof payload ) + ' payload' );`
	`74`	`+ return failure(new Error('invalid ' + invalid_options.join(',') + ' option for ' + (typeof payload ) + ' payload' ));`
`68`	`75`	`}`
`69`	`76`	`}`
`70`	`77`
`71`	`78`	`if (typeof payload.exp !== 'undefined' && typeof options.expiresIn !== 'undefined') {`
`72`		`- throw new Error('Bad "options.expiresIn" option the payload already has an "exp" property.');`
	`79`	`+ return failure(new Error('Bad "options.expiresIn" option the payload already has an "exp" property.'));`
`73`	`80`	`}`
`74`	`81`
`75`	`82`	`if (typeof payload.nbf !== 'undefined' && typeof options.notBefore !== 'undefined') {`
`76`		`- throw new Error('Bad "options.notBefore" option the payload already has an "nbf" property.');`
	`83`	`+ return failure(new Error('Bad "options.notBefore" option the payload already has an "nbf" property.'));`
`77`	`84`	`}`
`78`	`85`
`79`	`86`	`var validation_result = sign_options_schema.validate(options);`
`80`	`87`
`81`	`88`	`if (validation_result.error) {`
`82`		`- throw validation_result.error;`
	`89`	`+ return failure(validation_result.error);`
`83`	`90`	`}`
`84`	`91`
`85`	`92`	`var timestamp = payload.iat \|\| Math.floor(Date.now() / 1000);`
`@@ -93,22 +100,22 @@ module.exports = function(payload, secretOrPrivateKey, options, callback) {`
`93`	`100`	`if (typeof options.notBefore !== 'undefined') {`
`94`	`101`	`payload.nbf = timespan(options.notBefore);`
`95`	`102`	`if (typeof payload.nbf === 'undefined') {`
`96`		`- throw new Error('"notBefore" should be a number of seconds or string representing a timespan eg: "1d", "20h", 60');`
	`103`	`+ return failure(new Error('"notBefore" should be a number of seconds or string representing a timespan eg: "1d", "20h", 60'));`
`97`	`104`	`}`
`98`	`105`	`}`
`99`	`106`
`100`	`107`	`if (typeof options.expiresIn !== 'undefined' && typeof payload === 'object') {`
`101`	`108`	`payload.exp = timespan(options.expiresIn);`
`102`	`109`	`if (typeof payload.exp === 'undefined') {`
`103`		`- throw new Error('"expiresIn" should be a number of seconds or string representing a timespan eg: "1d", "20h", 60');`
	`110`	`+ return failure(new Error('"expiresIn" should be a number of seconds or string representing a timespan eg: "1d", "20h", 60'));`
`104`	`111`	`}`
`105`	`112`	`}`
`106`	`113`
`107`	`114`	`Object.keys(options_to_payload).forEach(function (key) {`
`108`	`115`	`var claim = options_to_payload[key];`
`109`	`116`	`if (typeof options[key] !== 'undefined') {`
`110`	`117`	`if (typeof payload[claim] !== 'undefined') {`
`111`		`- throw new Error('Bad "options.' + key + '" option. The payload already has an "' + claim + '" property.');`
	`118`	`+ return failure(new Error('Bad "options.' + key + '" option. The payload already has an "' + claim + '" property.'));`
`112`	`119`	`}`
`113`	`120`	`payload[claim] = options[key];`
`114`	`121`	`}`