alanning / meteor-package-stubber

Auto-stubber for Meteor smart packages
MIT License
5 stars 3 forks source link

Error stubbing Meteor.Collection export #5

Closed ghost closed 10 years ago

ghost commented 10 years ago

Error when stubbing the exports of Velocity. The error occurs in the mirror. So, you can only see it when you change to stdio: 'inherit' for the mirror spawn in Velocity. I get the error when I start velocity-example.

I20140716-21:59:46.183(2)? [PackageStubber] Error generating stub for exported object 'VelocityTestFiles in package 'velocity'. VelocityTestFiles' has been stubbed with an empty object but if you receive errors due to missing fields in this package, you will need to supply your own custom stub. The original error follows:
I20140716-21:59:46.183(2)?  Maximum call stack size exceeded
I20140716-21:59:46.454(2)? [PackageStubber] Error generating stub for exported object 'VelocityFixtureFiles in package 'velocity'. VelocityFixtureFiles' has been stubbed with an empty object but if you receive errors due to missing fields in this package, you will need to supply your own custom stub. The original error follows:
I20140716-21:59:46.454(2)?  Maximum call stack size exceeded
I20140716-21:59:46.632(2)? [PackageStubber] Error generating stub for exported object 'VelocityTestReports in package 'velocity'. VelocityTestReports' has been stubbed with an empty object but if you receive errors due to missing fields in this package, you will need to supply your own custom stub. The original error follows:
I20140716-21:59:46.634(2)?  Maximum call stack size exceeded
I20140716-21:59:46.771(2)? [PackageStubber] Error generating stub for exported object 'VelocityAggregateReports in package 'velocity'. VelocityAggregateReports' has been stubbed with an empty object but if you receive errors due to missing fields in this package, you will need to supply your own custom stub. The original error follows:
I20140716-21:59:46.774(2)?  Maximum call stack size exceeded
I20140716-21:59:46.935(2)? [PackageStubber] Error generating stub for exported object 'VelocityLogs in package 'velocity'. VelocityLogs' has been stubbed with an empty object but if you receive errors due to missing fields in this package, you will need to supply your own custom stub. The original error follows:
I20140716-21:59:46.936(2)?  Maximum call stack size exceeded
alanning commented 10 years ago

Guessing its due to a symbolic link.

On Jul 16, 2014, at 4:05 PM, Jonas Aschenbrenner notifications@github.com wrote:

Error when stubbing the exports of Velocity. The error occurs in the mirror. So, you can only see it when you change to stdio: 'inherit' for the mirror spawn in Velocity. I get the error when I start velocity-example.

I20140716-21:59:46.183(2)? [PackageStubber] Error generating stub for exported object 'VelocityTestFiles in package 'velocity'. VelocityTestFiles' has been stubbed with an empty object but if you receive errors due to missing fields in this package, you will need to supply your own custom stub. The original error follows: I20140716-21:59:46.183(2)? Maximum call stack size exceeded I20140716-21:59:46.454(2)? [PackageStubber] Error generating stub for exported object 'VelocityFixtureFiles in package 'velocity'. VelocityFixtureFiles' has been stubbed with an empty object but if you receive errors due to missing fields in this package, you will need to supply your own custom stub. The original error follows: I20140716-21:59:46.454(2)? Maximum call stack size exceeded I20140716-21:59:46.632(2)? [PackageStubber] Error generating stub for exported object 'VelocityTestReports in package 'velocity'. VelocityTestReports' has been stubbed with an empty object but if you receive errors due to missing fields in this package, you will need to supply your own custom stub. The original error follows: I20140716-21:59:46.634(2)? Maximum call stack size exceeded I20140716-21:59:46.771(2)? [PackageStubber] Error generating stub for exported object 'VelocityAggregateReports in package 'velocity'. VelocityAggregateReports' has been stubbed with an empty object but if you receive errors due to missing fields in this package, you will need to supply your own custom stub. The original error follows: I20140716-21:59:46.774(2)? Maximum call stack size exceeded I20140716-21:59:46.935(2)? [PackageStubber] Error generating stub for exported object 'VelocityLogs in package 'velocity'. VelocityLogs' has been stubbed with an empty object but if you receive errors due to missing fields in this package, you will need to supply your own custom stub. The original error follows: I20140716-21:59:46.936(2)? Maximum call stack size exceeded — Reply to this email directly or view it on GitHub.

ghost commented 10 years ago

I thought circular reference. The Velocity export has no problem. I will test it without symlink.

EDIT: Just realized that meteorite uses symlinks too. So definitely no symlink problem.

alanning commented 10 years ago

Right, the problem is a circular reference. Was thinking the mechanism that triggered it was probably a symlink. Just a first guess based on other circular references I've run into with package stubber.

Could also just be a normal circular reference in the objects that are exported.

Can you list steps to reproduce? I'm not familiar with the mirror 'inherit' stdio config.

ghost commented 10 years ago

I think I had the same problem with cloning a object that contained a reference to a Meteor.Collection.

  1. Clone velocity-example
  2. mrt install
  3. Open packages/velocity/main.js and search for 'pipe' and replace it with 'inherit' (https://github.com/xolvio/velocity/blob/master/main.js#L309). You will now also see the mirror console output.
  4. mrt
alanning commented 10 years ago

I'm having trouble reproducing that error with those steps. Will try more after dinner.

Dealing gracefully with cir-ref's would be nice. But I'm pretty sure we don't want PackageStubber stubbing the velocity stuff anyways, otherwise the mirror can't report results, right?

ghost commented 10 years ago

It only happens sometimes. Must be some internal state of the collection. I think stubbing Velocity is currently not a problem but I am also for adding Velocity to the ignore list. That should be enough for now. Exporting Meteor.Collection's isn't very common, right?

alanning commented 10 years ago

"Exporting Meteor.Collection's isn't very common, right?"

Right, I've never run into that before. Pretty sure that's not something that is required or good to do; I don't think I do that for the Roles package. Sam says the mirrors use the Meteor methods to report values so no issues there.

Have to think about that more...

samhatoum commented 10 years ago

correction

"Sam says he thinks the mirrors use the Meteor methods"

:)

alanning commented 10 years ago

FYI, this article says to export the Collection: http://shiggyenterprises.wordpress.com/2013/10/24/common-mistakes-made-while-developing-with-meteor-collections/

So its not unheard of

ghost commented 10 years ago

Yeah, we need the collection exports at least for read access for the plugins (reporter for example). I just meant, that I think it is enough for now to just ignore Velocity for stubbing, instead of implementing circular reference support or general Meteor.Collection support.

alanning commented 10 years ago

yep. PackageStubber should be ignoring the velocity package already so there's a bug there somewhere. Need to get a reliable repro going tho. or we can do a hangout-pair programming session if you can get it to reliably occur

ghost commented 10 years ago

Ah, ok. There is a bug here: https://github.com/alanning/meteor-package-stubber/blob/master/package-stubber/main.js#L313. options.packagesToIgnore contains the package names. But you compare it with the file path to package.js (for example 'velocity' !== 'velocity/package.js')

alanning commented 10 years ago

Nice find!

That was introduced in a recent refactoring. Formerly using a function that would compare against the start of the file path, now using a set.

Need more tests...

On Jul 16, 2014, at 8:21 PM, Jonas Aschenbrenner notifications@github.com wrote:

Ah, ok. There is a bug here: https://github.com/alanning/meteor-package-stubber/blob/master/package-stubber/main.js#L313. options.packagesToIgnore contains the package names. But you compare it with the file path to package.js (for example 'velocity' !== 'velocity/package.js')

— Reply to this email directly or view it on GitHub.

alanning commented 10 years ago

I'm guessing this is fixed already with more recent updates? I'll close for now and we can re-open if its still happening?