Three.js: VRMLLoader does no error forwarding calling the parser

[stewe]

Description of the problem

When an error occurs while parsing the loaded VRML file, the VRMLLoader does not handle it.

        load: function ( url, onLoad, onProgress, onError ) {

            var scope = this;

            var path = ( scope.path === '' ) ? LoaderUtils.extractUrlBase( url ) : scope.path;

            var loader = new FileLoader( this.manager );
            loader.setPath( scope.path );
            loader.load( url, function ( text ) {

                onLoad( scope.parse( text, path ) );

            }, onProgress, onError );

        },

It should call onError instead.

        load: function ( url, onLoad, onProgress, onError ) {

            var scope = this;

            var path = ( scope.path === '' ) ? LoaderUtils.extractUrlBase( url ) : scope.path;

            var loader = new FileLoader( this.manager );
            loader.setPath( scope.path );
            loader.load( url, function ( text ) {

              try {
                const parsed = scope.parse( text, path )
                  onLoad(parsed);

                         } catch (e) {
                            onError(e)
                       }

            }, onProgress, onError );

        },

Three.js version

• r116

Browser

• [x] All of them
• [ ] Chrome
• [ ] Firefox
• [ ] Internet Explorer

OS

• [x] All of them
• [ ] Windows
• [ ] macOS
• [ ] Linux
• [ ] Android
• [ ] iOS

[Mugen87]

It seems VRMLLoader is not the only loader which does not use try/catch around the invocation of parse(). It might be worth to check all loaders and change the code according to GLTFLoader.

[mrdoob]

I think loaders should use try/catch. It's annoying when it bubbles up to the Editor's Loader.js.

[Mugen87]

The code should actually look like so since onError() is no mandatory callback function. It's also useful to tell the loading manager that an error occured.

try {

    onLoad( scope.parse( text, path ) );

} catch ( e ) {

    if ( onError ) {

        onError( e );

    } else {

        console.error( e );

    }

    scope.manager.itemError( url );

}

[mrdoob]

Looks good to me!

[Mugen87]

Okay, I've added today a PR with the fix.