fix(express): fix express overriding 2xx status

changed the redirectRequest method so it returns the original status from WP not only the body of
the response

WP-2961

TICKET: WP-2961

Author: alebusse

modules/express/src/clientRoutes.ts

@@ -1114,7 +1114,13 @@ async function handleNetworkV1EnterpriseClientConnections(
  * @param req
  * @param next
  */
-function redirectRequest(bitgo: BitGo, method: string, url: string, req: express.Request, next: express.NextFunction) {
+export function redirectRequest(
+  bitgo: BitGo,
+  method: string,
+  url: string,
+  req: express.Request,
+  next: express.NextFunction
+) {
   let request;
 
   switch (method) {
@@ -1142,7 +1148,11 @@ function redirectRequest(bitgo: BitGo, method: string, url: string, req: express
     if (req.params.enterpriseId) {
       request.set('enterprise-id', req.params.enterpriseId);
     }
-    return request.result();
+
+    return request.result().then((result) => {
+      const status = request.res?.statusCode || 200;
+      return { status, body: result };
+    });
   }
 
   // something has presumably gone wrong
@@ -1217,8 +1227,7 @@ function prepareBitGo(config: Config) {
     next();
   };
 }
-
-type RequestHandlerResponse = string | unknown | undefined;
+type RequestHandlerResponse = string | unknown | undefined | { status: number; body: unknown };
 interface RequestHandler extends express.RequestHandler<ParamsDictionary, any, RequestHandlerResponse> {
   (req: express.Request, res: express.Response, next: express.NextFunction):
     | RequestHandlerResponse
@@ -1260,12 +1269,17 @@ function handleRequestHandlerError(res: express.Response, error: unknown) {
  * Promise handler wrapper to handle sending responses and error cases
  * @param promiseRequestHandler
  */
-function promiseWrapper(promiseRequestHandler: RequestHandler) {
+export function promiseWrapper(promiseRequestHandler: RequestHandler) {
   return async function promWrapper(req: express.Request, res: express.Response, next: express.NextFunction) {
     debug(`handle: ${req.method} ${req.originalUrl}`);
     try {
       const result = await promiseRequestHandler(req, res, next);
-      res.status(200).send(result);
+      if (typeof result === 'object' && result !== null && 'body' in result && 'status' in result) {
+        const { status, body } = result as { status: number; body: unknown };
+        res.status(status).send(body);
+      } else {
+        res.status(200).send(result);
+      }
     } catch (e) {
       handleRequestHandlerError(res, e);
     }

modules/express/test/unit/clientRoutes/index.ts

@@ -0,0 +1,137 @@
+import 'should';
+import * as sinon from 'sinon';
+import * as express from 'express';
+import { ApiResponseError, BitGo } from 'bitgo';
+import { promiseWrapper, redirectRequest } from '../../../src/clientRoutes';
+
+describe('common methods', () => {
+  describe('redirectRequest', () => {
+    let bitgo: BitGo;
+    let req: express.Request;
+    let next: express.NextFunction;
+
+    beforeEach(() => {
+      bitgo = new BitGo({ env: 'test' });
+      req = {
+        body: {},
+        params: {},
+        bitgo,
+      } as express.Request;
+      next = () => undefined;
+    });
+
+    afterEach(() => {
+      sinon.restore();
+    });
+
+    it('should handle GET request and return status and body', async () => {
+      const url = 'https://example.com/api';
+      const response = { res: { statusCode: 200 }, result: async () => ({ success: true }) };
+      sinon
+        .stub(bitgo, 'get')
+        .withArgs(url)
+        .returns(response as any);
+
+      const result = await redirectRequest(bitgo, 'GET', url, req, next);
+      result.status.should.equal(200);
+      result.body.should.deepEqual({ success: true });
+    });
+
+    it('should handle POST request and return status and body', async () => {
+      const url = 'https://example.com/api';
+      req.body = { data: 'test' };
+      const response = { res: { statusCode: 201 }, result: async () => ({ success: true }) };
+      sinon
+        .stub(bitgo, 'post')
+        .withArgs(url)
+        .returns({ send: () => response } as any);
+
+      const result = await redirectRequest(bitgo, 'POST', url, req, next);
+      result.status.should.equal(201);
+      result.body.should.deepEqual({ success: true });
+    });
+
+    it('should handle error response and return status and body', async () => {
+      const url = 'https://example.com/api';
+      const response = {
+        result: async () => {
+          throw new ApiResponseError('Bad Request', 400);
+        },
+      };
+      sinon
+        .stub(bitgo, 'get')
+        .withArgs(url)
+        .returns(response as any);
+
+      await redirectRequest(bitgo, 'GET', url, req, next).should.be.rejectedWith(ApiResponseError, {
+        message: 'Bad Request',
+        status: 400,
+      });
+    });
+  });
+
+  describe('promiseWrapper', () => {
+    afterEach(() => {
+      sinon.restore();
+    });
+    it('should handle successful request', async () => {
+      const handler = sinon.stub().resolves({ status: 200, body: { success: true } });
+      const req: any = {};
+      const res: any = {
+        status: sinon.stub().returnsThis(),
+        send: sinon.stub().returnsThis(),
+      };
+      const next = sinon.stub();
+
+      await promiseWrapper(handler)(req, res, next);
+
+      res.status.calledWith(200).should.be.true();
+      res.send.calledWith({ success: true }).should.be.true();
+    });
+
+    it('should handle successful request with status 202', async () => {
+      const handler = sinon.stub().resolves({ status: 202, body: { success: true } });
+      const req: any = {};
+      const res: any = {
+        status: sinon.stub().returnsThis(),
+        send: sinon.stub().returnsThis(),
+      };
+      const next = sinon.stub();
+
+      await promiseWrapper(handler)(req, res, next);
+
+      res.status.calledWith(202).should.be.true();
+      res.send.calledWith({ success: true }).should.be.true();
+    });
+
+    it('should handle successful request with default status', async () => {
+      const handler = sinon.stub().resolves({ success: true });
+      const req: any = {};
+      const res: any = {
+        status: sinon.stub().returnsThis(),
+        send: sinon.stub().returnsThis(),
+      };
+      const next = sinon.stub();
+
+      await promiseWrapper(handler)(req, res, next);
+
+      res.status.calledWith(200).should.be.true();
+      res.send.calledWith({ success: true }).should.be.true();
+    });
+
+    it('should handle error request', async () => {
+      const handler = sinon.stub().rejects(new Error('Test error'));
+      const req: any = {};
+      const res: any = {
+        status: sinon.stub().returnsThis(),
+        send: sinon.stub().returnsThis(),
+      };
+      const next = sinon.stub();
+
+      await promiseWrapper(handler)(req, res, next);
+
+      res.status.calledWith(500).should.be.true();
+      res.send.calledWithMatch((result: any) => result.message === 'Test error').should.be.true();
+    });
+  });
+});