Netflix / falcor-router

A Falcor JavaScript DataSource which creates a Virtual JSON Graph document on your app server.
http://netflix.github.io/falcor
Apache License 2.0
104 stars 46 forks source link

Big ranges confuse the router. #171

Open ThePrimeagen opened 8 years ago

ThePrimeagen commented 8 years ago

https://github.com/Netflix/falcor/issues/594

Eluinhost commented 8 years ago

Is there any way to deal with this currently? I'm evaluating different possibilities for my API and falcor seems to be what I want at the moment but the ease of abuse of the endpoint seems to be too easy.

I set up the following route:

module.exports.push({
    route: 'testItemsById[{integers}].id',
    get: ([,itemIds]) => itemIds.map(itemId => jsong.pathValue(['testItemsById', itemId, 'id'], jsong.atom(itemId)))
});

Running the above with stuff like {"to":"10000"} works fine, moving to 100,000 uses more RAM (up to about 120MB for the node process) and takes longer but still not much of an issue. Moving to 1,000,000+ causes RAM to quickly fluctuate between 400MB-1GB and never seems to actually finish, locking up the server; this is a very simple endpoint.

Adding if (itemIds.length > 10000) throw new Error('Too many items requested'); does improve the responsiveness for 1,000,000 but doesn't stop me from requesting 10,000,000 which then causes the server process to crash due to running out of memory.

Is the only way to handle this to parse the request myself? I could then calculate the amount of items it would be requesting before the router processes it and 400 error if too much is requested. I don't know how the falcor client would even handle that.

Ideally I would like to be able to add rate limiting based on the amount and/or weight of items requested and have the client be able to respect the limits with the batched requests, is this in a roadplan or currently possible?

sdesai commented 8 years ago

Hey @Eluinhost I think it makes sense to build the check into the falcor router. I'll work on adding it. It may also make sense to have the client batch based on this rate limit, but haven't thought it through currently. It's worth considering/adding to the roadmap plan.