Does node-mssql support multiSubnetFailover? about node-mssql HOT 9 CLOSED

@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 typeorm/typeorm#10804

from node-mssql.

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
-    }
})

from node-mssql.

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

I have open PR #1626 to resolve this issue.

from node-mssql.

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

from node-mssql.

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

from node-mssql.

@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.

from node-mssql.

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.

from node-mssql.

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

from node-mssql.

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.

from node-mssql.