chore(point-of-sale): update pos service response structure (#3675)

* chore(point-of-sale): update pos service response structure

* fix: map cleanup in finally block

Author: sanducb

packages/point-of-sale/src/payments/routes.test.ts

@@ -63,21 +63,34 @@ describe('Payment Routes', () => {
         })
 
       await paymentRoutes.payment(ctx)
-      expect(ctx.response.body).toBe(Result.APPROVED)
+      expect(ctx.response.body).toEqual({ result: { code: Result.APPROVED } })
       expect(ctx.status).toBe(200)
 
       expect(webhookWaitMap.get('incoming-payment-url')).toBeUndefined()
     })
 
-    test('returns cardService error code when thrown', async () => {
+    test('returns 200 with invalid_signature result when card service returns invalid signature', async () => {
+      const ctx = createPaymentContext()
+      mockPaymentService()
+      jest
+        .spyOn(cardServiceClient, 'sendPayment')
+        .mockResolvedValueOnce(Result.INVALID_SIGNATURE)
+      await paymentRoutes.payment(ctx)
+      expect(ctx.response.body).toEqual({
+        result: { code: Result.INVALID_SIGNATURE }
+      })
+      expect(ctx.status).toBe(200)
+    })
+
+    test('returns 400 invalid_request body when an error is thrown', async () => {
       const ctx = createPaymentContext()
       mockPaymentService()
       jest
         .spyOn(cardServiceClient, 'sendPayment')
         .mockRejectedValue(new CardServiceClientError('Some error', 404))
       await paymentRoutes.payment(ctx)
-      expect(ctx.response.body).toBe('Some error')
-      expect(ctx.status).toBe(404)
+      expect(ctx.response.body).toEqual({ error: { code: 'invalid_request' } })
+      expect(ctx.status).toBe(400)
     })
 
     test('returns 400 when there is a paymentService error', async () => {
@@ -86,22 +99,22 @@ describe('Payment Routes', () => {
         .spyOn(paymentService, 'getWalletAddress')
         .mockRejectedValueOnce(new Error('Wallet address error'))
       await paymentRoutes.payment(ctx)
-      expect(ctx.response.body).toBe('Wallet address error')
+      expect(ctx.response.body).toEqual({ error: { code: 'invalid_request' } })
       expect(ctx.status).toBe(400)
     })
 
-    test('returns 500 when an unknown error is thrown', async () => {
+    test('returns 400 when an unknown error is thrown', async () => {
       const ctx = createPaymentContext()
       jest
         .spyOn(paymentService, 'getWalletAddress')
         .mockRejectedValueOnce('Unknown error')
       await paymentRoutes.payment(ctx)
-      expect(ctx.response.body).toBe('Unknown error')
-      expect(ctx.status).toBe(500)
+      expect(ctx.response.body).toEqual({ error: { code: 'invalid_request' } })
+      expect(ctx.status).toBe(400)
     })
 
     test(
-      'returns 504 if incoming payment event times out',
+      'returns 400 if incoming payment event times out',
       withConfigOverride(
         () => config,
         { webhookTimeoutMs: 1 },
@@ -118,10 +131,10 @@ describe('Payment Routes', () => {
             .spyOn(cardServiceClient, 'sendPayment')
             .mockResolvedValueOnce(Result.APPROVED)
           await paymentRoutes.payment(ctx)
-          expect(ctx.response.body).toBe(
-            'Timed out waiting for incoming payment event'
-          )
-          expect(ctx.status).toBe(504)
+          expect(ctx.response.body).toEqual({
+            error: { code: 'invalid_request' }
+          })
+          expect(ctx.status).toBe(400)
 
           expect(deleteSpy).toHaveBeenCalled()
         }

packages/point-of-sale/src/payments/routes.ts

@@ -2,15 +2,13 @@ import { AppContext } from '../app'
 import { CardServiceClient, Result } from '../card-service-client/client'
 import { BaseService } from '../shared/baseService'
 import { PaymentService } from './service'
-import { CardServiceClientError } from '../card-service-client/errors'
 import { Deferred } from '../utils/deferred'
 import { webhookWaitMap } from '../webhook-handlers/request-map'
 import { WebhookBody } from '../webhook-handlers/routes'
 import { IAppConfig } from '../config/app'
 import {
   IncomingPaymentEventTimeoutError,
-  InvalidCardPaymentError,
-  PaymentRouteError
+  InvalidCardPaymentError
 } from './errors'
 
 interface ServiceDependencies extends BaseService {
@@ -66,6 +64,7 @@ async function payment(
   ctx: PaymentContext
 ): Promise<void> {
   const body = ctx.request.body
+  let incomingPaymentId: string | undefined
   try {
     const senderWalletAddress = await deps.paymentService.getWalletAddress(
       body.senderWalletAddress.replace(/^https:/, 'http:')
@@ -89,6 +88,7 @@ async function payment(
       deferred,
       deps.config.webhookTimeoutMs
     )
+    incomingPaymentId = incomingPayment.id
     const result = await deps.cardServiceClient.sendPayment(
       senderWalletAddress.cardService,
       {
@@ -101,20 +101,27 @@ async function payment(
       }
     )
 
+    if (result === Result.INVALID_SIGNATURE) {
+      ctx.body = { result: { code: Result.INVALID_SIGNATURE } }
+      ctx.status = 200
+      return
+    }
+
     if (result !== Result.APPROVED) throw new InvalidCardPaymentError(result)
     const event = await waitForIncomingPaymentEvent(deps.config, deferred)
-    webhookWaitMap.delete(incomingPayment.id)
     if (!event || !event.data.completed)
       throw new IncomingPaymentEventTimeoutError(incomingPayment.id)
-    ctx.body = result
+    ctx.body = { result: { code: Result.APPROVED } }
     ctx.status = 200
   } catch (err) {
     deps.logger.debug(err)
-    if (err instanceof IncomingPaymentEventTimeoutError)
-      webhookWaitMap.delete(err.incomingPaymentId)
     const { body, status } = handlePaymentError(err)
     ctx.body = body
     ctx.status = status
+  } finally {
+    if (incomingPaymentId) {
+      webhookWaitMap.delete(incomingPaymentId)
+    }
   }
 }
 
@@ -130,15 +137,6 @@ async function waitForIncomingPaymentEvent(
   ])
 }
 
-function handlePaymentError(err: unknown) {
-  if (err instanceof CardServiceClientError) {
-    return { body: err.message, status: err.status }
-  }
-  if (err instanceof PaymentRouteError) {
-    return { body: err.message, status: err.status }
-  }
-  if (err instanceof Error) {
-    return { body: err.message, status: 400 }
-  }
-  return { body: err, status: 500 }
+function handlePaymentError(_err: unknown) {
+  return { body: { error: { code: 'invalid_request' } }, status: 400 }
 }

packages/point-of-sale/src/webhook-handlers/routes.test.ts

@@ -51,7 +51,7 @@ describe('Webhook Handler Routes Tests', (): void => {
     )
 
     await webhookHandlerRoutes.handleWebhook(ctx)
-    expect(ctx.status).toEqual(202)
+    expect(ctx.status).toEqual(200)
     expect(deferredSpy).toHaveBeenCalledWith(ctx.request.body)
   })
 

packages/point-of-sale/src/webhook-handlers/routes.ts

@@ -53,7 +53,7 @@ async function handleWebhook(
   const deferred = webhookWaitMap.get(id)
   if (deferred) {
     deferred.resolve(ctx.request.body)
-    ctx.status = 202
+    ctx.status = 200
   } else {
     ctx.throw(404, 'Not awaiting webhook for incoming payment id')
   }