Searching crashes sinopia

[yamalight]

Doing a clean npm install sinopia (tested v1.2.1), running sinopia afterwards and trying to execute npm --registry http://localhost:4873/ search lodash crashes the sinopia with following error:

 http  --> 200, req: 'GET https://registry.npmjs.org/-/all/since?stale=update_after&startkey=1428935892000', bytes: 0/2811985
 fatal --- uncaught exception, please report this
RangeError: Maximum call stack size exceeded
    at sigmund (/Users/yamalight/Server/test/node_modules/sinopia/node_modules/minimatch/node_modules/sigmund/sigmund.js:2:18)
    at new Minimatch (/Users/yamalight/Server/test/node_modules/sinopia/node_modules/minimatch/minimatch.js:153:35)
    at Function.minimatch.makeRe (/Users/yamalight/Server/test/node_modules/sinopia/node_modules/minimatch/minimatch.js:782:10)
    at Config.get_package_spec (/Users/yamalight/Server/test/node_modules/sinopia/lib/config.js:154:19)
    at Auth.allow_access (/Users/yamalight/Server/test/node_modules/sinopia/lib/auth.js:132:43)
    at /Users/yamalight/Server/test/node_modules/sinopia/lib/index-api.js:90:14
    at iterate (/Users/yamalight/Server/test/node_modules/sinopia/node_modules/async/lib/async.js:149:13)
    at /Users/yamalight/Server/test/node_modules/sinopia/node_modules/async/lib/async.js:160:25
    at /Users/yamalight/Server/test/node_modules/sinopia/lib/index-api.js:102:11
    at /Users/yamalight/Server/test/node_modules/sinopia/lib/auth.js:143:22

Tried using with iojs v1.7.1 and node v0.12.2 - same result.

[yamalight]

That's actually seems to be related to the way async.eachSeries works. Replacing it in /lib/index-api.js:90 with promises fixes the crash.
Should I do the promises based fix and make a PR or do you have other fix in mind?

[jinkwon]

@yamalight
Have you try change stack size?
We had same issue, and fixed with this run option.
node --stack-size=100000000 /path/to/app.js

[yamalight]

@jinkwon yep, I did. decided that's no reliable enough. I'd fixed it with promises in a fork in the end.

[jinkwon]

@yamalight
Great.
Thanks for your works.
I'll do more test with your forked codes as well.
If I got some more issue, I'll leave comment in here.

[yamalight]

@jinkwon sure. I made some additional fixes there as well. feel free to poke around and write issues in the forked repo as well (if they are fork-related)

[rlidwka]

This and all other search issues should be fixed with #253 , please check that out.

I guess it was related to async.eachSeries, we should've used process.nextTick to delay callback invocation. That's a pretty novice mistake on my part. But the search code was so terrible, that I was thinking about removing/rewriting it anyway. So here we are.

PS: Maximum call stack size exceeded errors are always bugs in this code. Please report when you see them, I mean --stack-size is not really a good option.

[yamalight]

@rlidwka #253 works for me!

[yamalight]

@rlidwka any ETA on merging/publishing this fix?

[rlidwka]

any ETA on merging/publishing this fix?

I'm thinking this weekend as 1.4.0 with a couple of other fixes. Is there a reason to hurry up on that?

[yamalight]

@rlidwka not really :)
it's just that we're using it internally quite a lot and search is pretty important for us.
now we're using the self-fixed fork I made, but I'd love to switch back to main stable version.

[rlidwka]

Sorry it took me so much time to release it.

Fix published as [email protected], thanks for reporting!

[yamalight]

awesome, thanks! 👍