Issue with matb33:collection-hooks

[gregoribic]

I get an error starting my meteor project if I use matb33:collection-hooks and vsivsi:file-collection in the same project. Time ago it was working fine. I get a message ..2#issuecomment-120780592 in the console log.

[vsivsi]

Hi, yes this is expected. collection-hooks has a bug in how it monkey patches Mongo.Collection.

Please do as the error says and:

See https://github.com/vsivsi/meteor-file-sample-app/issues/2#issuecomment-120780592 for a workaround

[gregoribic]

Sorry, but I can't solve the issue. In those comments I read that the Sorter has to deal with the issue, or package order should be modified. How should I control package load order if I can't change their names.

Best regards, Gregor Ibic

[vsivsi]

Mongo.Collection.prototype.constructor = Mongo.Collection;

[thearabbit]

I have the same problem with this. But I don't understand to fix. could clarify for me?

[vsivsi]

matb33:collection-hooks replaces Mongo.Collection which is supplied by Meteor. This causes many potential problems... One of them is that it is buggy in how it does this in that it doesn't properly set the prototype constructor on its new version. This means that a file-collection created when you are using matb33:collection-hooks will not call the correct constructor and will be "half-patched". So it throws an error to alert you of this issue. Until matb33:collection-hooks fixes their bug, you can run the line of code above to patch their patch to make the problem go away. It's a hack, but it works.

Mongo.Collection.prototype.constructor = Mongo.Collection;
myFiles = new FileCollection( ... );

[thearabbit]

Work fine. Very thanks.

[vsivsi]

There's a PR on collection-hooks to fix this, but it hasn't been acted on, here: https://github.com/matb33/meteor-collection-hooks/pull/139

[vsivsi]

I'm reopening this until resolved by matb33:collection-hooks

[arvi]

Hi, I was able to get rid of same issue on my local machine by following this: https://github.com/vsivsi/meteor-file-collection/issues/61#issuecomment-124293863

However, when I mup deploy, I'm getting Invoking deployment process: FAILED

When I run mup logs -f: I'm getting the error again

[128.199.190.128] Error: [The global definition of Mongo.Collection has changed since the file-collection package was loaded. Please ensure that any packages that redefine Mongo.Collection are loaded before file-collection.]
    at new FileCollection (packages/vsivsi:file-collection/src/gridFS_server.coffee:24:23)
    at app/lib/3-collections/1-collections.js:46:18
    at app/lib/3-collections/1-collections.js:66:3
    at /opt/projecthere/app/programs/server/boot.js:222:10
    at Array.forEach (native)
    at Function._.each._.forEach (/opt/projecthere/app/programs/server/node_modules/underscore/underscore.js:79:11)
    at /opt/projecthere/app/programs/server/boot.js:117:5
error: Forever detected script exited with code: 8
error: Script restart attempt #15

[vsivsi]

You need to ensure that the collection hooks package (and any other package that replaces Mongo.Collection) loads before file-collection.

[dpatte]

Its not clear which modules might be replacing Mongo.Collection. Would a good strategy be to put file-collection at the end of the packages list?

[vsivsi]

That should work. You can also trying using meteor-defender to figure out which packages are replacing Meteor APIs: https://github.com/vsivsi/meteor-defender

By changing where defender loads, you can find all of the packages that are directly or indirectly (via their own dependencies) doing this.

[DrDanRyan]

I am having the same issue with the angular package, which uses dburles:mongo-collection-instances. Everything works fine in development mode (without even doing Mongo.Collection.prototype.constructor = Mongo.Collection); however, when doing meteor build or meteor --production my angular app fails to bootstrap because file-collection throws this error. Adding the prototype.constructor patch doesn't fix it in this case either.

Would making file-collection load after angular fix this issue? How can I make that happen? Changing order of listings in .meteor/package does not seem to have any effect.

[vsivsi]

Looking at response to the issue you filed on Meteor, it seems there is currently no way to control load order in production. This is news to me, however.

To be clear, these issues aren't because other packages are "monkey patching" Mongo.Collection in the traditional sense. These errors arise because other packages are replacing/redefining Mongo.Collection with their own (perhaps buggy) implementation that wraps the original.

The issue for file-collection is that when it loads, Mongo.Collection == X; but when it is later called (e.g. new FileCollection()), Mongo.Collection == Y. This causes problems because file-collection is written in Coffeescript and the FileCollection object extends (inherits from, but does not modify) Mongo.Collection. When the definition of Mongo.Collection changes between when FileCollection is defined and when it is used, then all hell breaks loose.

I have wasted hours trying to debug problems reported by users of my packages that came down to bugs in packages that replace Mongo.Collection. It irritated me so much that I wrote vsivsi:meteor-defender to make it possible to easily see which packages are doing what to the Meteor system calls.

Here's the fundamental issue: Any package that uses Mongo.Collection has a potential implicit dependency on other packages that replace it. The point of packages like collection-hooks or mongo-collection-instances is to add features to the vanilla Mongo.Collection. Unlike file-collection, they don't do this by creating a new 'extended' collection type, they do it by replacing Meteor's implementation. Loading these packages creates a global side-effect that can impact other packages, depending on load order.

Here's where the dependency comes in... File-collection doesn't depend on any such packages, but apps that use file-collection might want FileCollection objects to implement say, collection-hooks. That's great, I support that, and it totally works, so long as collection-hooks loads and performs its magic before file-collection loads. When it loads after file-collection, then at best your FileCollection objects won't have collection-hooks extensions, and at worst file-collection is destabilized because the version of Mongo.Collection it is an instanceof is no longer the version that is globally defined.

So the implicit dependency here comes either from the app developer ("I want FileCollections to have collection-hooks"), or even more likely, from the developer of some other package (e.g. angular "I want Mongo.Collection to have mongo-collection-instances"). The problem is that the dependency is transmitted silently through replacement of Meteor provided objects, rather than the explicit definition of new extended types that are new SomeExtendedType() instanceof Mongo.Collection == true.

This accepted way of doing things in the Meteor ecosystem is a bad practice, it makes the system difficult to debug and reason about, and it has been a real pain-point for many users. The fact that the production package loading mechanism appears to have no way for a developer to explicitly say "I want Package A to depend on global side-effects on the definition of Meteor system objects made by the loading of Package B" is a major deficiency, if we are to go on accepting that wrapping and replacement of Meteor defined objects is an acceptable practice.

[DrDanRyan]

@vsivsi: Would changing the error throw to a console warning be a bad thing? It seems that as long as the user is not trying to use a package like collection-hooks directly on a FileCollection they could coexist. There would also be the weirdness of the FileCollection not being an instanceof Mongo.Collection but the warning could point this out. This would make it at least compatible with angular (or any other package relying on this badness) and the --production flag.

[DrDanRyan]

I realized that I was only using the FileCollections on the server so I moved my FileCollections to server/lib instead of the shared lib folder and now I am not having any issues with the --production flag and angular. Bonk I should have realized that one sooner.

[vsivsi]

Glad you found a way through this. I've been asked about the throw decision before, and my answer has consistently been something along the lines of:

file-collection and job-collection are key pieces of infrastructure that developers expect to 'just work'. This issue adds significant uncertainty to that promise, and so these consistency checks are critical. Simple warnings are too easy to miss or ignore, and I'm not willing to support these packages under circumstances that cause these checks to fail. If a developer wishes to bypass these checks, it is simple enough to fork the repo or add the packages directly to your app packages directory and make whatever changes suit your needs, including converting the throws to warnings, or even removing the checks altogether. At least this way you will be forced to make an explicit choice.

[vsivsi]

One other comment: THANK YOU for creating the issue for Meteor on this topic. Seems like it finally got their attention this time. Awareness of this growing problem has been simmering for awhile, and I hope this time it gains traction and something is more formally done about it by MDG.