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

Fix infinite loading on file info #1977

Merged
merged 3 commits into from
Oct 18, 2023
Merged

Conversation

bogdanfazakas
Copy link
Member

@bogdanfazakas bogdanfazakas commented Oct 11, 2023

Fixes #1938 .

Changes proposed in this PR:

  • fixes infinite file loading
  • updates provider error handling

NOTE: requires ocean.js lib 3.1.3 (PR uses pre release ocean.js lib 3.1.3-next.1)
test asset: https://market-git-fix-infinite-file-error-handling-oceanprotocol.vercel.app/asset/did:op:8de687b14e5181c160a8ab602038f16ddc6557ed3ab494d4bc488d221686164d

@vercel
Copy link

vercel bot commented Oct 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
market ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 16, 2023 8:46pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
dubai-challenge ⬜️ Ignored (Inspect) Visit Preview Oct 16, 2023 8:46pm

@bogdanfazakas bogdanfazakas changed the title error handling updates Fix infinite loading on file info Oct 11, 2023
@jamiehewitt15
Copy link
Member

I opened up the test asset page and it's failing to load the price, just seems to be continuously loading. Is this due to the ocean.js version bump that's required?

image

@bogdanfazakas
Copy link
Member Author

bogdanfazakas commented Oct 13, 2023

I opened up the test asset page and it's failing to load the price, just seems to be continuously loading. Is this due to the ocean.js version bump that's required?

no it should not be related to ocean.js update, but the asset above requires about 1 min wait time untill the fileInfo request fails due to bag gateway, see print screen attached

Screenshot 2023-10-13 at 11 25 09

@LoznianuAnamaria
Copy link
Contributor

LoznianuAnamaria commented Oct 13, 2023

I tested it out a bit and I've seen the errors displayed, but the spinner is still loading.
Screenshot 2023-10-13 at 11 53 41

@LoznianuAnamaria
Copy link
Contributor

Noup, I was wrong. Waited a bit longer and I the spinner stoped and another error displayed

@LoznianuAnamaria LoznianuAnamaria self-requested a review October 13, 2023 08:56
@codeclimate
Copy link

codeclimate bot commented Oct 16, 2023

Code Climate has analyzed commit 277ccb5 and detected 4 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 4

The test coverage on the diff in this pull request is 0.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 20.1% (0.0% change).

View more on Code Climate.

@mihaisc
Copy link
Contributor

mihaisc commented Oct 18, 2023

good to go? i see it was approved

@bogdanfazakas
Copy link
Member Author

good to go? i see it was approved

yes but Ana is not a code owner so i should have bypassed the protections to merge it 😄

@mihaisc mihaisc merged commit d613f19 into main Oct 18, 2023
13 checks passed
@mihaisc mihaisc deleted the fix/infinite-file-error-handling branch October 18, 2023 13:53
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

Successfully merging this pull request may close these issues.

File download fails
4 participants