chore(build): enable mssql on travis

[vlapo]

Lets try travis with mssql. I want to see travis build and try rerun few times if it is stable.

[amaranth]

It looks like this passed but something went weird with the test run on node 10 and it timed out.

[vlapo]

Yeah, there is issue with mssql driver. Issue is reported and we are waiting to resolve. Will see in few days :-)

[pleerock]

@vlapo can you please provide more info - where issue is reported? btw, today I added CircleCI config, so we can also try to setup it as well.

[vlapo]

I think there is problem with unclosed connections in pool. Issue in mssql driver tediousjs/node-mssql#457 and in node-pool library coopernurse/node-pool#159.

The latest mocha do not exit properly if there are unclosed connections or resources. There is simple workaround - adding --exit flag for now but I hope issue will be resolved soon.

Note: Just do not know why is this issue only with node v10.

[dhensby]

1. You need to close your connection pools
2. You can use the --exit flag if you want the old mocha behaviour of terminating processes once all the test callbacks are complete.

[vlapo]

1. Yeah I see problem in typeorm code. We do not wait until connection pool is closed in SqlDriver. But I fixed this locally and have same issue with mocha.
2. Sure we know about this possibility but its just a workaround not solution of problem :)

I still thinks that problem is in node-pool library. Lets wait to fix and see if it will be help.

[dhensby]

@vlapo are you able to track down what connections are left open that are preventing the process from exiting?

I found for my test suites that maintaining a good record of connections and closing them was the solution, I didn't find any underlying issue with node-pool

[vlapo]

Have a logs provided in node-mssql issue:

pool(1): connection #3 created
connection(3): establishing
pool(1): connection #4 created
connection(4): establishing
connection(2): borrowed to request #115
request(115): query SELECT ...
connection(2): released
request(115): completed
connection(2): borrowed to request #116
request(116): query SELECT ...
connection(2): released
request(116): completed
connection(2): borrowed to request #117
request(117): query SELECT ...
connection(2): released
request(117): completed
    ✓ should be able to retrieve the whole tree (313ms)
====> before close
====> after close
connection(3): established
connection(4): established
connection(2): destroying
connection(2): destroyed
connection(3): destroying
connection(3): destroyed

As you can see there is established connection no. 4 and never destroyed. But this behaviour is not occur every time. In our large test suites it is almost every time but in different tests.

I provided failing test case in node-pool and it seem like race condition. Maybe I am totally wrong. Will see.

[dhensby]

@vlapo interesting - yeah, we work with a medium sized test suite and aren't seeing that problem itself, but I can see that if the pool has created a connection that hasn't established by the time pool.close() is called, it seems to fail to abort or clean that connection up... Which is then lost to the ether :/

[vlapo]

@dhensby We have a some progress here. coopernurse/node-pool#159 (comment) I tested it on my local environment by replacing generic-pool sources in mssql package and it works like a sharm. No hanging DB connections anymore.

[dhensby]

Let me update the deps on mssql, then :)

[vlapo]

We are all green 💯 🔥 🎉 @dhensby, @sandfox thank you for your effort 🍺

[pleerock]

Thank you guys!

@vlapo do we use any api of new mssql that doesn't exist in 4.x? (Is it a breaking change for exist mssql users with 4.x installed?)

[dhensby]

I can release a 4.x update too, if you like - the change log for 5 is in the mssql repo - there is 1 config change that is different (forced on us by upgrading tedious version) but otherwise tried to keep everything BC

[dhensby]

4.3.1 tagged too :)

[vlapo]

ok, lets use stable 4.3.1 version instead 5.0.0-beta :-)