From c59a3db4e8fb21e13b53a885fc469187f3e5c819 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Thu, 2 May 2019 02:04:59 +0200 Subject: [PATCH] [FSPersistorManager] fix the stream opening for node10+ Attaching a `readable` listener causes the stream to hang otherwise. Signed-off-by: Jakob Ackermann --- .../app/coffee/FSPersistorManager.coffee | 22 ++++++------- .../coffee/FSPersistorManagerTests.coffee | 33 +++++++++---------- 2 files changed, 25 insertions(+), 30 deletions(-) diff --git a/services/filestore/app/coffee/FSPersistorManager.coffee b/services/filestore/app/coffee/FSPersistorManager.coffee index 353e2ef099..7e2875fc28 100644 --- a/services/filestore/app/coffee/FSPersistorManager.coffee +++ b/services/filestore/app/coffee/FSPersistorManager.coffee @@ -41,20 +41,18 @@ module.exports = callback(err) # opts may be {start: Number, end: Number} - getFileStream: (location, name, opts, _callback = (err, res)->) -> - callback = _.once _callback + getFileStream: (location, name, opts, callback = (err, res)->) -> filteredName = filterName name logger.log location:location, name:filteredName, "getting file" - sourceStream = fs.createReadStream "#{location}/#{filteredName}", opts - sourceStream.on 'error', (err) -> - logger.err err:err, location:location, name:name, "Error reading from file" - if err.code == 'ENOENT' - return callback new Errors.NotFoundError(err.message), null - else - return callback err, null - sourceStream.on 'readable', () -> - # This can be called multiple times, but the callback wrapper - # ensures the callback is only called once + fs.open "#{location}/#{filteredName}", 'r', (err, fd) -> + if err? + logger.err err:err, location:location, name:name, "Error reading from file" + if err.code == 'ENOENT' + return callback new Errors.NotFoundError(err.message), null + else + return callback err, null + opts.fd = fd + sourceStream = fs.createReadStream null, opts return callback null, sourceStream diff --git a/services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee b/services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee index 7df7adfd8b..4cfde16f70 100644 --- a/services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee +++ b/services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee @@ -19,6 +19,7 @@ describe "FSPersistorManagerTests", -> rmdir:sinon.stub() exists:sinon.stub() readdir:sinon.stub() + open:sinon.stub() openSync:sinon.stub() fstatSync:sinon.stub() closeSync:sinon.stub() @@ -103,20 +104,21 @@ describe "FSPersistorManagerTests", -> @opts = {} it "should use correct file location", (done) -> - @Fs.createReadStream.returns({on: ->}) @FSPersistorManager.getFileStream @location, @name1, @opts, (err,res) => - @Fs.createReadStream.calledWith("#{@location}/#{@name1Filtered}").should.equal true + @Fs.open.calledWith("#{@location}/#{@name1Filtered}").should.equal true done() describe "with start and end options", -> beforeEach -> - @opts = {start: 0, end: 8} + @fd = 2019 + @opts_in = {start: 0, end: 8} + @opts = {start: 0, end: 8, fd: @fd} + @Fs.open.callsArgWith(2, null, @fd) it 'should pass the options to createReadStream', (done) -> - @Fs.createReadStream.returns({on: ->}) - @FSPersistorManager.getFileStream @location, @name1, @opts, (err,res)=> - @Fs.createReadStream.calledWith("#{@location}/#{@name1Filtered}", @opts).should.equal true + @FSPersistorManager.getFileStream @location, @name1, @opts_in, (err,res)=> + @Fs.createReadStream.calledWith(null, @opts).should.equal true done() describe "error conditions", -> @@ -125,12 +127,10 @@ describe "FSPersistorManagerTests", -> beforeEach -> @fakeCode = 'ENOENT' - @Fs.createReadStream.returns( - on: (key, callback) => - err = new Error() - err.code = @fakeCode - callback(err, null) - ) + err = new Error() + err.code = @fakeCode + @Fs.open.callsArgWith(2, err, null) + it "should give a NotFoundError", (done) -> @FSPersistorManager.getFileStream @location, @name1, @opts, (err,res)=> expect(res).to.equal null @@ -142,12 +142,9 @@ describe "FSPersistorManagerTests", -> beforeEach -> @fakeCode = 'SOMETHINGHORRIBLE' - @Fs.createReadStream.returns( - on: (key, callback) => - err = new Error() - err.code = @fakeCode - callback(err, null) - ) + err = new Error() + err.code = @fakeCode + @Fs.open.callsArgWith(2, err, null) it "should give an Error", (done) -> @FSPersistorManager.getFileStream @location, @name1, @opts, (err,res)=>