-
Notifications
You must be signed in to change notification settings - Fork 106
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
bug: endless session routing #501
Comments
If we don't handle this automatically, we should probably expose routers on helia/packages/interface/src/routing.ts Line 53 in 13daa26
|
running in test env locally, it seems like the problem is no providers have http/https and so we're just pausing because trustless-gateway/session.ts is not yielding any providers. In our test environment, we have a local test kubo configured with ipfsd-ctl that should be used in the session, or fallen back to somehow
|
FYI I threw together https://github.com/SgtPooki/repro-kubo-not-announcing-self-as-provider which shows that kubo doesn't announce itself as a provider, so we need some solution for our tests |
playing around with patch-package. I think we need some config option to allow using the Here are the diffs that solved my problem:
|
#501 (comment) also allows us to bypass the piping through of "allowLocal" and "allowInsecure" when we explicitly provide a local/insecure gateway in |
I guess maybe we want to limit how many times we search for providers unsuccessfully before giving up. Right now it'll give up if you pass a signal and that signal aborts (e.g. after a timeout) but maybe that's too subtle. As an aside I do think we should pass signals into every function that accepts them in our examples, to try to normalise the practice.
I was thinking about something like this but if we do that, you might as well not use the session? |
I think it would still be valuable with sessions, because there's no way to know if there are valuable providers or not until you try. If we don't find https providers after a timeout (we can use signals here sure) we could use the gateways configured on the root TrustlessGateway instance |
I think helia/packages/utils/src/abstract-session.ts Lines 119 to 124 in be86ac7
We |
I am able to reproduce this consistently with the steps in ipfs/helia-verified-fetch#84 Here is a section of the log, that slowly increases in the duplicates we find, and yet we still keep querying them all:
I've truncated the log a lot (33M of repeatedly querying the same local gateway in less than 10 seconds) log.txt but you can see some of the issues. I also modified abstract-session in node_modules with:
and don't see either of those lines in the log at all. |
Repro test in #538 |
When new providers are found, check the eviction filter in `isEvicted` rather than the current list of providers. Fixes #501
When new providers are found, check the eviction filter in `isEvicted` rather than the current list of providers. Fixes #501
When new providers are found, check the eviction filter in `isEvicted` rather than the current list of providers. Fixes #501
When new providers are found, check the eviction filter in `isEvicted` rather than the current list of providers. Fixes #501
When new providers are found, check the eviction filter in `isEvicted` rather than the current list of providers. Fixes #501
When looking at ipfs/helia-verified-fetch#50, I ran into an issue: ipfs/helia-verified-fetch#50 (comment)
It seems like a request for QmVP2ip92jQuMDezVSzQBWDqWFbp9nyCHNQSiciRauPLDg endlessly polls for providers of
QmVP2ip92jQuMDezVSzQBWDqWFbp9nyCHNQSiciRauPLDg
with the default delegated-ipfs.dev router. I think this is due tohelia/packages/utils/src/abstract-session.ts
Lines 215 to 254 in 13daa26
something like
maxProviders++
is occurring (i.e. #495 (comment)), but much less obviouslyThough the real fix in helia-verified-fetch is to fix tests to be offline only, we should probably provide a default timeout because i'm sure others not in test environments will also run into this.
references:
647b6cb
(#55) was the first fix while investigating.The text was updated successfully, but these errors were encountered: