Uploaded image for project: 'Node.js Driver'
  1. Node.js Driver
  2. NODE-426

GridStoreStream.write returns false

    • Type: Icon: Bug Bug
    • Resolution: Done
    • Priority: Icon: Major - P3 Major - P3
    • 2.0.28
    • Affects Version/s: 2.0.24, 2.0.25, 2.0.26, 2.0.27
    • Component/s: None
    • Labels:
    • Environment:
      node v0.12.2

      The last example in the Streams tutorial, "Streaming a File into GridFS" doesn't work.

      Stream piping doesn't work when writing to a GridStoreStream because GridStoreStream.prototype.write always returns false. As you can see from the node stream docs from writable.write:

      The return value indicates if you should continue writing right now. If the data had to be buffered internally, then it will return false. Otherwise, it will return true.

      This return value is strictly advisory. You MAY continue to write, even if it returns false. However, writes will be buffered in memory, so it is best not to do this excessively. Instead, wait for the drain event before writing more data.

      In my opinion, GridStoreStream.write should always return "true", but I understand that since the chunk is technically being buffered into memory "false" is actually the correct return value. That way the GridStoreStream instance passes the chunk along to self.gs.write that actually saves the chunk asynchronously. When it's done, GridStoreStream.write emits the 'drain' event which should properly signal stream.pipe to write another chunk but this is where another issue arrises that I can't get my head around: the drain event always gets triggered before GridStoreStream.write returns false. So now when stream.pipe gets a false response from GridStoreStream.write, it pauses the readstream and waits for the writestream to emit 'drain' - only the GridStoreStream has already emitted it's last 'drain' event already so stream.pipe just sits there waiting for something that will never happen.

      So here's my proposed solution:
      It seems like the internal function writeBuffer is async only when it has to close "self". This is actually a good thing b/c stream.pipe doesn't care what your writestream.write() returns if it has nothing else to write to it. So incorrectly returning "true" on the last write seems ok. That means that we can always return true from GridStoreStream.write except for when you've got to open the underlying "self.gs" which is async. So I think this function should return true if the underlying gridstore is open else return false and emit 'drain' when it finishes asynchronously opening the gridstore.

      A possible fallback is to leave the current implementation of always returning false from GridStoreStream.write but use process.nextTick before each self.emit('drain') to ensure GridStoreStream.write returns false first and drain is actually expecting our 'drain' event.

            Assignee:
            christkv Christian Amor Kvalheim
            Reporter:
            itsjustcon Connor Grady
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: