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

Proposal to increase ipfs metadata timeout to 10 seconds #85

Open
fudgebucket27 opened this issue Apr 4, 2022 · 11 comments
Open

Proposal to increase ipfs metadata timeout to 10 seconds #85

fudgebucket27 opened this issue Apr 4, 2022 · 11 comments

Comments

@fudgebucket27
Copy link
Owner

I'm finding more and more recently minted nfts(especially ones minted through the mint api) returning null metadata since we timeout after 5 seconds. I feel like 10 seconds is the sweet spot? What do you think @modersohn ? Or we show some sort of status on the nft detail and accounts page that says "Could not fetch nft details" if it times out on ipfs. I dont think we show any kind of information currently for that.

@fudgebucket27
Copy link
Owner Author

nftid: 0x96c9930928cee3e7e2e9e8183ceafa745a7d530d01bf4c2421b4941e81d6525d is one example i found that was timing out with the 5 seconds

@modersohn
Copy link
Collaborator

Couldn't we just get the header like for checking content? That way we could get a 404 etc. much quicker and once we know it's actually something there, we can easily increase the timeout.

I think actually this is 2 issues: support NFT data that is slow to load and make the GUI more explicit about load failures/indicators.

@fudgebucket27
Copy link
Owner Author

Thats an even better idea to check the header actually.

@modersohn
Copy link
Collaborator

I just gave this a try, but couldn't get it to work as originally intended:

  1. the original URL might indeed return something, but not the JSON we need. So checking for 404 is not enough
  2. With your audio test-case, the first URL returns the json, but the content-type is not JSON but "application/x-dbf"
  3. even though I made a RestRequest with Method.Head, the response already contained the JSON?

Anyways, I've now tried it the other way around, first trying to get the URL with "/metadata.json" added and then only using that URL, if (response.IsSuccessful). I see that makes all tests work, but I'm not sure we're actually gaining anything.

What do you think? What is the new normal case?

@modersohn
Copy link
Collaborator

It seems piñata is especially slow today. I had multiple failed action runs because of timeouts etc. When debugging I saw internal server error and timeout exceptions.

@modersohn
Copy link
Collaborator

Since I saw a retryable http client being mentioned on discord, "Polly" came up when I googled for something similar for ASP.NET: https://stackoverflow.com/a/65592734/386473

Might be worth a closer look!

@fudgebucket27
Copy link
Owner Author

Ooh looks interesting. It looks like it will work with RestSharp too if I'm not mistaken?

@modersohn
Copy link
Collaborator

modersohn commented Aug 3, 2022

After #239 I looked at this again and again ended up with Polly briefly covering RestSharp (interesting stack overflow link as well). To be honest this looks like overkill to me - for the specific problems we're having, that is in GetMetadataFromURL and GetContentTypeFromURL.

Almost every time a test run fails, re-running the job within a short time will make all test pass again.

So I'd actually suggest we implement a simple retry logic for these two methods only.

@modersohn
Copy link
Collaborator

Here's an NFT from monty whose metadata URL is timing out, even in the browser with a much longer timeout (feels like a minute): 0xd50d1587283f0cb2ca9cd791e29c7ccc68d9e829029cf56410003796653710c1. Because of our retry with a /metadata.json suffix, this takes 20 seconds and you end up with nothing. I wonder what happened here? Can you unpin a IPFS CID and then it's gone?

@fudgebucket27
Copy link
Owner Author

fudgebucket27 commented Aug 7, 2022 via email

@modersohn
Copy link
Collaborator

I now have simple code that tries to get metadata with a timeout of 3s and if that times out, gives it another go with 15s timeout. It's hard to verify that this actually improves things, because piñata is caching, which skews test results. OTOH it's exactly that caching, that makes the 2nd try much more effective than just sitting there for 10s, like we do now.

The problem with this are those unpinned files - they're just timing out, so there's no chance to find out, whether a 2nd try is worth it.

Together with our hack that we might need to add "metadata.json" to the path, this results in 2 * (3 + 15) = 36s wait time, until we give up - from 20s before. Not sure what to do here, we could shorten the 2nd try to 7s only.

@fudgebucket27 what do you think? If you want I can push to a branch on my fork so you can give it a go (or open a PR).

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

2 participants