Merge pull request #6425 from NMDSdevopsServiceAdm/fix/only-return-ex…

…pected-error-messages-for-certificates

Fix: Only return expected error messages from certificate endpoints

backend/server/routes/establishments/workerCertificate/qualificationCertificate.js

@@ -22,7 +22,7 @@ const requestUploadUrlEndpoint = async (req, res) => {
     );
     return res.status(200).json({ files: responsePayload });
   } catch (err) {
-    return res.status(err.statusCode).send(err.message);
+    return certificateService.sendErrorResponse(res, err);
   }
 };
 
@@ -33,7 +33,7 @@ const confirmUploadEndpoint = async (req, res) => {
     await certificateService.confirmUpload(req.body.files, req.params.qualificationUid);
     return res.status(200).send();
   } catch (err) {
-    return res.status(err.statusCode).send(err.message);
+    return certificateService.sendErrorResponse(res, err);
   }
 };
 
@@ -49,7 +49,7 @@ const getPresignedUrlForCertificateDownloadEndpoint = async (req, res) => {
     );
     return res.status(200).json({ files: responsePayload });
   } catch (err) {
-    return res.status(err.statusCode).send(err.message);
+    return certificateService.sendErrorResponse(res, err);
   }
 };
 
@@ -65,7 +65,7 @@ const deleteCertificatesEndpoint = async (req, res) => {
     );
     return res.status(200).send();
   } catch (err) {
-    return res.status(err.statusCode).send(err.message);
+    return certificateService.sendErrorResponse(res, err);
   }
 };
 

backend/server/routes/establishments/workerCertificate/trainingCertificate.js

@@ -8,16 +8,21 @@ const router = express.Router({ mergeParams: true });
 
 const initialiseCertificateService = () => {
   return WorkerCertificateService.initialiseTraining();
-}
+};
 
 const requestUploadUrlEndpoint = async (req, res) => {
   const certificateService = initialiseCertificateService();
 
   try {
-    const responsePayload = await certificateService.requestUploadUrl(req.body.files, req.params.id, req.params.workerId, req.params.trainingUid);
+    const responsePayload = await certificateService.requestUploadUrl(
+      req.body.files,
+      req.params.id,
+      req.params.workerId,
+      req.params.trainingUid,
+    );
     return res.status(200).json({ files: responsePayload });
   } catch (err) {
-    return res.status(err.statusCode).send(err.message);
+    return certificateService.sendErrorResponse(res, err);
   }
 };
 
@@ -28,29 +33,39 @@ const confirmUploadEndpoint = async (req, res) => {
     await certificateService.confirmUpload(req.body.files, req.params.trainingUid);
     return res.status(200).send();
   } catch (err) {
-    return res.status(err.statusCode).send(err.message);
+    return certificateService.sendErrorResponse(res, err);
   }
 };
 
 const getPresignedUrlForCertificateDownloadEndpoint = async (req, res) => {
   const certificateService = initialiseCertificateService();
 
   try {
-    const responsePayload = await certificateService.getPresignedUrlForCertificateDownload(req.body.files, req.params.id, req.params.workerId, req.params.trainingUid);
+    const responsePayload = await certificateService.getPresignedUrlForCertificateDownload(
+      req.body.files,
+      req.params.id,
+      req.params.workerId,
+      req.params.trainingUid,
+    );
     return res.status(200).json({ files: responsePayload });
   } catch (err) {
-    return res.status(err.statusCode).send(err.message);
+    return certificateService.sendErrorResponse(res, err);
   }
 };
 
 const deleteCertificatesEndpoint = async (req, res) => {
   const certificateService = initialiseCertificateService();
 
   try {
-    await certificateService.deleteCertificates(req.body.files, req.params.id, req.params.workerId, req.params.trainingUid);
+    await certificateService.deleteCertificates(
+      req.body.files,
+      req.params.id,
+      req.params.workerId,
+      req.params.trainingUid,
+    );
     return res.status(200).send();
   } catch (err) {
-    return res.status(err.statusCode).send(err.message);
+    return certificateService.sendErrorResponse(res, err);
   }
 };
 
@@ -63,4 +78,4 @@ module.exports = router;
 module.exports.requestUploadUrlEndpoint = requestUploadUrlEndpoint;
 module.exports.confirmUploadEndpoint = confirmUploadEndpoint;
 module.exports.getPresignedUrlForCertificateDownloadEndpoint = getPresignedUrlForCertificateDownloadEndpoint;
-module.exports.deleteCertificatesEndpoint = deleteCertificatesEndpoint;
+module.exports.deleteCertificatesEndpoint = deleteCertificatesEndpoint;

backend/server/routes/establishments/workerCertificate/workerCertificateService.js

@@ -251,6 +251,14 @@ class WorkerCertificateService {
     });
     await this.certificatesModel.destroy({ where: { uid: certificateRecordUids }, transaction: externalTransaction });
   }
+
+  sendErrorResponse(res, err) {
+    if (err instanceof HttpError) {
+      return res.status(err.statusCode).send(err.message);
+    }
+    console.error('WorkerCertificateService error: ', err);
+    return res.status(500).send('Internal server error');
+  }
 }
 
 module.exports = WorkerCertificateService;

backend/server/test/unit/routes/establishments/workerCertificate/workerCertificateService.spec.js

@@ -10,6 +10,7 @@ const s3 = require('../../../../../routes/establishments/workerCertificate/s3');
 const config = require('../../../../../config/config');
 
 const WorkerCertificateService = require('../../../../../routes/establishments/workerCertificate/workerCertificateService');
+const HttpError = require('../../../../../utils/errors/httpError');
 
 describe('backend/server/routes/establishments/workerCertificate/workerCertificateService.js', () => {
   const user = buildUser();
@@ -31,7 +32,6 @@ describe('backend/server/routes/establishments/workerCertificate/workerCertifica
   describe('requestUploadUrl', () => {
     const mockUploadFiles = ['cert1.pdf', 'cert2.pdf'];
     const mockSignedUrl = 'http://localhost/mock-upload-url';
-    let res;
 
     beforeEach(() => {
       mockRequestBody = {
@@ -641,4 +641,28 @@ describe('backend/server/routes/establishments/workerCertificate/workerCertifica
       ]);
     });
   });
+
+  describe('sendErrorResponse', () => {
+    let res;
+
+    beforeEach(() => {
+      res = httpMocks.createResponse();
+    });
+
+    it('should send status code and message from error when HttpError thrown', async () => {
+      const errorThrown = new HttpError('Invalid request', 400);
+
+      services.qualifications.sendErrorResponse(res, errorThrown);
+      expect(res.statusCode).to.equal(400);
+      expect(res._getData()).to.equal('Invalid request');
+    });
+
+    it('should send 500 status code and Internal server error message when unexpected error', async () => {
+      const errorThrown = new Error('Unexpected error');
+
+      services.qualifications.sendErrorResponse(res, errorThrown);
+      expect(res.statusCode).to.equal(500);
+      expect(res._getData()).to.equal('Internal server error');
+    });
+  });
 });