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

Catch promise rejections in GLTFParser.loadNode #15350

Merged
merged 2 commits into from Dec 3, 2018

Conversation

@thecharhan
Copy link
Contributor

commented Nov 30, 2018

The current GLTFLoader behavior will silently fail with an uncaught promise rejection if an error (say, a bad buffer file load) happens while parsing a node. This change should propagate such errors up to the user's onError function.

@@ -3129,16 +3129,16 @@ THREE.GLTFLoader = ( function () {

var nodeDef = json.nodes[ nodeIndex ];

return new Promise( function ( resolve ) {
return function( ) {

This comment has been minimized.

Copy link
@donmccurdy

donmccurdy Nov 30, 2018

Collaborator

I think we'll need to reorganize this a little at some point — would be nice to flatten it and avoid the IIFE here, and also see #14875. But this looks like an improvement for now. 👍

This comment has been minimized.

Copy link
@donmccurdy

donmccurdy Nov 30, 2018

Collaborator

actually, for now could you just wrap the IIFE in parentheses, with no space between the ()?

( function() {
  // ...
}() )

I find that makes it easier to see we've returned a promise, not a function. Thanks!

This comment has been minimized.

Copy link
@thecharhan

thecharhan Nov 30, 2018

Author Contributor

Done, thanks!

@mrdoob mrdoob added this to the r100 milestone Dec 3, 2018

@mrdoob mrdoob merged commit 1cd938a into mrdoob:dev Dec 3, 2018

2 checks passed

LGTM analysis: JavaScript No alert changes
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mrdoob

This comment has been minimized.

Copy link
Owner

commented Dec 3, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.