Bad options keys; should we throw?

[jlomas-stripe]

A question that came to mind as a result of #374, where I sent in api_version instead of stripe_version and it didn't surface until the latest API Version was bumped (and my set test value no longer matched latest):

Should we be throwing if we see something in the options that doesn't belong?

On the one hand, it would be useful to get immediate feedback that you'd provided bad arguments and we're not gonna do what you asked;

On the other hand, we'll need to keep that list up to date - which isn't really that much of a burden.

[brandur-stripe]

We did something a little similar in Ruby where we whitelist known options so that we could warn people when they accidentally put them in the parameters hash instead of the options hash:

stripe/stripe-ruby#545

This pattern helps insulate users from dumb mistakes that take time to debug and I like it. +1 to doing something like what you suggest.

One note is that we ended up settling on a warning to stderr instead of throwing an error. This has the advantage of being backwards compatible, and also forwards compatible to a degree (if we introduced a new option, it could be used even with an older version of the library, albeit with a warning). Thoughts?

[brandur-stripe]

And oops, I'm reading my inbox backwards and just saw #306 which is very similar to the stripe-ruby issue I linked.

[jlomas-stripe]

No worries. #306 is actually just "throw if you combine args and options", as we currently just ignore the args if you combine them - so it's a bit different.

I am 100% onside with warning via console or equivalent instead of throwing. Currently thinking of implementing this using process.emitWarning() which started being available in 6.0.0, and wrapping in a fn that will either use that if present, or just console.error() if not.

There's some extra usefulness to using process.emitWarning() if available - i.e., loggers using process.on('warning'), etc. - so that seems to be the right approach...?

[brandur-stripe]

I am 100% onside with warning via console or equivalent instead of throwing. Currently thinking of implementing this using process.emitWarning() which started being available in 6.0.0, and wrapping in a fn that will either use that if present, or just console.error() if not.

Yep, that sounds good to me.

[robz-stripe]

Looks like this was fixed in #465.