search

chrisvdm / redwoodjs-stripe

A Redwood/Stripe integration made easy
MIT License
46 stars 8 forks source link

fix: Correct null id check in retrieveStripeCustomer #144

Closed casey-matt closed 2 weeks ago

casey-matt commented 3 weeks ago

It looks like the retrieveStripeCustomer implementation has a small bug where the conditional is flipped. My project is passing in customer ID similar to the example-store-stripe repo like this: 

 <StripeProvider customer={{ id: customer }}>
    ...
 </StripeProvider>

And eventually results in a call to retrieveStripeCustomer(), which triggers this error:

api | [13:13:05.115] ERROR: Unexpectedly received null or undefined data. This may be a bug in redwoodjs-stripe.
api |
api | Please file an issue for it over here, and provide with the details given below: https://github.com/chrisvdm/redwoodjs-stripe/issues/new",
api |
api | **Context:**
api | retrieveStripeCustomer:id
api |
api | **Data:**
api | { id: '<customer_id>', addProps: null }
api |
api |     err: {
api |       "type": "GraphQLError",
api |       "message": "Unexpectedly received null or undefined data. This may be a bug in redwoodjs-stripe.\n\nPlease file an issue for it over here, and provide with the details given below: https://github.com/chrisvdm/redwoodjs-stripe/issues/new\",\n\n**Context:**\nretrieveStripeCustomer:id\n\n**Data:**\n{ id: '<customer_id>', addProps: null }\n",
api |       "stack":
api |           Error: Unexpectedly received null or undefined data. This may be a bug in redwoodjs-stripe.
api |
api |           Please file an issue for it over here, and provide with the details given below: https://github.com/chrisvdm/redwoodjs-stripe/issues/new",
api |
api |           **Context:**
api |           retrieveStripeCustomer:id
api |
api |           **Data:**
api |           { id: '<customer_id>', addProps: null }
api |
api |               at nonNilAssertionError (/Users/matt/workspace/moontower-rwjs/node_modules/@redwoodjs-stripe/api/dist/cjs/lib/nonNilAssertionError.js:25:49)
api |               at Object.retrieveStripeCustomer (/Users/matt/workspace/moontower-rwjs/node_modules/@redwoodjs-stripe/api/dist/cjs/services/customers/customers.js:44:64)
api |               at Object.retrieveStripeCustomer (/Users/matt/workspace/moontower-rwjs/node_modules/@redwoodjs/graphql-server/dist/makeMergedSchema.js:97:30)
api |               at useRedwoodDirectiveValidatorResolver (/Users/matt/workspace/moontower-rwjs/node_modules/@redwoodjs/graphql-server/dist/plugins/useRedwoodDirective.js:82:22)
api |               at executeField (/Users/matt/workspace/moontower-rwjs/node_modules/@graphql-tools/executor/cjs/execution/execute.js:324:24)
api |               at executeFields (/Users/matt/workspace/moontower-rwjs/node_modules/@graphql-tools/executor/cjs/execution/execute.js:272:28)
api |               at executeOperation (/Users/matt/workspace/moontower-rwjs/node_modules/@graphql-tools/executor/cjs/execution/execute.js:232:18)
api |               at /Users/matt/workspace/moontower-rwjs/node_modules/@graphql-tools/executor/cjs/execution/execute.js:69:64
api |               at new ValueOrPromise (/Users/matt/workspace/moontower-rwjs/node_modules/value-or-promise/src/ValueOrPromise.ts:35:15)
api |               at executeImpl (/Users/matt/workspace/moontower-rwjs/node_modules/@graphql-tools/executor/cjs/execution/execute.js:69:20)
api |       "path": [
api |         "retrieveStripeCustomer"
api |       ],
api |       "locations": [
api |         {
api |           "line": 2,
api |           "column": 3
api |         }
api |       ],
api |       "extensions": {}
api |     }
api | [13:13:05.115] DEBUG: Processing GraphQL Parameters done.
api | [13:13:05.116] INFO: request completed
api |     reqId: "req-8"
api |     res: {
api |       "statusCode": 200
api |     }
api |     responseTime: 19.324374988675117

Based on the other examples in customers.ts, this assertion is meant to catch null values (== null). It's currently flipped (!== null). If this is the intended behavior, we should use a different assertion.

casey-matt commented 3 weeks ago

Hey @chrisvdm, this is a tiny 1-line change to an edge case check in the retrieveStripeCustomer function. I tested it in my application locally & it fixes the error, but you're probably better suited to know if this result in unexpected impact somewhere.

I figured I'd just submit the PR instead to make it easier, instead of filing an issue.

chrisvdm commented 2 weeks ago

Hey @casey-matt, thanks so much for this. Much appreciated 👍

chrisvdm commented 2 weeks ago

Released!