If HTTP connection closed prematurely, consider retry #16025
Conversation
xhr2 (used for HTTP under node) will return response=null if the server returns a response but the connection is closed before the full response has been downloaded. Previously this would lead to a null reference in some handlers, e.g. glTFFileLoader would throw "Cannot read properties of null (reading 'byteLength')". With this change onSuccess is not called if the response is null; instead the retry logic can choose to retry or error.
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/16025/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/16025/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/16025/merge#BCU1XR#0 |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU (Experimental) |
1 similar comment
|
Visualization tests for WebGPU (Experimental) |
|
WebGL2 visualization test reporter: |
|
Could you share a link to the file in node ? |
|
Do you mean the xhr2.js file? I'm not sure I can easily send a link because the source (https://github.com/pwnall/node-xhr2) uses CoffeeScript which is a language I'm not familiar with. The following commands will get you a copy of the file I'm seeing though: Line numbers there agree with the stack trace I included in the forum post: _onHttpResponseClose is the place to look (line 637). It sets the ready state to DONE, which is what triggers the callback into Babylon's fileTools code. A couple of lines above it calls _setError which (among other things) sets response to null. _setError does not change status though. status is set on line 574 of xhr2.js, when the response headers have been received. If I'm understanding the code correctly: If the response headers are received status will be set, and if the connection is then closed before the whole response is downloaded then the response is set to null, the status is left unchanged but the ready state is set to DONE and the onReadyState callback is called. Babylon checks the status code, sees that it's OK and calls the callback with a null response, which (at least in the case of the gltfLoader) isn't expected and leads to the exception in the stack trace above. |
|
looking at It looks like null check only is fine and it should end up retrying so all good for me. |
|
Wait just noticing that null can be a valid response in the JSON case. I think this check should be added then. |
xhr2 (used for HTTP under node) will return response=null if the server returns a response but the connection is closed before the full response has been downloaded. Previously this would lead to a null reference in some handlers, e.g. glTFFileLoader would throw "Cannot read properties of null (reading 'byteLength')". With this change onSuccess is not called if the response is null; instead the retry logic can choose to retry or error.