Skip to content

Commit

Permalink
feat: return aggregate errors
Browse files Browse the repository at this point in the history
When multiple errors happen during request execution, all errors will be collected and returned as a single `AggregatError` object.

When errors happen when establishing a connection in third party libraries like `@azure/identity`, we will also wrap them with extended error information into an AggregateError.

Co-authored-by: Michael Sun <47126816+MichaelSun90@users.noreply.github.com>
Co-authored-by: mShan0 <96149598+mShan0@users.noreply.github.com>
  • Loading branch information
3 people authored Apr 27, 2022
1 parent 4a1f49c commit d8575b2
Show file tree
Hide file tree
Showing 5 changed files with 179 additions and 9 deletions.
94 changes: 92 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
19 changes: 14 additions & 5 deletions src/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -938,7 +939,7 @@ class Connection extends EventEmitter {
/**
* @private
*/
loginError: undefined | ConnectionError;
loginError: undefined | AggregateError | ConnectionError;
/**
* @private
*/
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand All @@ -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 {
Expand Down
9 changes: 8 additions & 1 deletion src/token/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 + '`');
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
}
}

Expand Down
64 changes: 63 additions & 1 deletion test/integration/errors-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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();
});
});
});

0 comments on commit d8575b2

Please sign in to comment.