From 9cf859d0dfda58dfdd4a0f92d676aa2adfa39c37 Mon Sep 17 00:00:00 2001
From: Walter Moar <Walter.Moar@gov.bc.ca>
Date: Wed, 3 Jul 2024 13:25:05 -0700
Subject: [PATCH] fix: FORMS-995 handle address responses (#1422)

* fix: FORMS-995 improve error handling

* minor formatting

* fix a broken test
---
 app/src/components/errorToProblem.js          | 48 +++++++++++++------
 .../unit/components/errorToProblem.spec.js    | 21 ++++++--
 2 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/app/src/components/errorToProblem.js b/app/src/components/errorToProblem.js
index c6264bec8..bd01f91b0 100755
--- a/app/src/components/errorToProblem.js
+++ b/app/src/components/errorToProblem.js
@@ -2,28 +2,46 @@ const Problem = require('api-problem');
 
 const log = require('./log')(module.filename);
 
-module.exports = function (service, e) {
-  if (e.response) {
-    // Handle raw data
-    let data;
-    if (typeof e.response.data === 'string' || e.response.data instanceof String) {
-      data = JSON.parse(e.response.data);
-    } else {
-      data = e.response.data;
-    }
+/**
+ * Try to convert response data to JSON, but failing that just return it as-is.
+ *
+ * @param {*} data the data to attempt to parse into JSON.
+ * @returns an object if data is JSON, otherwise data itself
+ */
+const _parseResponseData = (data) => {
+  let parsedData;
+
+  try {
+    parsedData = JSON.parse(data);
+  } catch (error) {
+    // Syntax Error: It's not valid JSON.
+    parsedData = data;
+  }
+
+  return parsedData;
+};
+
+module.exports = function (service, error) {
+  if (error.response) {
+    const data = _parseResponseData(error.response.data);
+
+    log.error(`Error from ${service}: status = ${error.response.status}, data: ${JSON.stringify(data)}`, error);
 
-    log.error(`Error from ${service}: status = ${e.response.status}, data : ${JSON.stringify(data)}`, e);
     // Validation Error
-    if (e.response.status === 422) {
-      throw new Problem(e.response.status, {
+    if (error.response.status === 422) {
+      throw new Problem(error.response.status, {
         detail: data.detail,
         errors: data.errors,
       });
     }
+
     // Something else happened but there's a response
-    throw new Problem(e.response.status, { detail: e.response.data.toString() });
+    throw new Problem(error.response.status, { detail: error.response.data.toString() });
   } else {
-    log.error(`Unknown error calling ${service}: ${e.message}`, e);
-    throw new Problem(502, `Unknown ${service} Error`, { detail: e.message });
+    log.error(`Unknown error calling ${service}: ${error.message}`, error);
+
+    throw new Problem(502, `Unknown ${service} Error`, {
+      detail: error.message,
+    });
   }
 };
diff --git a/app/tests/unit/components/errorToProblem.spec.js b/app/tests/unit/components/errorToProblem.spec.js
index 7efcdaadb..58cc9d8b5 100644
--- a/app/tests/unit/components/errorToProblem.spec.js
+++ b/app/tests/unit/components/errorToProblem.spec.js
@@ -3,20 +3,33 @@ const errorToProblem = require('../../../src/components/errorToProblem');
 const SERVICE = 'TESTSERVICE';
 
 describe('errorToProblem', () => {
+  it('should throw a 404', () => {
+    const error = {
+      response: {
+        data: { detail: 'detail' },
+        status: 404,
+      },
+    };
+
+    expect(() => errorToProblem(SERVICE, error)).toThrow('404');
+  });
+
   it('should throw a 422', () => {
-    const e = {
+    const error = {
       response: {
         data: { detail: 'detail' },
         status: 422,
       },
     };
-    expect(() => errorToProblem(SERVICE, e)).toThrow('422');
+
+    expect(() => errorToProblem(SERVICE, error)).toThrow('422');
   });
 
   it('should throw a 502', () => {
-    const e = {
+    const error = {
       message: 'msg',
     };
-    expect(() => errorToProblem(SERVICE, e)).toThrow('502');
+
+    expect(() => errorToProblem(SERVICE, error)).toThrow('502');
   });
 });