Issue with while (true) loop inside notification.handler.js

[joakimmillen]

There is a while loop in notification.handler.js that behaves a bit unpredictable in different environments. I can't reproduce the issue locally but in our production environment it occurs at every call to the function.

adyen-commercetools/notification/src/handler/notification/notification.handler.js

Line 88 in 97bee4e

while (true) {

Issue description

The while loop does not actually wait for the await statement inside the try block, leading to potentially huge amount of logs. We have this running in azure functions and one of invocations of this function had around 30 thousand logs of Update payment with key..... Running the function locally does not produce the issue but when i deploy a fix to our staging environment it seems to fix the excess logging.

Solution proposal

Instead of a while (true) {} loop just wrap everything in a recursive function that you call in the catch block if maxRetry isn't reached yet. Something like this:

async function updatePaymentWithRepeater(
  payment,
  notification,
  ctpClient,
  logger,
) {
  const maxRetry = 20
  let currentPayment = payment
  let currentVersion = payment.version
  let retryCount = 0
  let retryMessage
  let updateActions
  const repeater = async () => {
    updateActions = await calculateUpdateActionsForPayment(
      currentPayment,
      notification,
      logger,
    )
    if (updateActions.length === 0) {
      return;
    }
    logger.debug(
      `Update payment with key ${
        currentPayment.key
      } with update actions [${JSON.stringify(updateActions)}]`,
    )
    try {
      /* eslint-disable-next-line no-await-in-loop */
      await ctpClient.update(
        ctpClient.builder.payments,
        currentPayment.id,
        currentVersion,
        updateActions,
      )
      logger.debug(
        `Payment with key ${currentPayment.key} was successfully updated`,
      )
      return;
    } catch (err) {
      const moduleConfig = config.getModuleConfig()
      let updateActionsToLog = updateActions
      if (moduleConfig.removeSensitiveData)
        updateActionsToLog =
          _obfuscateNotificationInfoFromActionFields(updateActions)

      if (err.statusCode !== 409) {
        const errMsg =
          `Unexpected error on payment update with ID: ${currentPayment.id}.` +
          `Failed actions: ${JSON.stringify(updateActionsToLog)}`
        throw new VError(err, errMsg)
      }

      retryCount += 1
      if (retryCount > maxRetry) {
        retryMessage =
          'Got a concurrent modification error' +
          ` when updating payment with id "${currentPayment.id}".` +
          ` Version tried "${currentVersion}",` +
          ` currentVersion: "${err.body.errors[0].currentVersion}".`
        throw new VError(
          err,
          `${retryMessage} Won't retry again` +
            ` because of a reached limit ${maxRetry}` +
            ` max retries. Failed actions: ${JSON.stringify(
              updateActionsToLog,
            )}`,
        )
      } else (
				repeater()
			)
      /* eslint-disable-next-line no-await-in-loop */
      const response = await ctpClient.fetchById(
        ctpClient.builder.payments,
        currentPayment.id,
      )
      currentPayment = response.body // eslint-disable-line prefer-destructuring
      currentVersion = currentPayment.version
    }
  }
  return repeater()
}

[logeecom]

Hey @joakimmillen,

We have validated your request and it is correct. We have used the code you have provided, slightly refactored it, and tested it. It is working correctly. This will be included in the next release.

Best regards.

[logeecom]

Hello @joakimmillen,

Note that we’ve released a new version of the integration.

Kind regards.