Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing request timeouts in web browser #15

Closed
lidel opened this issue Jul 23, 2019 · 2 comments
Closed

Missing request timeouts in web browser #15

lidel opened this issue Jul 23, 2019 · 2 comments

Comments

@lidel
Copy link
Member

lidel commented Jul 23, 2019

Throttling introduced in #12 helps with request flood, but there is a separate problem of the lack of request timeouts in web browser. Some requests can take a very long time and will suffocate request queue because they have no built-in timeout (namely /api/v0/refs):

slow-6-refs-suffocating-pending-calls-2019-07-23--00-18-22

This also means we can't use js-ipfs-http-client because it does not support cancellations: https://github.com/ipfs/js-ipfs-http-client/issues/694. For this very reason js-ipfs does not use ipfs-http-client for preload calls to /api/v0/refs: instead it is using manual fetch with AbortController in web browser (preload-browser.js) and req.abort in Node (preload-nodejs.js)

We need to do the same, or use js-ipfs-http-client-lite (add missing commands if needed).

@jacobheun @alanshaw thoughts on which way is better? Is this library aimed only at browsers, or will it be used in Node as well?

@alanshaw
Copy link
Member

It'll be used in all environments.

If we call provide every CID we add (which I think we do) then we don't need the recursive flag. This will reduce the RTT for each call to provide significantly.

@achingbrain
Copy link
Member

This module has an internal request queue so this should be fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants