From 86892ee1c68b5e16b26095dc098fa962c394692f Mon Sep 17 00:00:00 2001 From: Jonathan Lomas Date: Wed, 30 May 2018 15:23:57 -0700 Subject: [PATCH 1/3] console.warn on unknown options --- lib/utils.js | 22 +++++++++++++++-- test/utils.spec.js | 60 +++++++++++++++++++++++++++++++++++++--------- 2 files changed, 69 insertions(+), 13 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index bf82bf92c3..9e1cd97e08 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -75,8 +75,8 @@ var utils = module.exports = { // (the first being args and the second options) and with known // option keys in the first so that we can warn the user about it. if (optionKeysInArgs.length > 0 && optionKeysInArgs.length !== argKeys.length) { - console.warn( // eslint-disable-line no-console - 'Stripe: Options found in arguments (' + optionKeysInArgs.join(', ') + '). Did you mean to pass an options ' + + emitStripeWarning( // eslint-disable-line + 'Options found in arguments (' + optionKeysInArgs.join(', ') + '). Did you mean to pass an options ' + 'object? See https://github.com/stripe/stripe-node/wiki/Passing-Options.' ); } @@ -98,6 +98,15 @@ var utils = module.exports = { opts.auth = args.pop(); } else if (utils.isOptionsHash(arg)) { var params = args.pop(); + + var extraKeys = Object.keys(params).filter(function(key) { + return OPTIONS_KEYS.indexOf(key) == -1; + }); + + if (extraKeys.length) { + emitStripeWarning('Invalid options found (' + extraKeys.join(', ') + '); ignoring.'); + } + if (params.api_key) { opts.auth = params.api_key; } @@ -195,3 +204,12 @@ var utils = module.exports = { return obj; }, }; + +function emitStripeWarning(warning) { + if (typeof process.emitWarning !== 'function') { + /* eslint-disable no-console */ + return console.warn('Stripe: ' + warning); + } + + return process.emitWarning(warning, 'Stripe'); +} diff --git a/test/utils.spec.js b/test/utils.spec.js index 4ad493b232..0a5e395cb7 100644 --- a/test/utils.spec.js +++ b/test/utils.spec.js @@ -106,26 +106,30 @@ describe('utils', function() { expect(utils.getDataFromArgs(args)).to.deep.equal({}); expect(args.length).to.equal(2); }); - it('ignores a hash with only options', function() { + it('ignores a hash with only options', function(done) { var args = [{api_key: 'foo'}]; - handleConsoleWarns(function() { + handleWarnings(function() { expect(utils.getDataFromArgs(args)).to.deep.equal({}); expect(args.length).to.equal(1); + + done(); }, function(message) { throw new Error('Should not have warned, but did: ' + message); }); }); - it('warns if the hash contains both data and options', function() { + it('warns if the hash contains both data and options', function(done) { var args = [{foo: 'bar', api_key: 'foo', idempotency_key: 'baz'}]; - handleConsoleWarns(function() { + handleWarnings(function() { utils.getDataFromArgs(args); }, function(message) { expect(message).to.equal( 'Stripe: Options found in arguments (api_key, idempotency_key).' + ' Did you mean to pass an options object? See https://github.com/stripe/stripe-node/wiki/Passing-Options.' ); + + done(); }); }); it('finds the data', function() { @@ -212,6 +216,25 @@ describe('utils', function() { }); expect(args.length).to.equal(0); }); + it('warns if the hash contains something that does not belong', function(done) { + var args = [{foo: 'bar'}, { + api_key: 'sk_test_iiiiiiiiiiiiiiiiiiiiiiii', + idempotency_key: 'foo', + stripe_version: '2010-01-10', + fishsticks: true, + custard: true, + },]; + + handleWarnings(function() { + utils.getOptionsFromArgs(args); + }, function(message) { + expect(message).to.equal( + 'Stripe: Invalid options found (fishsticks, custard); ignoring.' + ); + + done(); + }); + }); }); describe('arrayToObject', function() { @@ -262,15 +285,30 @@ describe('utils', function() { }); /* eslint-disable no-console */ -function handleConsoleWarns(doWithShimmedConsoleWarn, onWarn) { - // Shim `console.warn` - var _warn = console.warn; +function handleWarnings(doWithShimmedConsoleWarn, onWarn) { + /* eslint-disable no-console */ + if (typeof process.emitWarning !== 'function') { + // Shim `console.warn` + var _warn = console.warn; + console.warn = onWarn; + + doWithShimmedConsoleWarn(); - console.warn = onWarn; + // Un-shim `console.warn`, + console.warn = _warn; + } else { + /* eslint-disable no-inner-declarations */ + function onProcessWarn(warning) { + onWarn(warning.name + ': ' + warning.message); + } - doWithShimmedConsoleWarn(); + process.on('warning', onProcessWarn); - // Un-shim `console.warn` - console.warn = _warn; + doWithShimmedConsoleWarn(); + + process.nextTick(function() { + process.removeListener('warning', onProcessWarn); + }) + } } /* eslint-enable no-console */ From 9477bcb0b22ea1cf1f3216a5662b094ccb8d971f Mon Sep 17 00:00:00 2001 From: Jonathan Lomas Date: Thu, 31 May 2018 11:31:03 -0700 Subject: [PATCH 2/3] Fix eslint stuff, rename function --- lib/utils.js | 9 ++++----- test/utils.spec.js | 3 ++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 9e1cd97e08..4387969bed 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -75,7 +75,7 @@ var utils = module.exports = { // (the first being args and the second options) and with known // option keys in the first so that we can warn the user about it. if (optionKeysInArgs.length > 0 && optionKeysInArgs.length !== argKeys.length) { - emitStripeWarning( // eslint-disable-line + emitWarning( 'Options found in arguments (' + optionKeysInArgs.join(', ') + '). Did you mean to pass an options ' + 'object? See https://github.com/stripe/stripe-node/wiki/Passing-Options.' ); @@ -104,7 +104,7 @@ var utils = module.exports = { }); if (extraKeys.length) { - emitStripeWarning('Invalid options found (' + extraKeys.join(', ') + '); ignoring.'); + emitWarning('Invalid options found (' + extraKeys.join(', ') + '); ignoring.'); } if (params.api_key) { @@ -205,10 +205,9 @@ var utils = module.exports = { }, }; -function emitStripeWarning(warning) { +function emitWarning(warning) { if (typeof process.emitWarning !== 'function') { - /* eslint-disable no-console */ - return console.warn('Stripe: ' + warning); + return console.warn('Stripe: ' + warning); /* eslint-disable-line no-console */ } return process.emitWarning(warning, 'Stripe'); diff --git a/test/utils.spec.js b/test/utils.spec.js index 0a5e395cb7..d9269553b3 100644 --- a/test/utils.spec.js +++ b/test/utils.spec.js @@ -296,6 +296,8 @@ function handleWarnings(doWithShimmedConsoleWarn, onWarn) { // Un-shim `console.warn`, console.warn = _warn; + + /* eslint-enable no-console */ } else { /* eslint-disable no-inner-declarations */ function onProcessWarn(warning) { @@ -311,4 +313,3 @@ function handleWarnings(doWithShimmedConsoleWarn, onWarn) { }) } } -/* eslint-enable no-console */ From 7269a3d649d2aeaccb2553f39ecdca92f9196b06 Mon Sep 17 00:00:00 2001 From: Jonathan Lomas Date: Thu, 31 May 2018 16:27:01 -0700 Subject: [PATCH 3/3] Fix more eslint stuff --- test/utils.spec.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/utils.spec.js b/test/utils.spec.js index d9269553b3..66ee60b889 100644 --- a/test/utils.spec.js +++ b/test/utils.spec.js @@ -284,10 +284,10 @@ describe('utils', function() { }); }); -/* eslint-disable no-console */ function handleWarnings(doWithShimmedConsoleWarn, onWarn) { - /* eslint-disable no-console */ if (typeof process.emitWarning !== 'function') { + /* eslint-disable no-console */ + // Shim `console.warn` var _warn = console.warn; console.warn = onWarn; @@ -299,8 +299,7 @@ function handleWarnings(doWithShimmedConsoleWarn, onWarn) { /* eslint-enable no-console */ } else { - /* eslint-disable no-inner-declarations */ - function onProcessWarn(warning) { + function onProcessWarn(warning) { /* eslint-disable-line no-inner-declarations */ onWarn(warning.name + ': ' + warning.message); }