Skip to content
This repository was archived by the owner on Mar 10, 2020. It is now read-only.

Add timeout for the ipfs api calls #781

Closed
leonprou opened this issue May 30, 2018 · 9 comments
Closed

Add timeout for the ipfs api calls #781

leonprou opened this issue May 30, 2018 · 9 comments
Labels

Comments

@leonprou
Copy link

I would like to restrict the api call to a certain timeout, and if response not received to have some TimeoutError. Like an HTTP call. Looking into the docs didn't find that option.

@leonprou
Copy link
Author

leonprou commented Jun 4, 2018

@diasdavid hey, what's the ready tag means? Maybe I can give a hand in this?

@daviddias
Copy link
Contributor

Thanks for offering to help! The ready label comes from Waffle, see https://github.com/ipfs/js-ipfs/blob/master/MGMT.md to learn what which label mean.

This feature needs https://github.com/ipfs/interface-ipfs-core/issues/58 which is the crux to have timeouts.

@leonprou
Copy link
Author

leonprou commented Jun 4, 2018

@diasdavid I did some digging at the code, under the hood you're using the request of the http module. This module got a setTimeout method.

So I added the following code after:
https://github.com/ipfs/js-ipfs-api/blob/master/src/utils/send-request.js#L165

    req.setTimeout(timeout, function() {
         console.log('TIMEOUT')
         throw new Error('TIMEOUT')
    })

It did throw an error, so if I'm not missing something we can start with that as a prototype.

@leonprou
Copy link
Author

@diasdavid hey, any comment?

If the contributors are ok with that approach I'd like to make a PR

@obo20
Copy link
Contributor

obo20 commented Jan 14, 2019

Are there any updates on this? Such functionality would be super helpful.

@leonprou
Copy link
Author

@obo20 Eventually I just implemented it with a Promice.race and timeout.

You can see it here https://github.com/ColuLocalNetwork/CLN-community-app/blob/master/server/src/utils/metadata.js#L21

@obo20
Copy link
Contributor

obo20 commented Jan 15, 2019

@leonprou Correct me if I'm wrong, but wouldn't that lead to memory leakage?

It's my understanding that when using Promise.race, the promise that is "timed out" still actually executes code. If the promise never returns, then that memory would effectively be locked wouldn't it? From my research it seems like the js-ipfs-http-client doesn't actually have a way of timing out its own requests so theoretically the request could go on indefinitely, which would lead to memory leakage.

I'm really hoping that I'm wrong on this, and if anybody has a better understanding of this, I'd love to learn more.

For context, I'm using the ipfs.pin.add functionality, so I may never be able to find specific content that I'm requested to find.

@daviddias Is there anything under the hood of the js-ipfs-http-client that cleans up requests after a certain amount of time? Or are these types of memory leaks currently possible? I wasn't able to find any timeouts from my searches.

I do see timeouts being used in the test suite, so I'm curious how this is all accounted for in the tests? It seems throws are utilized, which wouldn't be great in a production environment.

@cle1000
Copy link

cle1000 commented Jun 28, 2019

Hi,
i found out that go-ipfs has a timeout flag, which is not supported by the js-ipfs-http-client, I modified the code for the cmd cat. Now I can pass a timeout flag to ipfs, see #1034

@alanshaw
Copy link
Contributor

Timeouts landed in master today and will be in the next release, see https://github.com/ipfs/js-ipfs-http-client#global-timeouts

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

No branches or pull requests

5 participants