refactor: return detailed response as second callback argument (#34)

BREAKING CHANGE: The response body is no longer the 2nd callback argument, the detailed response is. The body is located under the `result` property. The `data` property is removed.
IBM · Oct 3, 2019 · dc24154 · dc24154
1 parent d47c737
commit dc24154
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 27 deletions.
diff --git a/lib/requestwrapper.ts b/lib/requestwrapper.ts
@@ -205,11 +205,16 @@ export class RequestWrapper {
 
     this.axiosInstance(requestParams)
     .then(res => {
+      // these objects contain circular json structures and are not always relevant to the user
+      // if the user wants them, they can be accessed through the debug properties
       delete res.config;
       delete res.request;
+
       // the other sdks use the interface `result` for the body
       res.result = res.data;
-      _callback(null, res.data, res);
+      delete res.data;
+
+      _callback(null, res);
     })
     .catch(error => {
       _callback(this.formatError(error));

diff --git a/migration-guide.md b/migration-guide.md
@@ -1,4 +1,9 @@
 # Migration Guide for v1
 
 ## Breaking Changes
+
+### Node Versions
 Node versions 6 and 8 are no longer supported.
+
+### Callback arguments
+The old callback argument structure of `(error, body, response)` has been changed to `(error, response)`. The body is available under the `result` property of the response. The `data` property has been removed in favor of `result`.
diff --git a/test/unit/requestWrapper.test.js b/test/unit/requestWrapper.test.js
@@ -15,38 +15,43 @@ mockAxiosInstance.interceptors = {
 axios.default.create.mockReturnValue(mockAxiosInstance);
 
 const { RequestWrapper } = require('../../lib/requestwrapper');
-let requestWrapperInstance;
+const requestWrapperInstance = new RequestWrapper();
 
 describe('axios', () => {
   it('should enable debug', () => {
-    requestWrapperInstance = new RequestWrapper();
+    // these should have been called when requestWrapperInstance was instantiated
     expect(mockAxiosInstance.interceptors.request.use).toHaveBeenCalledTimes(1);
     expect(mockAxiosInstance.interceptors.response.use).toHaveBeenCalledTimes(1);
   });
 });
 
 describe('sendRequest', () => {
+  let axiosResolveValue;
+  let expectedResult;
+
+  beforeEach(() => {
+    // these objects get messed with, so reset them before each test
+    axiosResolveValue = {
+      data: 'test',
+      config: 'test',
+      request: 'test',
+      statusText: 'test',
+      status: 200,
+      headers: 'test',
+    };
+
+    expectedResult = {
+      result: 'test',
+      statusText: 'test',
+      status: 200,
+      headers: 'test',
+    };
+  });
+
   afterEach(() => {
     mockAxiosInstance.mockReset();
   });
 
-  const axiosResolveValue = {
-    data: 'test',
-    config: 'test',
-    request: 'test',
-    statusText: 'test',
-    status: 200,
-    headers: 'test',
-  };
-
-  const expectedResult = {
-    data: 'test',
-    result: 'test',
-    statusText: 'test',
-    status: 200,
-    headers: 'test',
-  };
-
   it('should send a request with default parameters', done => {
     const parameters = {
       defaultOptions: {
@@ -65,7 +70,7 @@ describe('sendRequest', () => {
 
     mockAxiosInstance.mockResolvedValue(axiosResolveValue);
 
-    requestWrapperInstance.sendRequest(parameters, (err, body, res) => {
+    requestWrapperInstance.sendRequest(parameters, (err, res) => {
       // assert results
       expect(mockAxiosInstance.mock.calls[0][0].data).toEqual('post=body');
       expect(mockAxiosInstance.mock.calls[0][0].url).toEqual(
@@ -103,10 +108,9 @@ describe('sendRequest', () => {
 
     mockAxiosInstance.mockRejectedValue('error');
 
-    requestWrapperInstance.sendRequest(parameters, (err, body, res) => {
+    requestWrapperInstance.sendRequest(parameters, (err, res) => {
       // assert results
       expect(err).toEqual(expect.anything());
-      expect(body).toBeUndefined();
       expect(res).toBeUndefined();
       done();
     });
@@ -150,7 +154,7 @@ describe('sendRequest', () => {
       return Promise.resolve(axiosResolveValue);
     });
 
-    requestWrapperInstance.sendRequest(parameters, (err, body, res) => {
+    requestWrapperInstance.sendRequest(parameters, (err, res) => {
       // assert results
       expect(serializedParams).toBe('version=2018-10-15&array_style=a%2Cb');
       expect(mockAxiosInstance.mock.calls[0][0].url).toEqual(
@@ -220,7 +224,7 @@ describe('sendRequest', () => {
       return Promise.resolve(axiosResolveValue);
     });
 
-    requestWrapperInstance.sendRequest(parameters, (err, body, res) => {
+    requestWrapperInstance.sendRequest(parameters, (err, res) => {
       // assert results
       expect(mockAxiosInstance.mock.calls[0][0].url).toEqual(
         'https://example.ibm.com/v1/environments/environment-id/configurations/configuration-id'
@@ -301,7 +305,7 @@ describe('sendRequest', () => {
       return Promise.resolve(axiosResolveValue);
     });
 
-    requestWrapperInstance.sendRequest(parameters, (err, body, res) => {
+    requestWrapperInstance.sendRequest(parameters, (err, res) => {
       // assert results
       expect(mockAxiosInstance.mock.calls[0][0].data).toEqual('a=a&b=b');
       expect(mockAxiosInstance.mock.calls[0][0].url).toEqual(
@@ -346,7 +350,7 @@ describe('sendRequest', () => {
   //
   //     mockAxiosInstance.mockResolvedValue('res');
   //
-  //     requestWrapperInstance.sendRequest(parameters, (err, body, res) => {
+  //     requestWrapperInstance.sendRequest(parameters, (err, res) => {
   //       // assert results
   //       expect(mockAxiosInstance.mock.calls[0][0].otherParam).toEqual(500);
   //       expect(res).toEqual(expectedResult);