-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
Failure of ImageProvider's error handling #44579
Comments
Hi @slightfoot, in point 4 you talk about the assert, in production it will continue over this assert which results in the following error.
|
same issue multiple times Non-fatal Exception: java.lang.Exception: NoSuchMethodError: The method 'getNextFrame' was called on null. |
I believe the fix for this is to just throw if the file is zero length rather than returning null. That should trigger the expected error handling to kick in. Am I missing something? |
Ah - I fixed part of this in #50242 - unintentionally. But the rest of the fix is to throw instead of returning null. |
Ahhh. We should probably add onError to
|
And in all fairness, the bug was opened against ImageProvider, not the Image widget. The errorBuilder is a perfectly reasonable request, but I hope it's understandable why I closed it for the ImageProvider related fixes. |
@dnfield What is strange, that if I select 'break on uncaught exceptions' in VS code, the debugger breaks on Image errors before it calls the new errorBuilder. |
It probably has something to do with the way the zones are set up and where the error handling occurs. The errors can get thrown in a lot of different ways (sync, async, into the zone wrapping image loading, into the zone above it). This is a real exception though, and if nothing else it's getting handled by the zone handler that propagates it back to error builder. |
Yeah but the error builder catches it or not? |
Image decoding is run in a separate zone - there's a zone error handler set up in image_cache.dart that's catching it. But it's catching it in a way the debugger probably doesn't handle (it's not a regular try..catch block, it's a zone error handler). |
So we can't do anything tere? |
I'm not clear on what the issue is - if you're concerned about breaking on uncaught exceptions and don't want to break here, then you should fix your code up locally so it doesn't throw an exception (e.g. make sure the image is available and valid). If you're concerned that this isn't getting propagated to the error handler, please share a test case and we can try to fix it. If it's something else entirely, I'm just missing it and could use some help :) |
Ok, the first one :-) in that case I would have to call a File.Exists before which isn't ideal either but I get your point |
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of |
@escamoteur and I have been working on an app and a bug in our revealed a whole set of issues within the framework for handling image errors. A bug in our app tried to load a Image file with zero-byte file size which causes the follow chain of events to occur.
Chain of events
ImageProvider.resolve
sets up a uncaught exception handler and callsobtainKey
andload
onFileImage
to load the image codec. The way it handles errors here seems very complicated for what is actually required.FileImage.load
is called and creates aMultiFrameImageStreamCompleter
and returns it with the codec loading asynchronously.FileImage._loadAsync
later returns anull
when it detects the 0 length file.MultiFrameImageStreamCompleter
constructor had setup an error handler for codec loading failures; however becausenull
is reported as a successful response the value is passed toMultiFrameImageStreamCompleter._handleCodecReady
which asserts that it can't be null. Because this function does not return aFuture
the assertion is passed down and caught by our uncaught exception handler rather than by the actual codec failure handler. Secondly this codec failure handler gobbles the error and does not pass it down to theImageStream
?ImageProvider.resolve.handleError
which then further attempts to set the completer on theImageStream
to its own to error completer which further causes an assertion because aImageStream
s completer can only be set once, and it was previously successfully set by the synchronousload
callback;Thoughts
There are several failures in this chain.. all of which should never have occurred. I am happy to address these as bugs, but to be honest it seems as though a rethink of how
ImageProvider
errors are handled is probably in order.It should also be possible to handle these errors gracefully and not silently from within the
Image
widget. I think we've needed aerrorBuilder
on it for a long time, and since we now have an error listener as part of ImageCompleter it should be accessible to the end developer.Steps to Reproduce
Run the example code below in Debug mode of your IDE.
Logs
The text was updated successfully, but these errors were encountered: