_acquire in case of reject return a promise, if this._acquire.promise is used in acquire, in case of reject an exception is raised that is not "captured" by the catch

[bombjackit]

What this does:

In case of reject acquire raise an exception that is controlled by catch : in previous version the raised exception was not captured from catch and if this node is used inside node-red-contrib-mssql-plus that run on node-red the result is an uncaught exception that stop it.

Related issues:

See this link https://github.com/bestlong/node-red-contrib-mssql-plus/issues/88#issuecomment-1840435259

Pre/Post merge checklist:

• [ ] Update change log

[dhensby]

I'm not sure I understand the problem.

At worst it seems the _acquire() function has an inconsistent return type (sometimes a promise, sometimes not), but it is an internal API, so that inconsistency shouldn't matter to any consuming libraries?

Do you have a test or some code to reproduce the problem?

[bombjackit]

It is a bit complex to explain, but I’ll try.

All start from here : this is an uncaught exception from node-red

[red] Uncaught Exception: [error] ConnectionError: Connection not yet open. at ConnectionPool._acquire (S:.node-red\node_modules\mssql\lib\base\connection-pool.js:380:36) at ConnectionPool.acquire (S:.node-red\node_modules\mssql\lib\base\connection-pool.js:366:56) at Immediate. (S:.node-red\node_modules\mssql\lib\tedious\request.js:419:19) at process.processImmediate (node:internal/timers:476:21)

If you can see all files in this stack trace is from this node (node-mssql), that is used by node-red-contrib-mssql-plus (a node-red node).

Your question maybe is “Why this uncaught exception ? node-red-contrib-mssql-plus must do try catch to avoid this”

See this piece of code

shared.Promise.resolve(this._acquire().promise).catch(err => { this.emit('error', err) throw err })

If _acquire fail it return a reject (that is a promise itself), then you call promise method of a promise object (that does not exist) this fire an uncatchable exception (this.emit is not called for example), because if you use promise you can catch exception only with method catch, not with try … catch.

Another point : it is more clear to return always one type from a method not one randomly selected that depends on what happens without managing each case.

I my environment I had applied the patch with success, when a reject happen a catchable exception was throw (with this.emit inside catch).

For the commit message, I don’t know how to change it, maybe I have to do another pull request ? I saw the web site that you indicated and I learned that I must start my commit message with fix :

[dhensby]

For the commit message, I don’t know how to change it, maybe I have to do another pull request ? I saw the web site that you indicated and I learned that I must start my commit message with fix :

$ git commit --amend
$ git push --force

git commit --amend will allow you to change the commit message for a commit, and the git push --force will update your branch and this pull request.

[bombjackit]

For the commit message, I don’t know how to change it, maybe I have to do another pull request ? I saw the web site that you indicated and I learned that I must start my commit message with fix :

$ git commit --amend
$ git push --force

git commit --amend will allow you to change the commit message for a commit, and the git push --force will update your branch and this pull request.

Thanks I modified commit message

[github-actions[bot]]

:tada: This PR is included in version 10.0.2 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: