Merge pull request #1409 from MichaelSun90/BetterErrorHandling

feat: better error handling and test fixes
tediousjs · Apr 27, 2022 · 6dea6c3 · 6dea6c3
2 parents 4a1f49c + d8575b2
commit 6dea6c3
Show file tree

Hide file tree

Showing 5 changed files with 179 additions and 9 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -43,7 +43,9 @@
     "@azure/identity": "^2.0.1",
     "@azure/keyvault-keys": "^4.3.0",
     "@js-joda/core": "^4.0.0",
+    "@types/es-aggregate-error": "^1.0.2",
     "bl": "^5.0.0",
+    "es-aggregate-error": "^1.0.7",
     "iconv-lite": "^0.6.3",
     "jsbi": "^3.2.1",
     "native-duplexpair": "^1.0.0",

diff --git a/src/connection.ts b/src/connection.ts
@@ -44,6 +44,7 @@ import { Parameter, TYPES } from './data-type';
 import { BulkLoadPayload } from './bulk-load-payload';
 import { Collation } from './collation';
 
+import AggregateError from 'es-aggregate-error';
 import { version } from '../package.json';
 import { URL } from 'url';
 import { AttentionTokenHandler, InitialSqlTokenHandler, Login7TokenHandler, RequestTokenHandler, TokenHandler } from './token/handler';
@@ -938,7 +939,7 @@ class Connection extends EventEmitter {
   /**
    * @private
    */
-  loginError: undefined | ConnectionError;
+  loginError: undefined | AggregateError | ConnectionError;
   /**
    * @private
    */
@@ -3163,6 +3164,13 @@ function emitAzureADPasswordClientIdDeprecationWarning() {
   );
 }
 
+function isTransientError(error: AggregateError | ConnectionError): boolean {
+  if (error instanceof AggregateError) {
+    error = error.errors[0];
+  }
+  return (error instanceof ConnectionError) && !!error.isTransient;
+}
+
 export default Connection;
 module.exports = Connection;
 
@@ -3352,7 +3360,7 @@ Connection.prototype.STATE = {
               this.transitionTo(this.STATE.LOGGED_IN_SENDING_INITIAL_SQL);
             }
           } else if (this.loginError) {
-            if (this.loginError.isTransient) {
+            if (isTransientError(this.loginError)) {
               this.debug.log('Initiating retry on transient error');
               this.transitionTo(this.STATE.TRANSIENT_FAILURE_RETRY);
             } else {
@@ -3418,7 +3426,7 @@ Connection.prototype.STATE = {
               this.socketError(err);
             });
           } else if (this.loginError) {
-            if (this.loginError.isTransient) {
+            if (isTransientError(this.loginError)) {
               this.debug.log('Initiating retry on transient error');
               this.transitionTo(this.STATE.TRANSIENT_FAILURE_RETRY);
             } else {
@@ -3510,7 +3518,8 @@ Connection.prototype.STATE = {
 
             getToken((err, token) => {
               if (err) {
-                this.loginError = new ConnectionError('Security token could not be authenticated or authorized.', 'EFEDAUTH');
+                this.loginError = new AggregateError(
+                  [new ConnectionError('Security token could not be authenticated or authorized.', 'EFEDAUTH'), err]);
                 this.emit('connect', this.loginError);
                 this.transitionTo(this.STATE.FINAL);
                 return;
@@ -3519,7 +3528,7 @@ Connection.prototype.STATE = {
               this.sendFedAuthTokenMessage(token!);
             });
           } else if (this.loginError) {
-            if (this.loginError.isTransient) {
+            if (isTransientError(this.loginError)) {
               this.debug.log('Initiating retry on transient error');
               this.transitionTo(this.STATE.TRANSIENT_FAILURE_RETRY);
             } else {

diff --git a/src/token/handler.ts b/src/token/handler.ts
@@ -33,6 +33,8 @@ import {
 } from './token';
 import BulkLoad from '../bulk-load';
 
+import AggregateError from 'es-aggregate-error';
+
 export class UnexpectedTokenError extends Error {
   constructor(handler: TokenHandler, token: Token) {
     super('Unexpected token `' + token.name + '` in `' + handler.constructor.name + '`');
@@ -368,12 +370,14 @@ export class Login7TokenHandler extends TokenHandler {
 export class RequestTokenHandler extends TokenHandler {
   connection: Connection;
   request: Request | BulkLoad;
+  errors: RequestError[];
 
   constructor(connection: Connection, request: Request | BulkLoad) {
     super();
 
     this.connection = connection;
     this.request = request;
+    this.errors = [];
   }
 
   onInfoMessage(token: InfoMessageToken) {
@@ -392,8 +396,11 @@ export class RequestTokenHandler extends TokenHandler {
       error.serverName = token.serverName;
       error.procName = token.procName;
       error.lineNumber = token.lineNumber;
-
+      this.errors.push(error);
       this.request.error = error;
+      if (this.request instanceof Request && this.errors.length > 1) {
+        this.request.error = new AggregateError(this.errors);
+      }
     }
   }
 

diff --git a/test/integration/errors-test.js b/test/integration/errors-test.js
@@ -4,8 +4,9 @@ const fs = require('fs');
 const assert = require('chai').assert;
 const debug = false;
 
-import Connection from '../../src/connection';
+import AggregateError from 'es-aggregate-error';
 import { RequestError } from '../../src/errors';
+import Connection from '../../src/connection';
 import Request from '../../src/request';
 
 const config = JSON.parse(
@@ -182,4 +183,65 @@ describe('Errors Test', function() {
       done();
     });
   });
+
+  it('should throw aggregate error with two error messages', function(done) {
+    const connection = new Connection(config);
+
+    connection.connect((err) => {
+      if (err) {
+        return done(err);
+      }
+
+      const request = new Request('create type test_type as table ( id int, primary key (code) );', (error) => {
+        assert.instanceOf(error, AggregateError);
+
+        if (error instanceof AggregateError) {
+          assert.strictEqual(error.errors.length, 2);
+          assert.strictEqual(error.errors[0].message, 'Column name \'code\' does not exist in the target table or view.');
+          assert.strictEqual(error.errors[1].message, 'Could not create constraint or index. See previous errors.');
+        }
+
+        connection.close();
+      });
+
+      connection.execSql(request);
+    });
+
+    connection.on('end', function() {
+      done();
+    });
+  });
+
+  it('should throw aggregate error with AAD token retrieve', function(done) {
+    config.server = 'help.kusto.windows.net';
+    config.authentication = {
+      type: 'azure-active-directory-password',
+      options: {
+        userName: 'username',
+        password: 'password',
+        // Lack of tenantId will generate a AAD token retrieve error
+        clientId: 'clientID',
+      }
+    };
+    config.options.tdsVersion = '7_4';
+    const connection = new Connection(config);
+
+    /** @type {Error | undefined} */
+    let connectionError;
+    connection.connect((err) => {
+      connectionError = err;
+    });
+
+    connection.on('end', function() {
+      assert.instanceOf(connectionError, AggregateError);
+
+      if (connectionError instanceof AggregateError) {
+        assert.strictEqual(connectionError.errors.length, 2);
+        assert.strictEqual(connectionError.errors[0].message, 'Security token could not be authenticated or authorized.');
+        assert.include(connectionError.errors[1].message, 'The grant type is not supported over the /common or /consumers endpoints.');
+      }
+
+      done();
+    });
+  });
 });