Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn on unknown options #465

Merged
merged 3 commits into from
May 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ' +
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.'
);
}
Expand All @@ -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) {
emitWarning('Invalid options found (' + extraKeys.join(', ') + '); ignoring.');
}

if (params.api_key) {
opts.auth = params.api_key;
}
Expand Down Expand Up @@ -195,3 +204,11 @@ var utils = module.exports = {
return obj;
},
};

function emitWarning(warning) {
if (typeof process.emitWarning !== 'function') {
return console.warn('Stripe: ' + warning); /* eslint-disable-line no-console */
}

return process.emitWarning(warning, 'Stripe');
}
64 changes: 51 additions & 13 deletions test/utils.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -261,16 +284,31 @@ describe('utils', function() {
});
});

/* eslint-disable no-console */
function handleConsoleWarns(doWithShimmedConsoleWarn, onWarn) {
// Shim `console.warn`
var _warn = console.warn;
function handleWarnings(doWithShimmedConsoleWarn, onWarn) {
if (typeof process.emitWarning !== 'function') {
/* eslint-disable no-console */

// Shim `console.warn`
var _warn = console.warn;
console.warn = onWarn;

console.warn = onWarn;
doWithShimmedConsoleWarn();

doWithShimmedConsoleWarn();
// Un-shim `console.warn`,
console.warn = _warn;

// Un-shim `console.warn`
console.warn = _warn;
/* eslint-enable no-console */
} else {
function onProcessWarn(warning) { /* eslint-disable-line no-inner-declarations */
onWarn(warning.name + ': ' + warning.message);
}

process.on('warning', onProcessWarn);

doWithShimmedConsoleWarn();

process.nextTick(function() {
process.removeListener('warning', onProcessWarn);
})
}
}
/* eslint-enable no-console */