holepunchto / hypercore

Hypercore is a secure, distributed append-only log.
https://docs.holepunch.to
MIT License
2.56k stars 183 forks source link

'TypeError: this.push is not a function' when setting batch > 1 in feed.createReadStream() #269

Closed jwerle closed 4 years ago

jwerle commented 4 years ago

If you set opts.batch to something greater than 1, this error is triggered. The cause is an unbound this: https://github.com/hypercore-protocol/hypercore/blob/master/index.js#L1361

A possible fix:

diff --git a/index.js b/index.js
index f1071f2..2132634 100644
--- a/index.js
+++ b/index.js
@@ -1314,7 +1314,8 @@ Feed.prototype.createReadStream = function (opts) {
   var first = true
   var range = this.download({ start: start, end: end, linear: true })

-  return from.obj(read).on('end', cleanup).on('close', cleanup)
+  var stream = from.obj(read).on('end', cleanup).on('close', cleanup)
+  return stream

   function read (size, cb) {
     if (!self.opened) return open(size, cb)
@@ -1358,7 +1359,7 @@ Feed.prototype.createReadStream = function (opts) {

       var lastIdx = result.length - 1
       for (var i = 0; i < lastIdx; i++) {
-        this.push(result[i])
+        stream.push(result[i])
       }
       cb(null, result[lastIdx])
     })
tinchoz49 commented 4 years ago

Hey @jwerle, how are you?

Can you show me the use case where is not working, I try the test and is passing and doing something like below also works:

const hypercore = require('hypercore')
const ram = require('random-access-memory')

const feed = hypercore(ram)

feed.append('foo', () => {
  feed.append('bar', () => {
    const stream = feed.createReadStream({ batch: 100 })
    stream.on('data', (data) => {
      console.log(data)
    })
  })
})

Just to know what is the case where the arrow function is not respecting the lexical scoping and maybe add it to the test too.

jwerle commented 4 years ago

Hey @tinchoz49 !

I didn't mention it, because I didn't realize it, but this was running inside of a worker launched by piscina

tinchoz49 commented 4 years ago

Ah ok, I don't how piscina it's losing the bound in that case (maybe a transpiler issue?) haha but anyway I prefer the fix on your PR than playing with arrows and this.

Thanks!

jwerle commented 4 years ago

yeah I didn't dig into it but no problem! :palm_tree:

mafintosh commented 4 years ago

Sounds like a transpiler problem to me. Down to merge the fix tho if that helps you

jwerle commented 4 years ago

@mafintosh thank you!