Skip to content
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

QueryStream broken in Node 4 and 6 (2.1) #847

Closed
wubzz opened this issue Feb 15, 2018 · 5 comments
Closed

QueryStream broken in Node 4 and 6 (2.1) #847

wubzz opened this issue Feb 15, 2018 · 5 comments
Labels

Comments

@wubzz
Copy link

wubzz commented Feb 15, 2018

  1. What is your Node.js version? Is it 64-bit or 32-bit? Run version.js from
    64-bit v4.8.7 & v6.13.0

  2. What is your node-oracledb version?
    Latest

  3. What error(s) you are seeing?

Uncaught TypeError: self.destroy is not a function

After this commit our CI tests over at knex started breaking with the above error message. After some digging I realized that .destroy was added to ReadableStream in node 8+, which means the current version of the lib does not work for Node < 8.

The README states

Use node-oracledb to connect Node.js 4, 6, 8 and 9 to Oracle Database.

@wubzz wubzz changed the title QueryStream broken in Node 4 and 6 QueryStream broken in Node 4 and 6 (2.1) Feb 15, 2018
@cjbj
Copy link
Member

cjbj commented Feb 15, 2018

Hmm.

@cjbj
Copy link
Member

cjbj commented Feb 15, 2018

@dmcghan and @anthony-tuininga have a fix:

diff --git a/lib/querystream.js b/lib/querystream.js
index f1a13a4b..bd412e9a 100644
--- a/lib/querystream.js
+++ b/lib/querystream.js
@@ -69,7 +69,7 @@ function QueryStream(resultSet, oracledb) {
     // Using setImmediate to ensure that end event handlers are processed
     // before the destroy logic is invoked.
     setImmediate(function() {
-      self.destroy();
+      self._destroy(null, function() {});
     });
   });
 }

When the world (i.e. the Windows people in the team) wake up and we can get Windows binaries built, we'll push a v2.1.1.

After Spring holiday is over and everyone is back at work, we'll have a postmortem to work out how this got missed!

@cjbj
Copy link
Member

cjbj commented Feb 16, 2018

I've released 2.1.1. Give it a whirl!

@elhigu
Copy link

elhigu commented Feb 16, 2018

It works! :) 🎉

@cjbj
Copy link
Member

cjbj commented Feb 16, 2018

@elhigu yay!

@cjbj cjbj closed this as completed Feb 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants