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.

Sign up for GitHub

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

Jump to bottom

Decoder hang with invalid URL #16

Closed
elalish opened this issue Nov 27, 2023 · 4 comments
Closed

Decoder hang with invalid URL #16

elalish opened this issue Nov 27, 2023 · 4 comments
Assignees
@daniele-pelagatti
Labels
bug Something isn't working

Comments

@elalish
Copy link

elalish commented Nov 27, 2023 •
edited
Loading

We have a test that's now failing upon incorporating HDRJPGLoader in google/model-viewer#4578:

test('throws on invalid URL', async () => {
      try {
        await textureUtils.loadEquirect('');
        expect(false).to.be.ok;
      } catch (e) {
        expect(true).to.be.ok;
      }
    });

It appears to time out reliably, so I think it's just hanging instead of throwing? Our similar test for a not found URL is throwing properly: await textureUtils.loadEquirect('./nope.png');
@elalish
Copy link
Author

elalish commented Nov 27, 2023

Also, it's interesting to note that HDRJPGLoader successfully loads not just SDR JPEG images, but also PNGs (which is great!). However, it makes the name slightly misleading...

@daniele-pelagatti daniele-pelagatti added the bug Something isn't working label Nov 28, 2023
@daniele-pelagatti daniele-pelagatti self-assigned this Nov 28, 2023
@daniele-pelagatti
Copy link
Contributor

I'm getting a proper error thrown when trying to load an invalid url in the three.js example:

with both the sync method
image

and the async one
image

I need to investigate this further

@daniele-pelagatti
Copy link
Contributor

ahhh, right, nevermind my previous comment, I misread your report, this seems to happens only with an empty string, I can reproduce the problem now, I'm investigating

@daniele-pelagatti daniele-pelagatti mentioned this issue Nov 28, 2023
Merged
daniele-pelagatti added a commit that referenced this issue Nov 29, 2023
@daniele-pelagatti
fix(loaders): properly catches render errors and calls onError callback
b9bcdd1 
related issue: #16

the problem is caused by the fact that an empty string or any other "real-file-but-not-an-image" (an empty string is a request to load index.html basically) gets fulfilled by the server with a 200 OK status, returned by FileLoader -> we get the HTML in return (which is not an image) and then we try to render it anyway.

this catches errors in the LoaderBase render function and properly calls the onError callback
daniele-pelagatti pushed a commit that referenced this issue Nov 29, 2023
@semantic-release-bot
chore(release): 3.0.0 [skip ci]
c49f21d 
# [3.0.0](v2.0.7...v3.0.0) (2023-11-29)

### Bug Fixes

* **loaders:** properly catches render errors and calls onError callback ([b9bcdd1](b9bcdd1)), closes [#16](#16)

### Features

* **core:** disables default mipmap generation, enables user to specify renderTarget (and toDataTexture) options ([147d278](147d278)), closes [#14](#14) [#15](#15)

### BREAKING CHANGES

* **core:** `generateMipmaps` is no longer `true` by default, both `minFilter` is  no longer `LinearMipMapLinearFilter` by default but `LinearFilter`, `wrapS` and `warpT` are no longer `RepeatWrapping` by default but `ClampToEdgeWrapping`
@daniele-pelagatti
Copy link
Contributor

this should be fixed in v3.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@daniele-pelagatti daniele-pelagatti

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@elalish @daniele-pelagatti