Fix "metadata.commercetoolsProjectKey" check and also return "accepte…

…d" by default. (#629)

Co-authored-by: Roman Butenko <[email protected]>

extension/package.json

@@ -1,6 +1,6 @@
 {
   "name": "commercetools-adyen-integration-extension",
-  "version": "7.1.0",
+  "version": "7.1.1",
   "description": "Integration between commercetools and Adyen payment service provider",
   "license": "MIT",
   "scripts": {

notification/index.googleFunction.js

@@ -24,8 +24,8 @@ exports.notificationTrigger = async (request, response) => {
       { notification: getNotificationForTracking(notificationItems), err },
       'Unexpected exception occurred.'
     )
-    if (err.isRecoverable) {
-      return response.status(400).send(err.message)
+    if (err.retry) {
+      return response.status(500).send(err.message)
     }
   }
 

notification/index.lambda.js

@@ -4,6 +4,7 @@ const { getNotificationForTracking } = require('./src/utils/commons')
 const { getCtpProjectConfig, getAdyenConfig } = require('./src/utils/parser')
 
 exports.handler = async (event) => {
+  // Reason for this check: if AWS API Gateway is used then event.body is provided as a string payload.
   const body = event.body ? JSON.parse(event.body) : event
   const { notificationItems } = body
   if (!notificationItems) {
@@ -33,7 +34,7 @@ exports.handler = async (event) => {
       { notification: getNotificationForTracking(notificationItems), err },
       'Unexpected exception occurred.'
     )
-    if (err.isRecoverable) {
+    if (err.retry) {
       throw err
     }
   }

notification/package.json

@@ -1,6 +1,6 @@
 {
   "name": "commercetools-adyen-integration-notification",
-  "version": "7.1.0",
+  "version": "7.1.1",
   "description": "Part of the integration of Adyen with commercetools responsible to receive and process notifications from Adyen",
   "scripts": {
     "check-coverage": "nyc check-coverage --statements 93",

notification/src/api/notification/notification.controller.js

@@ -30,7 +30,7 @@ async function handleNotification(request, response) {
       { notification: httpUtils.getNotificationForTracking(notification), err },
       'Unexpected exception occurred.'
     )
-    if (err.isRecoverable) {
+    if (err.retry) {
       return httpUtils.sendResponse(response, 500)
     }
     return sendAcceptedResponse(response)

notification/src/handler/notification/notification.handler.js

@@ -6,6 +6,28 @@ const { getNotificationForTracking } = require('../../utils/commons')
 const ctp = require('../../utils/ctp')
 const mainLogger = require('../../utils/logger').getLogger()
 
+class CommercetoolsError extends Error {
+  constructor({ stack, message, statusCode }) {
+    super()
+    this.stack = stack
+    this.message = message
+    this.retry = this._shouldRetry(statusCode)
+  }
+
+  /**
+   * recoverable: notification delivery can be retried by Adyen (return 500)
+   * non recoverable: notification delivery can not be retried by Adyen
+   * as it most probably would fail again (return "accepted")
+   *
+   * If commercetools status code is defined and is 5xx then return `500` to Adyen -> recoverable
+   * If during communication with commercetools we got a `NetworkError` then return `500` -> recoverable
+   * If commercetools status code is not OK but also not 5xx or 409 then return `accepted` -> non recoverable
+   */
+  _shouldRetry(statusCode) {
+    return statusCode < 200 || statusCode === 409 || statusCode >= 500
+  }
+}
+
 async function processNotification(
   notification,
   enableHmacSignature,
@@ -70,6 +92,9 @@ async function updatePaymentWithRepeater(
       currentPayment,
       notification
     )
+    if (updateActions.length === 0) {
+      break
+    }
     try {
       /* eslint-disable-next-line no-await-in-loop */
       await ctpClient.update(
@@ -83,23 +108,33 @@ async function updatePaymentWithRepeater(
       )
       break
     } catch (err) {
-      if (err.body.statusCode !== 409)
-        throw new Error(
-          `Unexpected error during updating a payment with ID: ${currentPayment.id}. Exiting. ` +
-            `Error: ${JSON.stringify(serializeError(err))}`
-        )
+      if (err.statusCode !== 409)
+        throw new CommercetoolsError({
+          stack: err.stack,
+          message:
+            `Unexpected error on payment update with ID: ${currentPayment.id}.` +
+            `Failed actions: ${JSON.stringify(
+              _obfuscateNotificationInfoFromActionFields(updateActions)
+            )}`,
+          statusCode: err.statusCode,
+        })
       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 Error(
-          `${retryMessage} Won't retry again` +
+        throw new CommercetoolsError({
+          stack: err.stack,
+          message:
+            `${retryMessage} Won't retry again` +
             ` because of a reached limit ${maxRetry}` +
-            ` max retries. Error: ${JSON.stringify(serializeError(err))}`
-        )
+            ` max retries. Failed actions: ${JSON.stringify(
+              _obfuscateNotificationInfoFromActionFields(updateActions)
+            )}`,
+          statusCode: err.statusCode,
+        })
       }
       /* eslint-disable-next-line no-await-in-loop */
       const response = await ctpClient.fetchById(
@@ -112,6 +147,19 @@ async function updatePaymentWithRepeater(
   }
 }
 
+function _obfuscateNotificationInfoFromActionFields(updateActions) {
+  const copyOfUpdateActions = _.cloneDeep(updateActions)
+  copyOfUpdateActions
+    .filter((value) => value.action === 'addInterfaceInteraction')
+    .filter((value) => value?.fields?.notification)
+    .forEach((value) => {
+      value.fields.notification = getNotificationForTracking(
+        JSON.parse(value.fields.notification)
+      )
+    })
+  return copyOfUpdateActions
+}
+
 function calculateUpdateActionsForPayment(payment, notification) {
   const updateActions = []
   const notificationRequestItem = notification.NotificationRequestItem
@@ -178,10 +226,9 @@ function compareTransactionStates(currentState, newState) {
     !transactionStateFlow.hasOwnProperty(currentState) ||
     !transactionStateFlow.hasOwnProperty(newState)
   )
-    throw Error(
-      'Wrong transaction state passed. ' +
-        `currentState: ${currentState}, newState: ${newState}`
-    )
+    throw new CommercetoolsError({
+      message: `Wrong transaction state passed. CurrentState: ${currentState}, newState: ${newState}`,
+    })
 
   return transactionStateFlow[newState] - transactionStateFlow[currentState]
 }
@@ -286,10 +333,14 @@ async function getPaymentByMerchantReference(merchantReference, ctpClient) {
     return result.body
   } catch (err) {
     if (err.statusCode === 404) return null
-    throw Error(
-      `Failed to fetch a payment with merchantReference: ${merchantReference}. ` +
-        `Error: ${JSON.stringify(serializeError(err))}`
-    )
+
+    throw new CommercetoolsError({
+      stack: err.stack,
+      message:
+        `Failed to fetch a payment with merchantReference: ${merchantReference}. ` +
+        `Error: ${JSON.stringify(serializeError(err))}`,
+      statusCode: err.statusCode,
+    })
   }
 }
 

notification/src/utils/parser.js

@@ -1,29 +1,30 @@
 const config = require('../config/config')
 
 class ValidationError extends Error {
-  constructor({ stack, message, notification, isRecoverable }) {
+  constructor({ stack, message }) {
     super()
     this.stack = stack
     this.message = message
-    this.notification = JSON.stringify(notification)
-    this.isRecoverable = isRecoverable
+
+    /*
+      recoverable: notification delivery can be retried by Adyen
+      non recoverable: notification delivery can not be retried by Adyen as it most probably would fail again
+
+      In this case, it's non recoverable, then return `accepted`.
+    */
+    this.retry = false
   }
 }
 
 function getCtpProjectConfig(notification) {
-  let commercetoolsProjectKey
-  try {
-    commercetoolsProjectKey =
-      notification.NotificationRequestItem.additionalData[
-        'metadata.commercetoolsProjectKey'
-      ]
-  } catch (e) {
+  const commercetoolsProjectKey =
+    notification?.NotificationRequestItem?.additionalData?.[
+      `metadata.commercetoolsProjectKey`
+    ]
+  if (!commercetoolsProjectKey) {
     throw new ValidationError({
-      stack: e.stack,
-      notification,
       message:
-        'Notification does not contain the field `metadata.commercetoolsProjectKey`.',
-      isRecoverable: false,
+        'Notification can not be processed as "commercetoolsProjectKey"  was not found on the notification.',
     })
   }
 
@@ -34,8 +35,6 @@ function getCtpProjectConfig(notification) {
     throw new ValidationError({
       stack: e.stack,
       message: e.message,
-      notification,
-      isRecoverable: true,
     })
   }
   return ctpProjectConfig
@@ -51,8 +50,6 @@ function getAdyenConfig(notification) {
     throw new ValidationError({
       stack: e.stack,
       message: e.message,
-      notification,
-      isRecoverable: true,
     })
   }
   return adyenConfig

notification/test/unit/lambda.spec.js

@@ -35,7 +35,36 @@ describe('Lambda handler', () => {
     expect(result).to.eql({ notificationResponse: '[accepted]' })
   })
 
-  it('logs unhandled exceptions', async () => {
+  it('throws and logs for unexpected exceptions', async () => {
+    const originalChildFn = logger.getLogger().child
+    try {
+      const logSpy = sinon.spy()
+      logger.getLogger().error = logSpy
+      logger.getLogger().child = () => ({
+        error: logSpy,
+      })
+
+      const error = new Error('some recoverable error')
+      error.retry = true
+      sinon.stub(notificationHandler, 'processNotification').throws(error)
+
+      const call = async () => handler(event)
+      await expect(call()).to.be.rejectedWith(error.message)
+
+      const notificationItem = event.notificationItems.pop()
+      logSpy.calledWith(
+        {
+          notification: getNotificationForTracking(notificationItem),
+          err: error,
+        },
+        'Unexpected error when processing event'
+      )
+    } finally {
+      logger.getLogger().child = originalChildFn
+    }
+  })
+
+  it('logs for retry=false exceptions and returns "accepted"', async () => {
     const originalChildFn = logger.getLogger().child
     try {
       const logSpy = sinon.spy()
@@ -45,10 +74,10 @@ describe('Lambda handler', () => {
       })
 
       const error = new Error('some error')
+      error.retry = false
       sinon.stub(notificationHandler, 'processNotification').throws(error)
 
       const result = await handler(event)
-
       expect(result).to.eql({ notificationResponse: '[accepted]' })
 
       const notificationItem = event.notificationItems.pop()