Error early when params not specified for card functions #484
Conversation
Card-related API calls are a bit of an unusual case because we need to have an instantiated params struct in order to determine the correct URL to which to make the request. Previously, if a `nil` was passed to any of these functions, we'd panic as we tried to access one of its members. This patch just makes the params requirement a little more explicit by returning an error if any of card functions is called with `nil` params. Fixes #483.
brandur-stripe
force-pushed
the
brandur-require-params
branch
from
848cfb6
to
6aacada
Compare
| @@ -129,6 +141,10 @@ func Del(id string, params *stripe.CardParams) (*stripe.Card, error) { | |||
| } | |||
|
|
|||
| func (c Client) Del(id string, params *stripe.CardParams) (*stripe.Card, error) { | |||
| if params == nil { | |||
There was a problem hiding this comment.
We're already doing if params != nil { below (not visible) so it's redundant. Should you fix the one below in that case?
There was a problem hiding this comment.
Yeah good point. Unfortunately the other one would still result in a panic because of the check on params.Account immediately afterwards, so I've taken that one out in favor of my addition.
|
|
||
| return &Iter{stripe.GetIter(lp, body, func(b *form.Values) ([]interface{}, stripe.ListMeta, error) { | ||
| list := &stripe.CardList{} | ||
| var err error | ||
|
|
||
| if params == nil { |
There was a problem hiding this comment.
Why not just error above like you do in the other functions if params == nil?
There was a problem hiding this comment.
Ah, basically because List functions only return an *Iter (and not an error). The first opportunity we get to return an error is the first time the iterator iterates, and so the check on params is further down than you'd expect.
There was a problem hiding this comment.
ah gotcha, that makes sense, thanks for clarifying!
|
Thanks Remi. Released as 28.4.1. |
Bumps [sorbet](https://github.com/sorbet/sorbet) from 0.5.10073 to 0.5.10087. - [Release notes](https://github.com/sorbet/sorbet/releases) - [Commits](https://github.com/sorbet/sorbet/commits) --- updated-dependencies: - dependency-name: sorbet dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Card-related API calls are a bit of an unusual case because we need to
have an instantiated params struct in order to determine the correct URL
to which to make the request. Previously, if a
nilwas passed to anyof these functions, we'd panic as we tried to access one of its members.
This patch just makes the params requirement a little more explicit by
returning an error if any of card functions is called with
nilparams.Fixes #483.
r? @remi-stripe