fix: parse result from response in token managers

* move `catch` block before `then` block in request handler

Author: dpopp07

auth/token-managers/jwt-token-manager.ts

@@ -83,7 +83,7 @@ export class JwtTokenManager {
       this.requestToken((err, tokenResponse) => {
         if (!err) {
           try {
-            this.saveTokenInfo(tokenResponse);
+            this.saveTokenInfo(tokenResponse.result);
           } catch(e) {
             // send lower level error through callback for user to handle
             err = e;

lib/requestwrapper.ts

@@ -206,21 +206,27 @@ 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;
-      delete res.data;
-
-      _callback(null, res);
-    })
-    .catch(error => {
-      _callback(this.formatError(error));
-    });
+      // placing `catch` block first because it is for catching request errors
+      // if it is after the `then` block, it will also catch errors if they occur
+      // inside of the `then` block
+      .catch(error => {
+        _callback(this.formatError(error));
+      })
+      .then(res => {
+        // sometimes error responses will still trigger the `then` block - escape that behavior here
+        if (!res) { return };
+
+        // 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;
+        delete res.data;
+
+        _callback(null, res);
+      });
   }
 
   /**

test/unit/iam-token-manager.test.js

@@ -18,6 +18,17 @@ RequestWrapper.mockImplementation(() => {
   };
 });
 
+const ACCESS_TOKEN = '9012';
+const IAM_RESPONSE = {
+  result: {
+    access_token: ACCESS_TOKEN,
+    refresh_token: '3456',
+    token_type: 'Bearer',
+    expires_in: 3600,
+    expiration: Math.floor(Date.now() / 1000) + 3600,
+  },
+};
+
 const CLIENT_ID_SECRET_WARNING =
   'Warning: Client ID and Secret must BOTH be given, or the header will not be included.';
 
@@ -37,21 +48,12 @@ describe('iam_token_manager_v1', function() {
   it('should turn an iam apikey into an access token', function(done) {
     const instance = new IamTokenManager({ apikey: 'abcd-1234' });
 
-    const accessToken = '9012';
-    const iamResponse = {
-      access_token: accessToken,
-      refresh_token: '3456',
-      token_type: 'Bearer',
-      expires_in: 3600,
-      expiration: Math.floor(Date.now() / 1000) + 3600,
-    };
-
     mockSendRequest.mockImplementation((parameters, _callback) => {
-      _callback(null, iamResponse);
+      _callback(null, IAM_RESPONSE);
     });
 
     instance.getToken(function(err, token) {
-      expect(token).toBe(accessToken);
+      expect(token).toBe(ACCESS_TOKEN);
       done();
     });
   });
@@ -70,21 +72,12 @@ describe('iam_token_manager_v1', function() {
 
     instance.tokenInfo = currentTokenInfo;
 
-    const accessToken = '9012';
-    const iamResponse = {
-      access_token: accessToken,
-      refresh_token: '3456',
-      token_type: 'Bearer',
-      expires_in: 3600,
-      expiration: Math.floor(Date.now() / 1000) + 3600,
-    };
-
     mockSendRequest.mockImplementation((parameters, _callback) => {
-      _callback(null, iamResponse);
+      _callback(null, IAM_RESPONSE);
     });
 
     instance.getToken(function(err, token) {
-      expect(token).toBe(accessToken);
+      expect(token).toBe(ACCESS_TOKEN);
       expect(requestMock).toHaveBeenCalled();
       done();
     });
@@ -94,9 +87,8 @@ describe('iam_token_manager_v1', function() {
     const instance = new IamTokenManager({ apikey: 'abcd-1234' });
     const requestMock = jest.spyOn(instance, 'requestToken');
 
-    const accessToken = '1234';
     const currentTokenInfo = {
-      access_token: accessToken,
+      access_token: ACCESS_TOKEN,
       refresh_token: '5678',
       token_type: 'Bearer',
       expires_in: 3600,
@@ -108,7 +100,7 @@ describe('iam_token_manager_v1', function() {
     instance.expireTime = currentTokenInfo.expiration;
 
     instance.getToken(function(err, token) {
-      expect(token).toBe(accessToken);
+      expect(token).toBe(ACCESS_TOKEN);
       expect(requestMock).not.toHaveBeenCalled();
       done();
     });
@@ -127,21 +119,12 @@ describe('iam_token_manager_v1', function() {
 
     instance.tokenInfo = currentTokenInfo;
 
-    const accessToken = '9012';
-    const iamResponse = {
-      access_token: accessToken,
-      refresh_token: '3456',
-      token_type: 'Bearer',
-      expires_in: 3600,
-      expiration: Math.floor(Date.now() / 1000) + 3600,
-    };
-
     mockSendRequest.mockImplementation((parameters, _callback) => {
-      _callback(null, iamResponse);
+      _callback(null, IAM_RESPONSE);
     });
 
     instance.getToken(function(err, token) {
-      expect(token).toBe(accessToken);
+      expect(token).toBe(ACCESS_TOKEN);
       expect(requestMock).toHaveBeenCalled();
       done();
     });
@@ -158,21 +141,12 @@ describe('iam_token_manager_v1', function() {
 
     instance.tokenInfo = currentTokenInfo;
 
-    const accessToken = '9012';
-    const iamResponse = {
-      access_token: accessToken,
-      refresh_token: '3456',
-      token_type: 'Bearer',
-      expires_in: 3600,
-      expiration: Math.floor(Date.now() / 1000) + 3600,
-    };
-
     mockSendRequest.mockImplementation((parameters, _callback) => {
-      _callback(null, iamResponse);
+      _callback(null, IAM_RESPONSE);
     });
 
     instance.getToken(function(err, token) {
-      expect(token).toBe(accessToken);
+      expect(token).toBe(ACCESS_TOKEN);
       done();
     });
   });

test/unit/jwt-token-manager.test.js

@@ -10,7 +10,7 @@ function getCurrentTime() {
 
 const ACCESS_TOKEN = 'abc123';
 
-describe('iam_token_manager_v1', () => {
+describe('JWT Token Manager', () => {
   it('should initialize base variables', () => {
     const url = 'service.com';
     const instance = new JwtTokenManager({ url });
@@ -37,7 +37,7 @@ describe('iam_token_manager_v1', () => {
 
       const requestTokenSpy = jest
         .spyOn(instance, 'requestToken')
-        .mockImplementation(cb => cb(null, { access_token: ACCESS_TOKEN }));
+        .mockImplementation(cb => cb(null, { result: { access_token: ACCESS_TOKEN } }));
 
       instance.getToken((err, res) => {
         expect(requestTokenSpy).toHaveBeenCalled();
@@ -64,7 +64,7 @@ describe('iam_token_manager_v1', () => {
 
       const requestTokenSpy = jest
         .spyOn(instance, 'requestToken')
-        .mockImplementation(cb => cb(null, { access_token: ACCESS_TOKEN }));
+        .mockImplementation(cb => cb(null, { result: { access_token: ACCESS_TOKEN } }));
 
       instance.getToken((err, res) => {
         expect(requestTokenSpy).toHaveBeenCalled();