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

Asset loading hangs forever when errors hit loading an asset. #5032

Closed
diarmidmackenzie opened this issue Mar 22, 2022 · 7 comments
Closed

Comments

@diarmidmackenzie
Copy link
Contributor

Description:

When a URL referred to by an asset is not resolvable, A-Frame scene loading just hangs forever.

See linked Glitch, which is a modified version of this https://aframe.io/examples/showcase/modelviewer/ but with the GLTF URL set to an invalid value.

One reason why it would be nice to initialize the scene, rather than hang forever, is that it would allow application-specific error handling.

I have a potential fix for this that seems to work, which I'll present as a PR.

@diarmidmackenzie diarmidmackenzie changed the title Asset loading hangs for ever when errors hit loading assets Asset loading hangs forever when errors hit loading an asset. Mar 22, 2022
@vincentfretin
Copy link
Contributor

vincentfretin commented Oct 6, 2022

It would be interesting to have an example on how you can catch error or timeout and show a message to the user.
If I have a single glb in my scene and this one produces an404 not found or 502 bad gateway (the timeout error produces a different result), with your PR it will now show a white canvas instead of infinite loading screen. I'm not sure which one is better here...
About timeout and loaded event, there is also those two old issues #2737 and #1155 that are still relevant.
Also currently the timeout produces an error with stacktrace #5130

@vincentfretin
Copy link
Contributor

Actually I looked at #2737 and #1155 those two should be closed really.

@kylebakerio
Copy link
Contributor

You can catch the error event that the individual THREE loaders emit. Just:

[...document.querySelectorAll('a-asset-item')].forEach(el => { el.addEventListener('error', evt => {
   console.error(evt);
   // if you decide you want to fool A-Frame into continuing, just run el.emit('loaded',{});
})

(from memory, you can look at a-load-screen for the actual implementation)

@vincentfretin
Copy link
Contributor

If it's the big glb representing the whole scene that gave an error, I probably want to show the user a message "something went wrong, please try again later" instead of showing a white canvas. If it's smaller glb that may not be so important for the experience, I may want to still enter the experience, yes, but this is a case by case thing.

@kylebakerio
Copy link
Contributor

kylebakerio commented Oct 6, 2022

yes, you can put whatever handling you want there. you can see what element threw the error and handle accordingly. a basic option would be set localStorage.retryCount = true;' and then location.reload()`. then, if it happens again, you also have a check; if you see that you've already reloaded, then you just throw an error message to user saying "site is down for maintenance" or something.

and/or just ignore if file is unimportant, which you can see by checking el.src.

@vincentfretin
Copy link
Contributor

vincentfretin commented Oct 6, 2022

Ok, the choice of showing a message to the user or retry downloading an asset that gave an error can be done in your app. An external library that implements another loader can help with that too.
For aframe core, the logic is currently to load the scene after the timeout, so it actually hides loading screen, and some other glb may finish downloading and appears after the timeout if understand it, at least the is the behavior I currently see.
To be consistent, it should indeed load the scene even if there was an error, which it doesn't do currently, the PR #5033 fixes that.

dmarcos pushed a commit that referenced this issue Dec 6, 2022
)

* Better handling of asset loading errors.

* Simple test for asset loading error.

* Ensure all assets loaded even when one is in error.  Plus test cases.

* fix typos

* Remove trailing spaces to pass linting checks.

* Fix linting errors arising from merge

* Fix up merge

* Fix test script that fails due to inadequate mocking of canvas.

* Revert "Fix test script that fails due to inadequate mocking of canvas."

This reverts commit 84a0d63.

* Remove example - this is now tested in UT.
@dmarcos
Copy link
Member

dmarcos commented Dec 6, 2022

Fixed by 5189765

@dmarcos dmarcos closed this as completed Dec 6, 2022
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

4 participants