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

Error when enabling Shopify inventory hooks #3974

Closed brent-hoover closed 6 years ago

brent-hoover commented 6 years ago

Issue Description

Receive "Error setting up Shopify Sync" in browser and error in server console when enabling shopify inventory sync

Steps to Reproduce

  1. Login as admin
  2. Go to "Connectors" panel
  3. Select "Update Inventory when new Shopify order is placed"
  4. Click on "Setup Sync"
  5. Observe errors in server and browser
  server-error Shopify API Error creating new webhook: Response code 422 (Unprocessable Entity) { HTTPError: Response code 422 (Unprocessable Entity)
      at stream.catch.then.data (/Users/brenthoover/Projects/js/reaction/node_modules/got/index.js:341:13)
      at /Users/brenthoover/.meteor/packages/promise/.0.10.1.trxnxh.msck++os+web.browser+web.cordova/npm/node_modules/meteor-promise/fiber_pool.js:43:40
   => awaited here:
      at Function.Promise.await (/Users/brenthoover/.meteor/packages/promise/.0.10.1.trxnxh.msck++os+web.browser+web.cordova/npm/node_modules/meteor-promise/promise_server.js:56:12)
      at Promise.asyncApply (imports/plugins/included/connectors-shopify/server/methods/webhooks/webhooks.js:55:13)

Versions

Node: 8.9.4
NPM: 5.6.0
Meteor Node: 8.9.4
Meteor NPM: 5.6.0
Reaction CLI: 0.28.0
Reaction: 1.9.0
Reaction branch: release-1.9.0
Docker: 17.12.0-ce
brent-hoover commented 6 years ago

This is not a regression as it happens in master/1.8.2 as well

Akarshit commented 6 years ago

This happens if you try to setup the sync again on the same address. We see this because we keep resetting our databases which removes the history of webhook locally, but still use the same URL to pipe to localhost. The acutal error is address: [The address has already been taken]

Should we do something about this?

brent-hoover commented 6 years ago

Does the API allow you to reset and then set the webhooks?

brent-hoover commented 6 years ago

I agree with the change in impact status, but in the future try to put a note like "bumping down from major to minor because it only affects you when trying to set the hooks multiple times".

Akarshit commented 6 years ago

@zenweasel Why would we want to delete the webhook and then recreate the webhook with the same callback address?

brent-hoover commented 6 years ago

Because it would make this idempotent.

Akarshit commented 6 years ago

To delete a webhook we need the Shopify's Id of the webhook. This id is stored in the database. But if we are able to get the id of the webhook, that means the webhook data was already present in the database and the control would never reach where new webhook is created.

brent-hoover commented 6 years ago

So do we need to query to see if there is already an existing webhook and get that data?

Akarshit commented 6 years ago

We will have to make a request to get all the webhooks subscription for the shop. Then we can check if those already contain the webhooks we are trying to make and skip the creation accordingly.

brent-hoover commented 6 years ago

That seems like a solid plan

jamesporl commented 6 years ago

Fixed by https://github.com/reactioncommerce/reaction/pull/4148 so I'm closing this.