vsivsi / meteor-file-collection

Extends Meteor Collections to handle file data using MongoDB gridFS.
http://atmospherejs.com/vsivsi/file-collection
Other
159 stars 37 forks source link

Downloading a file loses socket connection #40

Closed dpatte closed 8 years ago

dpatte commented 9 years ago

A very odd problem here.

I am using a file collection and have created a template which shows all the files in the file-collection (their filenames). Each filename is wrapped in an a element which when clicked correctly initiates a download of the file in question.

The collection itself is published using a feature of the reactivePublish module, where I base the publication on this.userId. In my particular case the user has the role admin and can see all the files in the collection.

Meteor.reactivePublish('TheFiles', function() { if (isAdmin(Meteor.users.findOne({_id: this.userId}, {reactive: true})) )
return Files.find(); return null; });

The file list does indeed show all the files in the collection until a download on one of the files is initiated with a click. At that point, reactively, the list of files on the page is emptied, and the filecount goes to zero. This could happen if the click/download clears this.userId used by the publish, or if the roles collection gets disabled.

The file download completes correctly.

But the list of files does not return to the template until I manually refresh the page.

Quite an odd problem!

vsivsi commented 9 years ago

Strange indeed! I've never used the reactivePublish package, but the meteor-file-sample-app works similarly, and does not exhibit this behavior.

When file-collection reads/writes data from gridFS, it uses a separate mongoDB driver from the one that Meteor provides. The reasons for this are complicated, but suffice to say that if reactivePublish works by monkey patching the Mongo object in Meteor, then it will not be getting a full picture of the activity of file-collection. It is similar to if your Meteor app was built on top of a MongoDB instance that was also being updated live by non-Meteor processes (a common case!) Meteor itself works fine in this case, but who knows what is happening in another package that makes assumptions about how/when the DB may be changing based on observing Meteor alone. I'm speculating here...

I suggest that you check out the sample app linked above to see how it works and verify that this problem is probably an issue with reactivePublish.

dpatte commented 9 years ago

Thanks. I actually disconnected the reactive publish and replaced it with a standard publish

Meteor.publish('TheFiles', function() { return Files.find(); });

and the file list still empties when I click a link. This occurs on FF as well as Chrome. But I'll take your advice and check into your sample, and report back if I find something. Maybe there is still something odd in my a elements.

dpatte commented 9 years ago

strangely, adding target="_blank" to the links resolves the problem. The links in the table now look like this

<a href="{{path}}" target="_blank">{{filename}}</a>  

My helper for path is: path: function() { return '/gridfs/Files/'+this._id.valueOf()+'?download=true'; },

vsivsi commented 9 years ago

Hmmm. Using ?download=true causes the server to add a Content-Disposition header to the HTTP response, which should trigger the SaveAs... Dialog behavior in all browsers.

https://github.com/vsivsi/meteor-file-collection/blob/master/http_access_server.coffee#L184

target=_blank should only make a difference in cases where the browser is intending to try to render the response on a page, which the aforementioned HTTP header is designed to prevent.

Does this behavior happen in all browsers with your code? This issue doesn't occur with the sample app (to my knowledge) so something must be different in your code.

If you're willing to share your code so I can reproduce this behavior, I'd be happy to take a look.

dpatte commented 9 years ago

The SaveAs popups up in all my browsers, even before the target= fix. My Browsers are FF 35.0.1 on Ubuntu, FF 37.0.1 on Win 8.1 and Chrome 41.0.2272 on Win8.1. I tried different attributes on the a element, but only the target tag seemed to fix my issue. I'll try to minimize my code and see what I can send along.

vsivsi commented 9 years ago

Any progress on this?

dpatte commented 9 years ago

No, and its difficult to isolate. The problem still exists and my workaround is still to use target=_blank But even with that, most browsers seem to complain that a popup has been attempted at that point, which the user then has to authorize before the download dialog appears.

vsivsi commented 9 years ago

Okay, keep me posted.

dpatte commented 9 years ago

I now realize that the without the target="_blank" that it causes my client to losa the socket connection. My browser console get cleared. This is what wipes out my page. I need to manually refresh the page to get the data back on it. But if I use the target="_blank" it doesnt lose the socket connection.

vsivsi commented 9 years ago

Okay, that kind of makes sense. Although I'm still unsure why this behavior is affecting your code, but doesn't seem to occur with the sample app I linked to above...

If all of your client browsers are HTML5 compatible, you should try the new download attribute on the anchor tag.

<a href="URL" download="filename.ext">

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a

I would hope this will prohibit the behavior you are seeing.

dpatte commented 9 years ago

I haven't gotten any further on this, but thought I should just post my code to see if anything obvious stands out. Please excuse my indenting

Files = new FileCollection('files', { resumable: false, // Enable built-in resumable.js upload support http: [ { method: 'get', path: '/:_id', // this will be at route "/gridfs/Files/:_id" lookup: function (params) { return { _id: params._id}; } }, { method: 'post', path: '/:_id', lookup: function (params) { return { _id: params._id}; } }, { method: 'delete', path: '/:_id', lookup: function (params) { return { _id: params._id}; } } ] });

if (Meteor.isServer) { Files.allow( { insert: function (userId, file) { return true; }, remove: function (userId, file) { return true; }, read: function (userId, file) { return true; }, write: function (userId, file, fields) { return true; } }); }

To store the file I am using

      Files.importFile(filePath,
      {
        filename : 'pipeline.xlsx',
        contentType: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
        metadata: {ownerId: Meteor.userId()}
      }, function(err, file)
      {
        if (err)
        {
          futureResponse.throw(err);
        }
        else
        {
          futureResponse.return('/gridfs/Files/' + file._id);
        }
      });

To download the file I use:

 <a href="{{path}}?download=true" target="_blank">{{filename}}</a>

Where

path: function() { return '/gridfs/Files/'+this._id.valueOf(); },

Not sure if that helps ??

dpatte commented 9 years ago

So the symptom is:

without target="_blank" clicking the href cause the page to lose its socket connection - causing the page to be emptied of data. After completing the download, I can refresh the page and the data comes back

but with target="_blank" the connection is NOT lost, but it complains that the app is generating a popup, and asks me if i want to allow it. If I do, the download dialog appears and all continues as it should.

dpatte commented 9 years ago

I am beginning to think this may be a router issue. I am using Iron.Router, but I dont have anything defined in the iron.router setup for '/gridfs/Files/xxxxxxx?download=true' Perhaps that is causing the lost connection. Do you have any suggestions?

vsivsi commented 9 years ago

The file-collection photo album sample app uses iron-router and doesn't have the issues you describe... https://github.com/vsivsi/meteor-file-job-sample-app

vsivsi commented 9 years ago

Really the only way I can help with this is if you create a repo with a minimal, complete working Meteor app that reproduces the issue. If I can make it happen on one of my boxes, then I can try to figure it out, otherwise all I can do is speculate, and I'm pretty much out of ideas.

dpatte commented 8 years ago

I have resolved the issue. I will post more complete details shortly, but in essence what I had were issues in the difference between browsers requirements to download files. The issue was not in your module after all.

vsivsi commented 8 years ago

@dpatte Glad you figured it out!