Throw an exception with the asset key if a string asset load fails #9325
Conversation
|
We should document in the dartdocs for load() and loadString() what the semantics are for the case where the asset is unavailable. Should they always throw? Always return null? The fix in this PR only throws for the specific case of loadString() from an AssetBundle. It looks like the semantics for NetworkAssetBundle are to return null in the case of a successful load (!) for load(), and return null in the case of a failed load for loadString(); and for CachingAssetBundle, in case of failure load() returns null but loadString() throws. We should make this more consistent. |
|
Changed all load() implementations to throw on a missing asset |
There was a problem hiding this comment.
please add this to loadString's dartdocs as well
There was a problem hiding this comment.
This should probably include information from response as well (like the status code, maybe more? I guess we don't want to include the whole body, that might be absurdly verbose).
See https://docs.flutter.io/flutter/foundation/FlutterError/message.html for some style notes about these error messages.
|
This is great. Please add tests that verify the behaviour for the things for which you changed the behaviour. LGTM. |
|
Added a test for AssetBundle - PTAL |
There was a problem hiding this comment.
nit: braces around multiline statements, and if you put a newline after the (, put a newline before the ).
|
LGTM. Bonus points if you can also add a test for verifying the change to the network asset bundle... we really wouldn't want to regress the ==/!= 200 thing... |
3 participants
Fixes #7715