Skip to content

Commit

Permalink
fix: skip config validation when using connector
Browse files Browse the repository at this point in the history 
This changeset fixes a problem in the original custom `connector`
factory method implementation that required the `server` config property
to be defined even though its value is ignored.

Removing the validation when `config.options.connector` is defined fixes
the problem.

Ref: #1540
Fixes: #1541
Signed-off-by: Ruy Adorno <ruyadorno@google.com>
  • Loading branch information
@ruyadorno
ruyadorno committed Jun 1, 2023
1 parent e4eadf8 commit 8e92a0b
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 10 deletions.
41 changes: 33 additions & 8 deletions src/connection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1052,7 +1052,7 @@ class Connection extends EventEmitter {
throw new TypeError('The "config" argument is required and must be of type Object.');
}

if (typeof config.server !== 'string') {
if (typeof config.server !== 'string' && !config.options!.connector) {
throw new TypeError('The "config.server" property is required and must be of type string.');
}

Expand Down Expand Up @@ -1343,8 +1343,15 @@ class Connection extends EventEmitter {
if (typeof config.options.connector !== 'function') {
throw new TypeError('The "config.options.connector" property must be a function.');
}
if (config.server) {
throw new Error('Server and connector are mutually exclusive, but ' + config.server + ' and a connector function were provided');
}
if (config.options.port) {
throw new Error('Port and connector are mutually exclusive, but ' + config.options.port + ' and a connector function were provided');
}

this.config.options.connector = config.options.connector;
this.config.options.port = undefined;
}

if (config.options.cryptoCredentialsDetails !== undefined) {
Expand Down Expand Up @@ -1513,6 +1520,9 @@ class Connection extends EventEmitter {
this.config.options.fallbackToDefaultDb = config.options.fallbackToDefaultDb;
}

if (config.options.connector !== undefined) {
}

if (config.options.instanceName !== undefined) {
if (typeof config.options.instanceName !== 'string') {
throw new TypeError('The "config.options.instanceName" property must be of type string.');
Expand Down Expand Up @@ -1900,7 +1910,10 @@ class Connection extends EventEmitter {
initialiseConnection() {
const signal = this.createConnectTimer();

if (this.config.options.port) {
if (this.config.options.connector) {
// port and multiSubnetFailover are not used when using a custom connector
return this.connectOnPort(0, false, signal, this.config.options.connector);
} else if (this.config.options.port) {
return this.connectOnPort(this.config.options.port, this.config.options.multiSubnetFailover, signal, this.config.options.connector);
} else {
return instanceLookup({
Expand Down Expand Up @@ -1995,7 +2008,11 @@ class Connection extends EventEmitter {
this.socket = socket;

this.closed = false;
this.debug.log('connected to ' + this.config.server + ':' + this.config.options.port);

const connectedMsg = customConnector ?
'connected via custom connector' :
'connected to ' + this.config.server + ':' + this.config.options.port;
this.debug.log(connectedMsg);

this.sendPreLogin();
this.transitionTo(this.STATE.SENT_PRELOGIN);
Expand Down Expand Up @@ -2073,7 +2090,8 @@ class Connection extends EventEmitter {
*/
connectTimeout() {
const message = `Failed to connect to ${this.config.server}${this.config.options.port ? `:${this.config.options.port}` : `\\${this.config.options.instanceName}`} in ${this.config.options.connectTimeout}ms`;
this.debug.log(message);
const customConnectorMessage = `Failed to connect using custom connector in ${this.config.options.connectTimeout}ms`;
this.debug.log(this.config.options.connector ? customConnectorMessage : message);
this.emit('connect', new ConnectionError(message, 'ETIMEOUT'));
this.connectTimer = undefined;
this.dispatchEvent('connectTimeout');
Expand Down Expand Up @@ -2202,7 +2220,8 @@ class Connection extends EventEmitter {
socketError(error: Error) {
if (this.state === this.STATE.CONNECTING || this.state === this.STATE.SENT_TLSSSLNEGOTIATION) {
const message = `Failed to connect to ${this.config.server}:${this.config.options.port} - ${error.message}`;
this.debug.log(message);
const customConnectorMessage = `Failed to connect using custom connector - ${error.message}`;
this.debug.log(this.config.options.connector ? customConnectorMessage : message);
this.emit('connect', new ConnectionError(message, 'ESOCKET'));
} else {
const message = `Connection lost - ${error.message}`;
Expand All @@ -2228,15 +2247,21 @@ class Connection extends EventEmitter {
* @private
*/
socketClose() {
this.debug.log('connection to ' + this.config.server + ':' + this.config.options.port + ' closed');
const message = 'connection to ' + this.config.server + ':' + this.config.options.port + ' closed';
const customConnectorMessage = 'connection closed';
this.debug.log(this.config.options.connector ? customConnectorMessage : message);
if (this.state === this.STATE.REROUTING) {
this.debug.log('Rerouting to ' + this.routingData!.server + ':' + this.routingData!.port);
const message = 'Rerouting to ' + this.routingData!.server + ':' + this.routingData!.port;
const customConnectorMessage = 'Rerouting';
this.debug.log(this.config.options.connector ? customConnectorMessage : message);

this.dispatchEvent('reconnect');
} else if (this.state === this.STATE.TRANSIENT_FAILURE_RETRY) {
const server = this.routingData ? this.routingData.server : this.config.server;
const port = this.routingData ? this.routingData.port : this.config.options.port;
this.debug.log('Retry after transient failure connecting to ' + server + ':' + port);
const message = 'Retry after transient failure connecting to ' + server + ':' + port;
const customConnectorMessage = 'Retry after transient failure connecting';
this.debug.log(this.config.options.connector ? customConnectorMessage : message);

this.dispatchEvent('retry');
} else {
Expand Down
26 changes: 24 additions & 2 deletions test/unit/custom-connector.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ describe('custom connector', function() {
const host = server.address().address;
const port = server.address().port;
const connection = new Connection({
server: host,
options: {
connector: async () => {
customConnectorCalled = true;
Expand All @@ -37,7 +36,6 @@ describe('custom connector', function() {
port,
});
},
port
},
});

Expand All @@ -59,4 +57,28 @@ describe('custom connector', function() {

connection.connect();
});

it('should not define both server and connector options', function(done) {
assert.throws(() => {
new Connection({
server: '0.0.0.0',
options: {
connector: async () => {},
},
});
}, Error, 'Server and connector are mutually exclusive, but 0.0.0.0 and a connector function were provided');
done();
});

it('should not define both port and connector options', function(done) {
assert.throws(() => {
new Connection({
options: {
connector: async () => {},
port: 8080,
},
});
}, Error, 'Port and connector are mutually exclusive, but 8080 and a connector function were provided');
done();
});
});

0 comments on commit 8e92a0b

Please sign in to comment.