-
Notifications
You must be signed in to change notification settings - Fork 1.1k
ensuring that port is number and host is string | fixes the #1600 #1601
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ensuring that port is number and host is string | fixes the #1600 #1601
Conversation
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
The following contributors of this PR have not signed the OCA:
- PR author: @danielsrod
- coeng.daniel@gmail.com (@danielsrod)
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.
When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.
If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public.
@danielsrod thanks for this. Can you sign the OCA? If not, since this is a small change, we can treat it as a bug report.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better if the source of address.port and address.host were adjusted as needed.
I already sign the OCA
just waiting the aprove
I already sign the OCA just waiting the aprove
Excellent! Thank you If you don't hear back, let me know and I'll chase up internally.
Thank you for signing the OCA.
Hi @cjbj
Any news for this PR ?
@danielsrod, I am able to reproduce this in-house. This seems to happen when we do a BUN run of the sample Typescript code. We will get back to you on this soon.
Hi @cjbj Any news for this PR ?
Can you look at the review comment from @anthony-tuininga ?
bun 1.0 just dropped
really need this merge 😅
bun 1.0 just dropped really need this merge 😅
Dev has been reviewing and testing in our internal repo. The final fix will be a little earlier in the code path than you propose, in line with the review request.
@danielsrod This PR is now available as a patch in this GitHub repo. Please take the latest version of the file /lib/thin/sqlnet/navNodes.js
index 3c5b266b..134fd059 100644 --- a/lib/thin/sqlnet/navNodes.js +++ b/lib/thin/sqlnet/navNodes.js @@ -77,7 +77,7 @@ class Address { const httpsProxyPortNVP = findNVPairRecurse(nvp, 'https_proxy_port'); if (portnvp) - this.port = portnvp.atom; + this.port = Number(portnvp.atom); if (hostnvp) this.host = hostnvp.atom; @@ -88,7 +88,7 @@ class Address { if (httpsProxyNVP) this.httpsProxy = httpsProxyNVP.atom; if (httpsProxyPortNVP) - this.httpsProxyPort = httpsProxyPortNVP.atom; + this.httpsProxyPort = Number(httpsProxyPortNVP.atom); this.addr = nvp.toString(); }
It will fixed in the node-oracledb 6.2 release. Thank you for your contribution!
I will close this PR once 6.2 is out.
@sharadraju thank you
Closing this PR as this issue has been fixed in node-oracledb 6.2.
Thank you for your contribution!
this pull request fixes the #1600