Better SNS publish errors (#140)

kibertoad · May 6, 2024 · 63891f3 · 63891f3
1 parent 4efd1e6
commit 63891f3
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 11 deletions.
diff --git a/packages/amqp/lib/AbstractAmqpPublisher.ts b/packages/amqp/lib/AbstractAmqpPublisher.ts
@@ -1,4 +1,5 @@
 import type { Either } from '@lokalise/node-core'
+import { InternalError } from '@lokalise/node-core'
 import type {
   BarrierResult,
   MessageInvalidFormatError,
@@ -58,7 +59,17 @@ export abstract class AbstractAmqpPublisher<MessagePayloadType extends object>
         this.logger.error(`AMQP channel closed`)
         void this.reconnect()
       } else {
-        throw err
+        throw new InternalError({
+          message: `Error while publishing to AMQP ${(err as Error).message}`,
+          errorCode: 'AMQP_PUBLISH_ERROR',
+          details: {
+            publisher: this.constructor.name,
+            queueName: this.queueName,
+            // @ts-ignore
+            messageType: message[this.messageTypeField] ?? 'unknown',
+          },
+          cause: err as Error,
+        })
       }
     }
   }

diff --git a/packages/amqp/test/publishers/AmqpPermissionPublisher.spec.ts b/packages/amqp/test/publishers/AmqpPermissionPublisher.spec.ts
@@ -1,3 +1,4 @@
+import type { InternalError } from '@lokalise/node-core'
 import { waitAndRetry } from '@lokalise/node-core'
 import type { Channel } from 'amqplib'
 import type { AwilixContainer } from 'awilix'
@@ -134,22 +135,45 @@ describe('PermissionPublisher', () => {
     })
 
     it('publish unexpected message', async () => {
-      let error: unknown
+      expect.assertions(3)
       try {
         permissionPublisher.publish({
           hello: 'world',
           messageType: 'add',
           // eslint-disable-next-line @typescript-eslint/no-explicit-any
         } as any)
-      } catch (e) {
-        error = e
+      } catch (error) {
+        expect(error).toBeDefined()
+        expect(error).toBeInstanceOf(Error)
+        expect(error).toBeInstanceOf(ZodError)
       }
-      expect(error).toBeDefined()
-      expect(error).toBeInstanceOf(Error)
-      expect(error).toBeInstanceOf(ZodError)
     })
 
-    it('publish message with uns Unsupported message type', async () => {
+    it('return details if publish failed', async () => {
+      expect.assertions(3)
+      try {
+        await permissionPublisher.close()
+        permissionPublisher.publish({
+          id: '11',
+          messageType: 'add',
+        })
+      } catch (error) {
+        expect(error).toBeInstanceOf(Error)
+        expect((error as InternalError).message).toMatchInlineSnapshot(
+          `"Error while publishing to AMQP Cannot read properties of undefined (reading 'sendToQueue')"`,
+        )
+        expect((error as InternalError).details).toMatchInlineSnapshot(`
+          {
+            "messageType": "add",
+            "publisher": "AmqpPermissionPublisher",
+            "queueName": "user_permissions_multi",
+          }
+        `)
+      }
+      await diContainer.cradle.amqpConnectionManager.reconnect()
+    })
+
+    it('publish message with unsupported message type', async () => {
       let error: unknown
       try {
         permissionPublisher.publish({

diff --git a/packages/sns/lib/sns/AbstractSnsPublisher.ts b/packages/sns/lib/sns/AbstractSnsPublisher.ts
@@ -1,6 +1,7 @@
 import { PublishCommand } from '@aws-sdk/client-sns'
 import type { PublishCommandInput } from '@aws-sdk/client-sns/dist-types/commands/PublishCommand'
-import type { Either } from '@lokalise/node-core'
+import type { Either} from '@lokalise/node-core';
+import { InternalError } from '@lokalise/node-core'
 import type {
   AsyncPublisher,
   BarrierResult,
@@ -77,8 +78,19 @@ export abstract class AbstractSnsPublisher<MessagePayloadType extends object>
       await this.snsClient.send(command)
       this.handleMessageProcessed(message, 'published')
     } catch (error) {
-      this.handleError(error)
-      throw error
+      const err = error as Error
+      this.handleError(err)
+      throw new InternalError({
+        message: `Error while publishing to SNS: ${err.message}`,
+        errorCode: 'SNS_PUBLISH_ERROR',
+        details: {
+          publisher: this.constructor.name,
+          topic: this.topicArn,
+          // @ts-ignore
+          messageType: message[this.messageTypeField] ?? 'unknown',
+        },
+        cause: err,
+      })
     }
   }
 

diff --git a/packages/sns/test/publishers/SnsPermissionPublisher.spec.ts b/packages/sns/test/publishers/SnsPermissionPublisher.spec.ts
@@ -1,5 +1,6 @@
 import type { SNSClient } from '@aws-sdk/client-sns'
 import type { SQSClient } from '@aws-sdk/client-sqs'
+import type { InternalError} from '@lokalise/node-core';
 import { waitAndRetry } from '@lokalise/node-core'
 import type { SQSMessage } from '@message-queue-toolkit/sqs'
 import { assertQueue, deleteQueue, FakeConsumerErrorResolver } from '@message-queue-toolkit/sqs'
@@ -183,6 +184,48 @@ describe('SnsPermissionPublisher', () => {
       consumer.stop()
     })
 
+    it('preserves message metadata in a publish error message', async () => {
+      expect.assertions(1)
+      const { permissionPublisher } = diContainer.cradle
+      const permissions: [string, ...string[]] = ['perm']
+      for (let i = 0; i < 5000; i++) {
+        permissions.push('really-long-permissions-for-testing-excessively-large-payloads')
+      }
+
+      const message = {
+        id: '1',
+        userIds,
+        messageType: 'add',
+        permissions,
+      } satisfies PERMISSIONS_MESSAGE_TYPE
+
+      await subscribeToTopic(
+        sqsClient,
+        snsClient,
+        {
+          QueueName: queueName,
+        },
+        {
+          Name: SnsPermissionPublisher.TOPIC_NAME,
+        },
+        {
+          updateAttributesIfExists: false,
+        },
+      )
+
+      try {
+        await permissionPublisher.publish(message)
+      } catch (err) {
+        expect((err as InternalError).details).toMatchInlineSnapshot(`
+          {
+            "messageType": "add",
+            "publisher": "SnsPermissionPublisher",
+            "topic": "arn:aws:sns:eu-west-1:000000000000:user_permissions_multi",
+          }
+        `)
+      }
+    })
+
     it('publish message with lazy loading', async () => {
       const newPublisher = new SnsPermissionPublisher(diContainer.cradle)