Webhooks not registering

Chadyka commented 1 year ago

I've been playing around with your implementation but I can't get webhooks to work (multi-store API connections seem flawless though). The app context seems to load without an api key first when the webhook process runs, because only the shop query parameter sets the correct api keys. Would love to hear your take on this.

This is my setup in express

app.use(createMultistoreMiddleware(shopify));

app.get(
  shopifyAppConfig.auth.path,
  useShopifyApp((shopifyApp) => shopifyApp.auth.begin())
);

app.get(
  shopifyAppConfig.auth.callbackPath,
  useShopifyApp((shopifyApp) => shopifyApp.auth.callback()),
  useShopifyApp((shopifyApp) => shopifyApp.redirectToShopifyOrAppRoot())
);

app.post(
  shopifyAppConfig.webhooks.path,
  useShopifyApp((shopifyApp) =>
    shopifyApp.processWebhooks({ webhookHandlers: InventoryHandlers })
  )
);

shopify fn to choose the right keys

export const shopifyAppConfig: Pick<AppConfigParams, "auth" | "webhooks"> = {
  auth: {
    path: "/api/auth",
    callbackPath: "/api/auth/callback",
  },
  webhooks: {
    path: "/api/webhooks",
  },
};

export const shopify = (shop: string): ShopifyApp =>
  shopifyApp({
    api: {
      apiVersion: LATEST_API_VERSION,
      restResources,
      apiKey: TOKENS[`SHOPIFY_API_KEY_${shop}`],
      apiSecretKey: TOKENS[`SHOPIFY_API_SECRET_${shop}`],
    },
    auth: shopifyAppConfig.auth,
    webhooks: shopifyAppConfig.webhooks,
    sessionStorage: sessionDb,
  });

Webhook handlers

export const InventoryHandlers: { [key: string]: WebhookHandler } = {
  PRODUCTS_UPDATE: {
    deliveryMethod: DeliveryMethod.Http,
    callbackUrl: shopifyAppConfig.webhooks.path,
    callback: async (topic, shop, body, webhookId, apiVersion) => {
      const payload = JSON.parse(body);
      console.log(payload);
      console.log(topic, shop, webhookId, apiVersion);
    },
  },
  CUSTOMERS_DATA_REQUEST: {
    deliveryMethod: DeliveryMethod.Http,
    callbackUrl: shopifyAppConfig.webhooks.path,
    callback: async (topic, shop, body, webhookId) => {
      const payload = JSON.parse(body);
    },
  },
  CUSTOMERS_REDACT: {
    deliveryMethod: DeliveryMethod.Http,
    callbackUrl: shopifyAppConfig.webhooks.path,
    callback: async (topic, shop, body, webhookId) => {
      const payload = JSON.parse(body);
    },
  },
  SHOP_REDACT: {
    deliveryMethod: DeliveryMethod.Http,
    callbackUrl: shopifyAppConfig.webhooks.path,
    callback: async (topic, shop, body, webhookId) => {
      const payload = JSON.parse(body);
    },
  },
};

I get this error on startup

and no matter what I do, even after shopify-app/INFO logs that it's registering webhooks, res.locals.shopify.app.api.webhooks.getTopicsAdded() is always an empty array

Anyways, thanks for the multi store suggestion in the first place and I hope you have some time to look at this.

paweljedrzejczyk commented 1 year ago

@Chadyka first thing you can try is rewriting your shopify app create function to only create the app once for given shop.

const shopifyAppMap: Record<string, ShopifyApp> = {};

export const shopify = (shop: string): ShopifyApp => {
  if (shopifyAppMap[shop]) {
    return shopifyAppMap[shop];
  }

  const app = shopifyApp({
    api: {
      apiVersion: LATEST_API_VERSION,
      restResources,
      apiKey: TOKENS[`SHOPIFY_API_KEY_${shop}`],
      apiSecretKey: TOKENS[`SHOPIFY_API_SECRET_${shop}`],
    },
    auth: shopifyAppConfig.auth,
    webhooks: shopifyAppConfig.webhooks,
    sessionStorage: sessionDb,
  });

  shopifyAppMap[shop] = app;

  return app;
};

Let me know how that works. I will investigate further. Also can you try to post more logs since the app starts up to this errors you can getting in:

2023-03-01 08:35:53 | webhooks | APP_UNINSTALLED webhook delivery failed
2023-03-01 08:35:53 | backend | TypeError: Cannot read properties of undefined (reading 'app')
2023-03-01 08:35:53 | backend | at getShopifyApp @multistoreEnv.ts:83:22)

Chadyka commented 1 year ago

No luck with this at the moment.

It fails the same way as before when reinstalling the app on a store with yarn dev --reset, but might be unrelated, because it only happens on reinstalling the app.

2023-03-01 10:19:36 | frontend |
2023-03-01 10:19:36 | frontend | > dev
2023-03-01 10:19:36 | frontend | > conc "tsc -w" "vite"
2023-03-01 10:19:36 | frontend |
2023-03-01 10:19:36 | backend  |
2023-03-01 10:19:36 | backend  | > dev
2023-03-01 10:19:36 | backend  | > cross-env NODE_ENV=development nodemon --exec node --loader ts-node/esm index.ts --ignore ../frontend
2023-03-01 10:19:36 | backend  |
2023-03-01 10:19:36 | backend  | [nodemon] 2.0.20
2023-03-01 10:19:36 | backend  | [nodemon] to restart at any time, enter `rs`
2023-03-01 10:19:36 | backend  | [nodemon] watching path(s): *.*
2023-03-01 10:19:36 | backend  | [nodemon] watching extensions: ts,json
2023-03-01 10:19:36 | backend  | [nodemon] starting `node --loader ts-node/esm index.ts`
2023-03-01 10:19:36 | backend  | (node:143973) ExperimentalWarning: Custom ESM Loaders is an experimental feature. This feature could change at any time
2023-03-01 10:19:36 | backend  | (Use `node --trace-warnings ...` to show where the warning was created)
2023-03-01 10:19:36 | frontend | [0] 
2023-03-01 10:19:36 | frontend |
2023-03-01 10:19:36 | frontend | 11:19:36 AM - Starting compilation in watch mode...
2023-03-01 10:19:36 | frontend | [0] 
2023-03-01 10:19:36 | frontend | [1] 
2023-03-01 10:19:36 | frontend |
2023-03-01 10:19:36 | frontend | [1]   vite v2.9.15 dev server running at:
2023-03-01 10:19:36 | frontend | [1] 
2023-03-01 10:19:36 | frontend | [1]   > Local:    http://localhost:34717/
2023-03-01 10:19:36 | frontend | [1]   > Network:  http://192.168.8.161:34717/
2023-03-01 10:19:36 | frontend | [1]   > Network:  http://172.22.0.1:34717/
2023-03-01 10:19:36 | frontend | [1] 
2023-03-01 10:19:36 | frontend | [1]   ready in 541ms.
2023-03-01 10:19:36 | frontend | [1] 
2023-03-01 10:19:37 | webhooks | Sending APP_UNINSTALLED webhook to app server
2023-03-01 10:19:37 | webhooks | App isn't responding yet, retrying in 5 seconds
2023-03-01 10:19:39 | frontend | [0] 
2023-03-01 10:19:39 | frontend |
2023-03-01 10:19:39 | frontend | [0] 11:19:39 AM - Found 0 errors. Watching for file changes.
2023-03-01 10:19:39 | frontend | [1] 
2023-03-01 10:19:39 | frontend | [1] [TypeScript] Found 0 errors. Watching for file changes.
2023-03-01 10:19:42 | webhooks | APP_UNINSTALLED webhook delivery failed
2023-03-01 10:19:42 | backend  | TypeError: Cannot read properties of undefined (reading 'app')
2023-03-01 10:19:42 | backend  |     at getShopifyApp (file:///home/chadyka/Code/webhook-app/web/backend/helpers/multistoreEnv.ts:83:22)
2023-03-01 10:19:42 | backend  |     at file:///home/chadyka/Code/webhook-app/web/backend/helpers/multistoreEnv.ts:92:24
2023-03-01 10:19:42 | backend  |     at Layer.handle [as handle_request] (/home/chadyka/Code/webhook-app/web/backend/node_modules/express/lib/router/layer.js:95:5)
2023-03-01 10:19:42 | backend  |     at next (/home/chadyka/Code/webhook-app/web/backend/node_modules/express/lib/router/route.js:144:13)
2023-03-01 10:19:42 | backend  |     at Route.dispatch (/home/chadyka/Code/webhook-app/web/backend/node_modules/express/lib/router/route.js:114:3)
2023-03-01 10:19:42 | backend  |     at Layer.handle [as handle_request] (/home/chadyka/Code/webhook-app/web/backend/node_modules/express/lib/router/layer.js:95:5)
2023-03-01 10:19:42 | backend  |     at /home/chadyka/Code/webhook-app/web/backend/node_modules/express/lib/router/index.js:284:15
2023-03-01 10:19:42 | backend  |     at Function.process_params (/home/chadyka/Code/webhook-app/web/backend/node_modules/express/lib/router/index.js:346:12)
2023-03-01 10:19:42 | backend  |     at next (/home/chadyka/Code/webhook-app/web/backend/node_modules/express/lib/router/index.js:280:10)
2023-03-01 10:19:42 | backend  |     at file:///home/chadyka/Code/webhook-app/web/backend/helpers/multistoreEnv.ts:79:5

This happens on the second yarn dev, says registering webhooks as an info (I guessed it means it was successfull), however in the end when the page loads and the API is called, res.locals.shopify.app.api.webhooks.getTopicsAdded() prints empty array

2023-03-01 10:31:12 | backend  |
2023-03-01 10:31:12 | backend  | > dev
2023-03-01 10:31:12 | backend  | > cross-env NODE_ENV=development nodemon --exec node --loader ts-node/esm index.ts --ignore ../frontend
2023-03-01 10:31:12 | backend  |
2023-03-01 10:31:12 | frontend |
2023-03-01 10:31:12 | frontend | > dev
2023-03-01 10:31:12 | frontend | > conc "tsc -w" "vite"
2023-03-01 10:31:12 | frontend |
2023-03-01 10:31:12 | backend  | [nodemon] 2.0.20
2023-03-01 10:31:12 | backend  | [nodemon] to restart at any time, enter `rs`
2023-03-01 10:31:12 | backend  | [nodemon] watching path(s): *.*
2023-03-01 10:31:12 | backend  | [nodemon] watching extensions: ts,json
2023-03-01 10:31:12 | backend  | [nodemon] starting `node --loader ts-node/esm index.ts`
2023-03-01 10:31:12 | backend  | (node:157779) ExperimentalWarning: Custom ESM Loaders is an experimental feature. This feature could change at any time
2023-03-01 10:31:12 | backend  | (Use `node --trace-warnings ...` to show where the warning was created)
2023-03-01 10:31:13 | frontend | [0] 
2023-03-01 10:31:13 | frontend |
2023-03-01 10:31:13 | frontend | 11:31:13 AM - Starting compilation in watch mode...
2023-03-01 10:31:13 | frontend | [0] 
2023-03-01 10:31:13 | frontend | [1] 
2023-03-01 10:31:13 | frontend |
2023-03-01 10:31:13 | frontend | [1]   vite v2.9.15 dev server running at:
2023-03-01 10:31:13 | frontend | [1] 
2023-03-01 10:31:13 | frontend | [1]   > Local:    http://localhost:34215/
2023-03-01 10:31:13 | frontend | [1]   > Network:  http://192.168.8.161:34215/
2023-03-01 10:31:13 | frontend | [1]   > Network:  http://172.22.0.1:34215/
2023-03-01 10:31:13 | frontend | [1] 
2023-03-01 10:31:13 | frontend | [1]   ready in 499ms.
2023-03-01 10:31:13 | frontend | [1] 
2023-03-01 10:31:15 | frontend | [0] 
2023-03-01 10:31:15 | frontend |
2023-03-01 10:31:15 | frontend | [0] 11:31:15 AM - Found 0 errors. Watching for file changes.
2023-03-01 10:31:16 | frontend | [1] 
2023-03-01 10:31:16 | frontend |
2023-03-01 10:31:16 | frontend | [1] [TypeScript] Found 0 errors. Watching for file changes.
2023-03-01 10:31:20 | backend  | [shopify-api/INFO] version 6.1.0, environment Node v18.12.0
2023-03-01 10:31:20 | backend  | [shopify-app/INFO] Running ensureInstalledOnShop
2023-03-01 10:31:20 | backend  | [shopify-app/INFO] Found a session, but it is not valid. Redirecting to auth | {shop: webhook-dev.myshopify.com}
2023-03-01 10:31:20 | backend  | [shopify-api/INFO] Beginning OAuth | {shop: webhook-dev.myshopify.com, isOnline: false, callbackPath: /api/auth/callback}
2023-03-01 10:31:28 | backend  | [shopify-api/INFO] version 6.1.0, environment Node v18.12.0
2023-03-01 10:31:28 | backend  | [shopify-app/INFO] Handling request to complete OAuth process
2023-03-01 10:31:28 | backend  | [shopify-api/INFO] Completing OAuth | {shop: webhook-dev.myshopify.com}
2023-03-01 10:31:28 | backend  | [shopify-api/INFO] Creating new session | {shop: webhook-dev.myshopify.com, isOnline: false}
2023-03-01 10:31:28 | backend  | [shopify-api/INFO] Registering webhooks | {shop: webhook-dev.myshopify.com}
2023-03-01 10:31:32 | backend  | [shopify-api/INFO] version 6.1.0, environment Node v18.12.0
2023-03-01 10:31:32 | backend  | [shopify-app/INFO] Running ensureInstalledOnShop
2023-03-01 10:31:32 | backend  | [shopify-app/INFO] App is installed and ready to load | {shop: webhook-dev.myshopify.com}
2023-03-01 10:31:37 | backend  | [shopify-api/INFO] version 6.1.0, environment Node v18.12.0
2023-03-01 10:31:37 | backend  | [shopify-app/INFO] Running validateAuthenticatedSession
2023-03-01 10:31:37 | backend  | [shopify-api/INFO] version 6.1.0, environment Node v18.12.0
2023-03-01 10:31:37 | backend  | [shopify-app/INFO] Running validateAuthenticatedSession
2023-03-01 10:31:38 | backend  | [shopify-app/INFO] Request session has a valid access token | {shop: webhook-dev.myshopify.com}
2023-03-01 10:31:38 | backend  | [shopify-app/INFO] Request session has a valid access token | {shop: webhook-dev.myshopify.com}
2023-03-01 10:31:38 | backend  | webhooks
2023-03-01 10:31:38 | backend  | []

paweljedrzejczyk commented 1 year ago

@Chadyka Can you paste code how are you registering webhooks? Is that automatic or you have some code written? In my test app I am not getting Registering webhooks log, so I might be missing something on my side.

Chadyka commented 1 year ago

@paweljedrzejczyk in the node example app I was registering webhoooks with a snippet like this after oauth finishes and redirects to root:

app.post(
  shopifyAppConfig.webhooks.path,
  useShopifyApp((shopifyApp) =>
    shopifyApp.processWebhooks({ webhookHandlers: InventoryHandlers })
  )
);

paweljedrzejczyk commented 1 year ago

@Chadyka Looks like I've had the shopify app caching already implemented in createMultistoreMiddleware, but it was not working correctly since the cache object was recreated every time. I've pushed a fix and release new version of the package https://github.com/paweljedrzejczyk/shopify-multistore-app-middleware/releases/tag/v0.0.3 You can try if it fixes your issue. Only one app is created for given shop so webhook handlers should be registered on that instance.

Chadyka commented 1 year ago

@paweljedrzejczyk This is a nice touch but it has not fixed webhooks for me. I was looking into the processWebhooks() implementation and it seems to be just like other middleware that is used for your code. Also, it is setting the APP_UNINSTALLED webhook on it's own, which is also not registering in my app. I'll do some more digging I guess.

export function processWebhooks({
  api,
  config,
}: ApiAndConfigParams): ProcessWebhooksMiddleware {
  return function ({webhookHandlers}: ProcessWebhooksMiddlewareParams) {
    mountWebhooks(api, config, webhookHandlers);

    return [
      express.text({type: '*/*'}),
      async (req: Request, res: Response) => {
        await process({
          req,
          res,
          api,
          config,
        });
      },
    ];
  };
}

function mountWebhooks(
  api: Shopify,
  config: AppConfigInterface,
  handlers: WebhookHandlersParam,
) {
  api.webhooks.addHandlers(handlers as AddHandlersParams);

  // Add our custom app uninstalled webhook
  const appInstallations = new AppInstallations(config);

  api.webhooks.addHandlers({
    APP_UNINSTALLED: {
      deliveryMethod: DeliveryMethod.Http,
      callbackUrl: config.webhooks.path,
      callback: deleteAppInstallationHandler(appInstallations, config),
    },
  });
}

paweljedrzejczyk commented 1 year ago

@Chadyka You can rewrite your code not to use my package for testing and see if the issue is causes by the package and working with multiple stores or it's just the way shopify js api works.

Chadyka commented 1 year ago

@paweljedrzejczyk that's where I started from. If I set an API key directly from env instead of the function implementation and I remove all the useShopifyApp() calls, it works as expected and registers the webhooks. This follows the node app template.

I also tried refractoring from this point to the new implementations you gave me but it causes the same error as before. I'll keep you posted in case I manage to handle this somehow.

elnygren commented 1 year ago

This library doesn't support webhooks. Following things are missing:

must check 'x-shopify-shop-domain' header for store -- webhooks do not send a ?shop param or Authorization header
Shopifys app.processWebhooks() returns a list of middleware, however, this library does not process them correctly.
- I don't think this is a correct way to do it. See something like https://stackoverflow.com/questions/29090331/how-to-make-dynamic-chain-of-middleware-in-express-js

davedc commented 1 year ago

@Chadyka did you manage to handle this? I'm taking a look at this at minute and has run into the same thing.

Chadyka commented 1 year ago

Hey @davedc, I couldn't resolve this issue with this library. This was a dead end for me and I ended up using 2 different apps hosted individually for the 2 stores in the end.

paweljedrzejczyk commented 1 year ago

@Chadyka @davedc I've fixed webhooks handling and released new version 0.1.0. https://github.com/paweljedrzejczyk/shopify-multistore-app-middleware/commit/c55e166b753451ee51f553418a70e6ea34bde5ec

Updated the dependencies to latest shopify packages as well.

Thanks @elnygren for feedback - that was the source of the issue.

Let me know if that new version works for you and we can close the issue.

Chadyka commented 1 year ago

@paweljedrzejczyk That's great to hear! I'll be testing this in the next couple of days. Have you taken a look at how the new app extension features work? Do you think app extensions could be compatible with this setup?

paweljedrzejczyk commented 1 year ago

@Chadyka no, I haven't looked into that. I am not currently working on Shopify project either at my current job or after hours so was not paying attention into Shopify world for a while.

daniyal741 commented 1 month ago

@paweljedrzejczyk , Can we please get a latest version for new @shopify/shopify-api pacakges?

daniyal741 commented 1 month ago

@Chadyka , Do you have any insights about it?

daniyal741 commented 1 month ago

@elnygren , Do you have any workaround for registering and processing the webhooks for the latest SHopify package? Thanks in advance.

paweljedrzejczyk / shopify-multistore-app-middleware