fix: return json error response body as object

etc/ibm-cloud-sdk-core.api.md

@@ -309,6 +309,9 @@ export function isFileWithMetadata(obj: any): obj is FileWithMetadata;
 // @public
 export function isHTML(text: string): boolean;
 
+// @public
+export function isJsonMimeType(mimeType: string): boolean;
+
 // @public
 export class JwtTokenManager extends TokenManager {
     constructor(options: JwtTokenManagerOptions);

lib/helper.ts

@@ -1,5 +1,5 @@
 /**
- * (C) Copyright IBM Corp. 2014, 2022.
+ * (C) Copyright IBM Corp. 2014, 2023.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -340,3 +340,14 @@ export function constructServiceUrl(
 
   return formattedUrl;
 }
+
+/**
+ * Returns true if and only if "mimeType" is a "JSON-like" mime type
+ * (e.g. "application/json; charset=utf-8").
+ * @param mimeType - the mimeType string
+ * @returns true if "mimeType" represents a JSON media type and false otherwise
+ */
+export function isJsonMimeType(mimeType: string) {
+  logger.debug(`Determining if the mime type '${mimeType}' specifies JSON content.`);
+  return !!mimeType && /^application\/json(\s*;.*)?$/i.test(mimeType);
+}

lib/request-wrapper.ts

@@ -30,6 +30,7 @@ import {
   isEmptyObject,
   isFileData,
   isFileWithMetadata,
+  isJsonMimeType,
   stripTrailingSlash,
 } from './helper';
 import logger from './logger';
@@ -266,7 +267,7 @@ export class RequestWrapper {
 
         // the other sdks use the interface `result` for the body
         // eslint-disable-next-line @typescript-eslint/dot-notation
-        res['result'] = res.data;
+        res['result'] = ensureJSONResponseBodyIsObject(res);
         delete res.data;
 
         // return another promise that resolves with 'res' to be handled in generated code
@@ -306,20 +307,19 @@ export class RequestWrapper {
 
       error.message = parseServiceErrorMessage(axiosError.data) || axiosError.statusText;
 
-      // some services bury the useful error message within 'data'
-      // adding it to the error under the key 'body' as a string or object
+      // attach the error response body to the error
       let errorBody;
       try {
-        // try/catch to handle objects with circular references
+        // try/catch to detect objects with circular references
         errorBody = JSON.stringify(axiosError.data);
       } catch (e) {
-        // ignore the error, use the object, and tack on a warning
+        logger.warn('Error field `result` contains circular reference(s)');
+        logger.debug(`Failed to stringify error response body: ${e}`);
         errorBody = axiosError.data;
-        errorBody.warning = 'Body contains circular reference';
-        logger.error(`Failed to stringify axiosError: ${e}`);
       }
 
-      error.body = errorBody;
+      error.result = ensureJSONResponseBodyIsObject(axiosError);
+      error.body = errorBody; // ** deprecated **
 
       // attach headers to error object
       error.headers = axiosError.headers;
@@ -545,3 +545,36 @@ function parseServiceErrorMessage(response: any): string | undefined {
   logger.info(`Parsing service error message: ${message}`);
   return message;
 }
+
+/**
+ * Check response for a JSON content type and a string-formatted body. If those
+ * conditions are met, we want to return an object for the body to the user. If
+ * the JSON string coming from the service is invalid, we want to raise an
+ * exception.
+ *
+ * @param response - incoming response object
+ * @returns response body - as either an object or a string
+ * @throws error - if the content is meant as JSON but is malformed
+ */
+function ensureJSONResponseBodyIsObject(response: any): any | string {
+  if (typeof response.data !== 'string' || !isJsonMimeType(response.headers['content-type'])) {
+    return response.data;
+  }
+
+  // If the content is supposed to be JSON but axios gave us a string, it is most
+  // likely due to the fact that the service sent malformed JSON, which is an error.
+  //
+  // We'll try to parse the string and return a proper object to the user but if
+  // it fails, we'll log an error and raise an exception.
+
+  let dataAsObject = response.data;
+  try {
+    dataAsObject = JSON.parse(response.data);
+  } catch (e) {
+    logger.error('Response body was supposed to have JSON content but JSON parsing failed.');
+    logger.error(`Malformed JSON string: ${response.data}`);
+    throw e;
+  }
+
+  return dataAsObject;
+}

test/unit/is-json-mime-type.test.js

@@ -0,0 +1,80 @@
+/**
+ * (C) Copyright IBM Corp. 2023.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+const { isJsonMimeType } = require('../../dist/lib/helper');
+const logger = require('../../dist/lib/logger').default;
+
+const debugLogSpy = jest.spyOn(logger, 'debug').mockImplementation(() => {});
+
+describe('isJsonMimeType()', () => {
+  afterEach(() => {
+    debugLogSpy.mockClear();
+  });
+
+  it('should return `false` for `undefined`', async () => {
+    expect(isJsonMimeType(undefined)).toBe(false);
+    expect(debugLogSpy.mock.calls[0][0]).toBe(
+      "Determining if the mime type 'undefined' specifies JSON content."
+    );
+  });
+
+  it('should return `false` for `null`', async () => {
+    expect(isJsonMimeType(null)).toBe(false);
+    expect(debugLogSpy.mock.calls[0][0]).toBe(
+      "Determining if the mime type 'null' specifies JSON content."
+    );
+  });
+
+  it('should return `false` for empty-string', async () => {
+    expect(isJsonMimeType('')).toBe(false);
+    expect(debugLogSpy.mock.calls[0][0]).toBe(
+      "Determining if the mime type '' specifies JSON content."
+    );
+  });
+
+  it('should return `false` for non-JSON mimetype', async () => {
+    expect(isJsonMimeType('application/octect-stream')).toBe(false);
+    expect(isJsonMimeType('text/plain')).toBe(false);
+    expect(isJsonMimeType('multipart/form-data; charset=utf-8')).toBe(false);
+    expect(debugLogSpy.mock.calls[0][0]).toBe(
+      "Determining if the mime type 'application/octect-stream' specifies JSON content."
+    );
+    expect(debugLogSpy.mock.calls[1][0]).toBe(
+      "Determining if the mime type 'text/plain' specifies JSON content."
+    );
+    expect(debugLogSpy.mock.calls[2][0]).toBe(
+      "Determining if the mime type 'multipart/form-data; charset=utf-8' specifies JSON content."
+    );
+  });
+
+  it('should return `true` for a JSON mimetype', async () => {
+    expect(isJsonMimeType('application/json')).toBe(true);
+    expect(isJsonMimeType('application/json;charset=utf-8')).toBe(true);
+    expect(debugLogSpy.mock.calls[0][0]).toBe(
+      "Determining if the mime type 'application/json' specifies JSON content."
+    );
+    expect(debugLogSpy.mock.calls[1][0]).toBe(
+      "Determining if the mime type 'application/json;charset=utf-8' specifies JSON content."
+    );
+  });
+
+  it('should return `true` for a JSON mimetype including optional whitespace', async () => {
+    expect(isJsonMimeType('application/json  ; charset=utf-8')).toBe(true);
+    expect(debugLogSpy.mock.calls[0][0]).toBe(
+      "Determining if the mime type 'application/json  ; charset=utf-8' specifies JSON content."
+    );
+  });
+});