Handle error on reader.read error#24450
Conversation
joshualim92
force-pushed
the
rethrow-reader-error
branch
from
1ab90e9 to
88c059a
Compare
| } ).catch( err => { | ||
|
|
||
| } | ||
| throw err; | ||
|
|
There was a problem hiding this comment.
What are you expecting this to do? Throwing the same error you caught is effectively a no op.
There was a problem hiding this comment.
Apologies, when I was running this locally I thought this was sufficient but it was only because something went wrong with my hot reload.
I've replaced the commit I've had before so hopefully it'll be clearer in what the intent is. I'm open to altering the approach though. I'm not sure if a shared error handler is preferred here. Another solution is to switch this to be an async/await so it's easier to have an outer error handle for the recursive promises.
joshualim92
force-pushed
the
rethrow-reader-error
branch
from
88c059a to
9df7cd4
Compare
joshualim92
changed the title
|
|
||
| function readData() { | ||
| if ( done ) { | ||
|
|
||
| reader.read().then( ( { done, value } ) => { | ||
| controller.close(); | ||
|
|
||
| if ( done ) { | ||
| } else { | ||
|
|
||
| controller.close(); | ||
| loaded += value.byteLength; | ||
|
|
||
| } else { | ||
| const event = new ProgressEvent( 'progress', { lengthComputable, loaded, total } ); | ||
| for ( let i = 0, il = callbacks.length; i < il; i ++ ) { | ||
|
|
||
| loaded += value.byteLength; | ||
| const callback = callbacks[ i ]; | ||
| if ( callback.onProgress ) callback.onProgress( event ); | ||
|
|
There was a problem hiding this comment.
Can we revert these indentation changes? It makes it hard to follow what has meaningfully changed. Please take make a minimal change we can more easily follow the code adjustments.
joshualim92
force-pushed
the
rethrow-reader-error
branch
from
9df7cd4 to
02e5031
Compare
|
Great thanks - can you provide steps on how to reproduce the issue for testing? |
For sure, the steps involve using the dev tools to get since it's an edge case error while downloading. For ease of testing I created a code sandbox, please let me know if you'd prefer a different format https://codesandbox.io/s/tender-sinoussi-cjnzko For ease of testing it uses one of our models that's a little larger. Steps:
Expected: Actual:
|
|
Should be fixed via #28216. |
3 participants
Description
Ran into an edge case where if the network is interrupted during a download, an uncaught network error occurs. Unfortunately FileLoader isn't able to recover from here because the url is still listed in the module level
loadingobject. There is already a catch handler for the fetch call which should handle this case so this is just added in so the error handling is propagated up.This contribution is funded by VRIFY