shopify-api-js icon indicating copy to clipboard operation
shopify-api-js copied to clipboard

Shopify.Webhooks.Registry.process doesn't terminate

Open david-wb opened this issue 4 years ago • 9 comments

Issue summary

Write a short description of the issue here ↓

I am setting up an app-uninstalled webhook as in this tutorial, but it seems the webhook handler hangs at

      await Shopify.Webhooks.Registry.process(ctx.req, ctx.res);

The full handler code is here

  router.post("/webhooks", async (ctx) => {
    // eslint-disable-next-line no-console
    console.log(`Webhook request received`);

    try {
      // NEXT LINE HANGS FOREVER
      await Shopify.Webhooks.Registry.process(ctx.req, ctx.res);
    } catch (err) {
      console.error(err);
      err.status = err.statusCode || err.status || 500;
      throw err;
    }

    // eslint-disable-next-line no-console
    console.log(`Webhook processed with status code 200`);
  });

And the webhook handler is registered like this

  server.use(
    createShopifyAuth({
      async afterAuth(ctx) {
        const { shop, scope, accessToken } = ctx.state.shopify;
        await firestore.saveShop(shop, scope, accessToken);

        const registration = await Shopify.Webhooks.Registry.register({
          shop,
          accessToken,
          path: "/webhooks",
          topic: "APP_UNINSTALLED",
          webhookHandler: async (_topic, shop, _body) => {
            // eslint-disable-next-line no-console
            console.info(`App uninstalled from ${shop}`);
            await firestore.deleteShop(shop);
          },
        });

        if (registration.success) {
          // eslint-disable-next-line no-console
          console.info("Successfully registered APP_UNINSTALLED webhook.");
        } else {
          // eslint-disable-next-line no-console
          console.info(
            "Failed to register APP_UNINSTALLED webhook",
            registration.result
          );
        }

        ctx.redirect(
          `https://${CWA_HOST}/shopify&shop=${shop}&accessToken=${accessToken}`
        );
      },
    })
  );

Any ideas Shopify.Webhooks.Registry.process simply hangs without throwing an error?

Expected behavior

What do you think should happen?

Actual behavior

What actually happens?

Tip: include an error message (in a <details></details> tag) if your issue is related to an error

Steps to reproduce the problem

Reduced test case

The best way to get your bug fixed is to provide a reduced test case.


Checklist

  • [x] I have described this issue in a way that is actionable (if possible)

david-wb avatar Apr 28 '21 12:04 david-wb

I'm having the same issue : timeout on await Shopify.Webhooks.Registry.register.

Relevant code :

Shopify.Webhooks.Registry.webhookRegistry = [
  {
    path: "/handleWebhooks",
    topic: "APP_UNINSTALLED",
    webhookHandler: handleUninstall.bind(this)
  },
  {
    path: "/handleWebhooks",
    topic: "THEMES_UPDATE",
    webhookHandler: handleThemeChange.bind(this)
  }
];

export const handleWebhooks = async (
  req: functions.https.Request,
  res: functions.Response<any>
) => {
  try{
    if(!Shopify.Webhooks.Registry.isWebhookPath(req.path)) {
      functions.logger.error("not a webhook path", req.path);
      return;
    }

    await Shopify.Webhooks.Registry.process(req, res);
  } catch(error) {
    functions.logger.error("failed processing webhook", error);
  }
}

thibautvdu avatar May 05 '21 15:05 thibautvdu

Hey there,

I had the same issue and after a few hours of pissing around I finally worked out why it stalls. I'm running a serverless function that had some body parser middleware running. Basically you just need to turn that off.

Under the hood, when you call Shopify.Webhooks.Registry.process(), the req.on() event callbacks would be messed up by the body parser and stall the request.

Might not be the exact problem for you, but worth giving it a try.

sam-potter avatar May 10 '21 06:05 sam-potter

@saampotter

Thanks a lot ! After digging a bit in the code, I was suspecting something of that flavor with the req.on callbacks. The issue is that I'm using firebase functions, so if there is indeed some kind of body parsing, I can't do much about it...

I ended up re-implementing the hmac check and WebhookRegistry.process feature myself, but I'd prefer to stick with the shpify way of things.

thibautvdu avatar May 11 '21 09:05 thibautvdu

It would be great if we didn't need the work around to disable the body parser for this to work. Is this possible at all?

dan-turner avatar Jun 23 '21 00:06 dan-turner

@saampotter I just faced the same issue using body parser as middleware. Did you able to find any workaround ?

atiqueahmedziad avatar Jul 06 '21 11:07 atiqueahmedziad

@atiqueahmedziad I simply disabled body parser middleware for the specific webhook route. Ended up implementing the HMAC verification myself because I'm not a fan of the API design of this package.

sam-potter avatar Jul 07 '21 06:07 sam-potter

+1 for the request to have Shopify.Webhooks.Registry.process not reliant on req.on('end'), which causes it to silently break when using bodyParser or express.json(). (Or, at bare, minimum, to include a warning about this in the documentation! It's a pretty gnarly dependency to not call attention to in the developer onboarding guide)

bishpls avatar Jan 21 '22 20:01 bishpls

router.post('/webhooks', async (ctx) => {
      try {
        await Shopify.Webhooks.Registry.process(ctx.req, ctx.res);
        console.log(`Webhook processed, returned status code 200`);
      } catch (error) {
        console.log(`Failed to process webhook: ${error}`);
      }
    });

    server.use(bodyParser());

I put the bodyParser below the route but it still hangs forever. How can I disable the bodyparser without removing the code line?

pnyennhi avatar Feb 25 '22 08:02 pnyennhi

This issue is mainly because of body parser or express.json() middleware. Try this code to exclude body parser or express.json()

function excludeShopifyWebhookEndpoint(fn) {
    return function (req, res, next) {
        if (req.path === '/api/webhooks' && req.method === 'POST') {
            next();
        } else {
            fn(req, res, next);
        }
    }
}

app.use(excludeShopifyWebhookEndpoint(express.json()));

Note: Above solution is applicable for stripe and shopify webhooks :)

aditodkar avatar Jul 05 '22 11:07 aditodkar

Documentation regarding body parsers here

Note that webhook processing in v6.0.0 will be changing (along with just about everything else!) ... release candidate v6.0.0-rc1 is available.

mkevinosullivan avatar Sep 27 '22 19:09 mkevinosullivan