reactioncommerce / reaction

Mailchimp Open Commerce is an API-first, headless commerce platform built using Node.js, React, GraphQL. Deployed via Docker and Kubernetes.
https://mailchimp.com/developer/open-commerce/
GNU General Public License v3.0
12.34k stars 2.17k forks source link

[Marketplace] Receive multiple errors in server console when there are Braintree orders #2622

Closed brent-hoover closed 7 years ago

brent-hoover commented 7 years ago

Expected behavior

No errors in server console

Actual Behavior

Receive the following errors when visiting the order panel where there have been Braintree orders

00:38:40.011Z ERROR Reaction: Cannot read property 'settings' of undefined
  TypeError: Cannot read property 'settings' of undefined
      at getAccountOptions (imports/plugins/included/payments-braintree/server/methods/braintreeApi.js:45:20)
      at getGateway (imports/plugins/included/payments-braintree/server/methods/braintreeApi.js:70:26)
      at Object._BraintreeApi.apiCall.listRefunds (imports/plugins/included/payments-braintree/server/methods/braintreeApi.js:205:19)
      at [object Object]._listRefunds (imports/plugins/included/payments-braintree/server/methods/braintreeMethods.js:142:51)
      at packages/check.js:129:16
      at [object Object]._.extend.withValue (packages/meteor.js:1126:17)
      at Object.exports.Match._failIfArgumentsAreNotAllChecked (packages/check.js:128:41)
      at maybeAuditArgumentChecks (packages/ddp-server/livedata_server.js:1765:18)
      at packages/ddp-server/livedata_server.js:1686:15
      at [object Object]._.extend.withValue (packages/meteor.js:1126:17)
      at packages/ddp-server/livedata_server.js:1684:36
      at [object Object]._.extend.applyAsync (packages/ddp-server/livedata_server.js:1683:12)
      at [object Object]._.extend.apply (packages/ddp-server/livedata_server.js:1622:26)
      at [object Object]._.extend.call (packages/ddp-server/livedata_server.js:1604:17)
      at [object Object].ordersRefundsList (server/methods/core/orders.js:817:12)
      at packages/check.js:129:16
00:38:40.011Z FATAL Reaction: Braintree call failed, refunds not listed

Steps to Reproduce the Behavior

  1. Login as admin
  2. Set payment processor as Braintree
  3. Place order
  4. Visit order dashboard and open order panel
  5. Observe errors

Versions

Node: 7.10.0
NPM: 5.0.3
Meteor Node: 4.8.4
Meteor NPM: 4.6.1
Reaction CLI: 0.11.0
Reaction: 1.5.0
Reaction branch: marketplace
Docker: 17.06.0-ce
commit: b6e18a4b5a35b852bd75648ae395a2f4b77eae81
brent-hoover commented 7 years ago

This error is more of an edge case than I had originally thought. It only occurs when you process payments via Braintree and then later disable BT (because you enable another payment method), so then the getRefunds method breaks because the package is not enabled. While it should probably handle this more gracefully this is also in the "just don't do that" category of workarounds.

foladipo commented 7 years ago

Here are some details about this bug and what fixed it:

The origin of the bug is getAccountOptions() in imports/plugins/included/payments-braintree/server/methods/braintreeApi.js. It was querying the DB for data about Braintree but assumed that BT would always be the payment processor in use, which is wrong. I took a look around for code that calls that function, and I found that it is only called by getGateway() in the same file. However, the latter function has multiple clients i.e all the other functions below it in that same file. And the only scenario where the BT plugin must be enabled is in paymentSubmit(). So, rather than make that a condition for all other functions, I made it a condition for that one only.

However, while searching around, I stumbled on: imports/plugins/included/payments-authnet/server/methods/authnet.js I noticed that it has an almost identical getAccountOptions(). I wondered why this hadn't thrown an error before, and checked again by testing it with the error-producing steps for the BT plugin. And guess what? It did. (For capturing payment, that is.) However, this one was kinda wrapped in a try...catch, so it was displayed by Logger or something instead of the error's entire stack trace. I fixed this one too, to make sure the plugin asserted that the Authorize.net plugin was enabled only when a user is making a payment.

brent-hoover commented 7 years ago

Closed via #2659