Does node-mssql support multiSubnetFailover?

[ibm-mai]

I have used typeorm along with node-mssql for client driver. However, passing extra.multiSubnetFailover does not inherit this property to tedious driver.

Expected behaviour:

Passing multiSubnetFailover to extra should be send to tedious driver

export const AppDataSource = new DataSource({
    type: "mssql",
    host: "localhost",
    username: "sa",
    password: "***",
    database: "test",
    synchronize: false,
    logging: false,
    entities: [User],
    migrations: [],
    subscribers: [],
    options: {
        trustServerCertificate: true
    },
    extra: {
        multiSubnetFailover: true
    }
})

Actual behaviour:

extra.multiSubnetFailover not being sent to tedious driver. The current code does not support adding multiSubnetFailover to options that can pass to tedious driver https://github.com/tediousjs/node-mssql/blob/d4a976c91c1d67b0f8dbc29a88b15a7b19bfb77b/lib/tedious/connection-pool.js#L10

Configuration:

extra: {
    multiSubnetFailover: true
}

Are you willing to resolve this issue by submitting a Pull Request?

Yes, I have the time, and I know how to start.

Software versions

• NodeJS: 18
• node-mssql: 10.0.2
• SQL Server: 2019

[dhensby]

I'm not sure what the extra prop is meant to be, we don't support that as part of our config object at all.

The way to pass options directly to tedious is using the options prop (as you are for trustServerCertificate).

I don't know about tedious' underlying support for the option (I don't see it in the link you provided), which is going to be what decides if it is supported or not.

---

edit: I was only my phone when initially reviewing this and didn't realise you were linking to the node-mssql connection code and not tedious.

Tedious does accept the option: https://github.com/tediousjs/tedious/blob/7e84a2f54c6cb983b18a75ad1fc4bf274b2398e5/src/connection.ts#L377 - so it just needs to be set appropriately:

export const AppDataSource = new DataSource({
    type: "mssql",
    host: "localhost",
    username: "sa",
    password: "***",
    database: "test",
    synchronize: false,
    logging: false,
    entities: [User],
    migrations: [],
    subscribers: [],
    options: {
+        multiSubnetFailover: true,
        trustServerCertificate: true
    },
-    extra: {
-        multiSubnetFailover: true
-    }
})

[isaru66]

@dhensby , I worked with @ibm-mai . Currently the issue is that node-mssql did not pass the multiSubnetFailover to tediosjs.

I have open PR https://github.com/tediousjs/node-mssql/pull/1626 to resolve this issue.

[isaru66]

from TypeORM, option prop is type check with https://github.com/typeorm/typeorm/blob/83567f533482d0170c35e2f99e627962b8f99a08/src/driver/sqlserver/SqlServerConnectionOptions.ts , and currently SqlServerConnectionOptions.ts does not have multiSubnetFailover, thus we cannot add multiSubnetFailover in TypeORM option yet, since it will violate type-checking.

extra prop is custom properties with any type, thus it allow us to pass any property that may not yet defined in SqlServerConnectionOptions.ts, we can see from the below source code of TypeORM that extra prob will be merge with options prop to construct options for NodeMSSQL. https://github.com/typeorm/typeorm/blob/83567f533482d0170c35e2f99e627962b8f99a08/src/driver/sqlserver/SqlServerDriver.ts#L1100

[dhensby]

Can you elaborate on the extra prop is merged with the base object and not the options sub object, which is where it is needed.

You can coerce types in typescript so you can add arbitrary props. Eg: value as unknown as Interface

[isaru66]

@dhensby

to elaborate that options and extra prop get merged in TypeORM before send to Node-MSSQL.

let say in TypeORM we construct datasource with both trustServerCertificate and multiSubnetFailover in extra section only

export const AppDataSource = new DataSource({
    type: "mssql",
    host: "127.0.0.1",
    username: "sa",
    password: "Passwd@1234",
    database: "tempdb",
    synchronize: true,
    logging: false,
    entities: [User],
    migrations: [],
    subscribers: [],
    extra: {
        trustServerCertificate: true,  
        multiSubnetFailover: true,
    }
})

Once I ran this code and set break point at TypeORM SqlServerDriver.js , function createPool().

We can see that the connectionOptions is the merged value between options and extra prop this is the value that TypeORM send to node-mssql.

so... using extra option in TypeORM is able to pass multiSubnetFailover option to node-mssql.

[isaru66]

the current issue is that mssql.ConnectionPool(connectionOptions) in node-mssql did not passed down multiSubnetFailover option to tediusjs driver currently.

That why i create the PR to try to fix this issue.

[dhensby]

This library does not support the property multiSubnetFailover in the config object. If you want to provide that value to tedious it must be done in the options sub-object in the config.

There is no problem here and I'm quite confused as to why my solution is not satisfactory.

If TypeORM has issues with the typing for the DB config object, then those typings need fixing instead of expanding the API surface of this library.

export const AppDataSource = new DataSource({
    type: "mssql",
    host: "localhost",
    username: "sa",
    password: "***",
    database: "test",
    synchronize: false,
    logging: false,
    entities: [User],
    migrations: [],
    subscribers: [],
    options: {
        multiSubnetFailover: true,
        trustServerCertificate: true
    },
} as unknown as SqlServerConnectionOptions);

Better yet, open a PR there to fix SqlServerConnectionOptions

[dhensby]

Also, just to add (for sake of clarity) your example of adding trustServerCertificate to extra will also not work because it must be in the the options sub-object not the root level. Your solution in the linked PR is not scalable or reasonable as a solution as the natural conclusion is that every property that is added in the root object be duplicated in the options just to allow TypeORM to add these values in the extra object...

Fix the upstream library, don't add one-off "fixes" to this library to support the narrow implementation of TypeORM.

[isaru66]

@dhensby thank for your suggestion.

I have change the code in TypeORM, once multSubnetFailover option get added to SqlServerConnectionOptions. After the change, the multSubnetFailover option will be passed down from TypeORM => NodeMSSQL => TediosJS correctly.

I have create a pull request in TypeORM as https://github.com/typeorm/typeorm/pull/10804