Release v1.42.2 (#1024)

* Add additional logging for worker status
* Add retries for Postman email and SMS

Author: kevinkim-ogp

package-lock.json

@@ -106,5 +106,5 @@
     "tsconfig-paths": "^4.2.0",
     "type-fest": "4.10.3"
   },
-  "version": "1.42.1"
+  "version": "1.42.2"
 }

packages/backend/package.json

@@ -73,19 +73,43 @@ describe('Postman SMS request error handler', () => {
   })
 
   describe('Other errors', () => {
-    it('throws StepError with generic error message', async () => {
+    it.each([500, 502, 503])('should retry on %s', async (status) => {
       const axiosError = {
         isAxiosError: true,
         name: 'AxiosError',
         message: 'Internal server error',
         response: {
-          status: 500,
+          status,
         },
       } as unknown as AxiosError
 
       const error = new HttpError(axiosError)
 
-      await expect(requestErrorHandler($, error)).rejects.toThrow(StepError)
+      await expect(requestErrorHandler($, error)).rejects.toThrow(
+        RetriableError,
+      )
+      await expect(requestErrorHandler($, error)).rejects.toMatchObject({
+        delayType: 'step',
+        delayInMs: 3000,
+      })
+    })
+
+    it('should retry on read ETIMEDOUT', async () => {
+      const axiosError = {
+        isAxiosError: true,
+        name: 'AxiosError',
+        message: 'read ETIMEDOUT',
+      } as unknown as AxiosError
+
+      const error = new HttpError(axiosError)
+
+      await expect(requestErrorHandler($, error)).rejects.toThrow(
+        RetriableError,
+      )
+      await expect(requestErrorHandler($, error)).rejects.toMatchObject({
+        delayType: 'step',
+        delayInMs: 3000,
+      })
     })
   })
 })

packages/backend/src/apps/postman-sms/__tests__/request-error-handler.test.ts

@@ -21,32 +21,53 @@ const handle429: ThrowingHandler = (_, error): never => {
   })
 }
 
+const handle500and502and503: ThrowingHandler = (_, error): never => {
+  const status = error.response.status
+  throw new RetriableError({
+    error: `Retrying HTTP ${status} from Postman SMS`,
+    delayType: 'step',
+    delayInMs: 'default',
+  })
+}
+
 const requestErrorHandler: IApp['requestErrorHandler'] = async function (
   $,
   error,
 ) {
-  if (error.response.status === 429) {
-    return handle429($, error)
-  }
+  switch (error.response.status) {
+    case 429:
+      return handle429($, error)
+    case 500:
+    case 502:
+    case 503:
+      return handle500and502and503($, error)
+    default:
+      if (error.message === 'read ETIMEDOUT') {
+        throw new RetriableError({
+          error: `Retrying ${error.message} from Postman SMS`,
+          // pausing the entire queue here is not a good idea because we wont be able to fully utilize
+          // the throughput that the campaign supports, so we throw step error here instead of group
+          delayType: 'step',
+          delayInMs: 'default',
+        })
+      }
 
-  if (
-    error.response.status === 400 &&
-    error.response.data.error?.code === 'parameter_invalid'
-  ) {
-    throw new StepError(
-      'Campaign template was not set up correctly',
-      'Ensure that you have followed the instructions in our guide to set up your campaign template.',
-      $.step.position,
-      $.app.name,
-    )
-  }
+      if (error.response.data.error?.code === 'parameter_invalid') {
+        throw new StepError(
+          'Campaign template was not set up correctly',
+          'Ensure that you have followed the instructions in our guide to set up your campaign template.',
+          $.step.position,
+          $.app.name,
+        )
+      }
 
-  throw new StepError(
-    'Error sending SMS',
-    error.message,
-    $.step.position,
-    $.app.name,
-  )
+      throw new StepError(
+        'Error sending SMS',
+        error.message,
+        $.step.position,
+        $.app.name,
+      )
+  }
 }
 
 export default requestErrorHandler

packages/backend/src/apps/postman-sms/common/request-error-handler.ts

@@ -350,11 +350,13 @@ describe('send transactional email', () => {
     })
   })
 
-  it('should retry on 502, 504, 520', async () => {
+  it('should retry on 500, 502, 504, 520, 524', async () => {
     const recipients = [
       '[email protected]',
       '[email protected]',
       '[email protected]',
+      '[email protected]',
+      '[email protected]',
     ]
     $.step.parameters.destinationEmail = recipients.join(',')
     $.http.post = vi
@@ -387,11 +389,72 @@ describe('send transactional email', () => {
           },
         } as AxiosError),
       )
+      .mockRejectedValueOnce(
+        new HttpError({
+          response: {
+            data: '<html>cloudflare error</html>',
+            status: 520,
+            statusText: 'Web server is returning an unknown error',
+          },
+        } as AxiosError),
+      )
+      .mockRejectedValueOnce(
+        new HttpError({
+          response: {
+            data: '<html>cloudflare error</html>',
+            status: 524,
+            statusText: 'A timeout occurred',
+          },
+        } as AxiosError),
+      )
+
+    await expect(sendTransactionalEmail.run($)).rejects.toThrow(RetriableError)
+    expect($.setActionItem).toHaveBeenCalledWith({
+      raw: {
+        status: [
+          'ACCEPTED',
+          'INTERMITTENT-ERROR',
+          'INTERMITTENT-ERROR',
+          'INTERMITTENT-ERROR',
+          'INTERMITTENT-ERROR',
+        ],
+        recipient: recipients,
+        subject: 'test subject',
+        body: 'test body',
+        from: 'jack',
+        reply_to: '[email protected]',
+      },
+    })
+  })
 
+  it('should retry on socket hang up', async () => {
+    const recipients = ['[email protected]', '[email protected]']
+    $.step.parameters.destinationEmail = recipients.join(',')
+    $.http.post = vi
+      .fn()
+      .mockResolvedValueOnce({
+        data: {
+          params: {
+            body: 'test body',
+            subject: 'test subject',
+            from: 'jack',
+            reply_to: '[email protected]',
+          },
+        },
+      })
+      .mockRejectedValueOnce(
+        new HttpError({
+          response: {
+            data: 'socket hang up',
+            status: 400,
+            statusText: 'socket hang up',
+          },
+        } as AxiosError),
+      )
     await expect(sendTransactionalEmail.run($)).rejects.toThrow(RetriableError)
     expect($.setActionItem).toHaveBeenCalledWith({
       raw: {
-        status: ['ACCEPTED', 'ERROR', 'INTERMITTENT-ERROR'],
+        status: ['ACCEPTED', 'ERROR'],
         recipient: recipients,
         subject: 'test subject',
         body: 'test body',

packages/backend/src/apps/postman/__tests__/actions/send-transactional-email.test.ts

@@ -16,7 +16,7 @@ type PostmanApiErrorData = {
 // These are HTTP error codes returned by Cloudflare, which likely indicate
 // that Postman's origin server did not receive the request.
 // Until this is fixed, we will retry these requests on behalf of the user
-const POSTMAN_RETRIABLE_HTTP_CODES = [502, 504, 520]
+const POSTMAN_RETRIABLE_HTTP_CODES = [500, 502, 504, 520, 524]
 
 export function getPostmanErrorStatus(
   error: HttpError,
@@ -121,6 +121,14 @@ export function throwPostmanStepError({
       })
     case 'ERROR':
     default:
+      if (error.message === 'socket hang up') {
+        throw new RetriableError({
+          error: `Retrying ${error.message} from Postman`,
+          delayInMs: 'default',
+          delayType: 'step',
+        })
+      }
+
       throw new StepError(
         'Something went wrong',
         'Please contact [email protected] for assistance.',

packages/backend/src/apps/postman/common/throw-errors.ts

@@ -1,5 +1,7 @@
 import ioRedis from 'ioredis'
 
+import logger from '@/helpers/logger'
+
 import appConfig from './app'
 
 // Maximum of 16; be careful when adding!
@@ -13,6 +15,7 @@ export const REDIS_DB_INDEX = {
 
 function reconnectOnError(err: Error) {
   const targetError = 'READONLY'
+  logger.error('Redis connection error', err)
   if (err.message.includes(targetError)) {
     // Only reconnect when the error contains "READONLY"
     // during node failover, this is thrown: 149: -READONLY You can't write against a read only replica.

packages/backend/src/config/redis.ts

@@ -67,6 +67,14 @@ worker.on('failed', (job, err) => {
   )
 })
 
+worker.on('ready', () => {
+  logger.info('Flow worker is ready!')
+})
+
+worker.on('closed', () => {
+  logger.info('Flow worker is closed!')
+})
+
 worker.on('error', (err) => {
   if (!err) {
     logger.error('Worker undefined error')

packages/backend/src/workers/flow.ts

@@ -278,6 +278,14 @@ export function makeActionWorker(
     }
   })
 
+  worker.on('ready', () => {
+    logger.info(`[${queueName}] Worker is ready!`)
+  })
+
+  worker.on('closed', () => {
+    logger.info(`[${queueName}] Worker is closed!`)
+  })
+
   worker.on('error', (err) => {
     if (!err) {
       logger.error(`[${queueName}] Worker had undefined error`)

packages/backend/src/workers/helpers/make-action-worker.ts

@@ -66,6 +66,14 @@ worker.on('failed', (job, err) => {
   )
 })
 
+worker.on('ready', () => {
+  logger.info('Trigger worker is ready!')
+})
+
+worker.on('closed', () => {
+  logger.info('Trigger worker is closed!')
+})
+
 worker.on('error', (err) => {
   if (!err) {
     logger.error('Worker undefined error')