Skip to content
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

Bugfix: Change how errors are reported by ImageLoader. Emits as stream instead of re-throwing #937

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

robert-northmind
Copy link

@robert-northmind robert-northmind commented Apr 10, 2024

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

This is a bug fix

⤵️ What is the current behavior?

If the CachedNetworkImageProvider gets some error when fetching an image url, then internally the ImageLoader will rethrow that error.
This leads to the unwanted behavior that you get an uncaught exception.
And this for example then leads to faulty fatal errors being reported in Crashlytics.

🆕 What is the new behavior (if this is a feature change)?

Instead of throwing the error inside the ImageLoader, we are now emitting the error as a stream error into the current stream.
This has the added benefit that this will not be an uncaught exception.
And all existing ErrorListeners, ErrorBuilders will still work.

💥 Does this PR introduce a breaking change?

No

🐛 Recommendations for testing

You can test this change with the existing example app.
If you run the example app for a mobile device and enabled the debugger to catch uncaught exceptions, then with this fix you will not get any uncaught exceptions. And before this fix, you would get uncaught exceptions for the https://notAvalid.uri and the not a uri at all.

📝 Links to relevant issues/docs

N/A

🤔 Checklist before submitting

  • All projects build
  • Follows style guide lines (code style guide)
  • Relevant documentation was updated
  • Rebased onto current develop

Screenshots

These screenshots come from VSCode running the previous version with uncaught exceptions turned on:
Screenshot 2024-04-10 at 15 16 51
Screenshot 2024-04-10 at 15 16 57

And here is a screenshot coming from this version with uncaught exceptions turned on:
Screenshot 2024-04-10 at 15 16 14

@robert-northmind robert-northmind changed the title Change how errors are reported by ImageLoader. Emits as stream instea… Change how errors are reported by ImageLoader. Emits as stream instead of re-throwing Apr 10, 2024
@robert-northmind robert-northmind changed the title Change how errors are reported by ImageLoader. Emits as stream instead of re-throwing Bugfix: Change how errors are reported by ImageLoader. Emits as stream instead of re-throwing Apr 10, 2024
@@ -141,7 +141,7 @@ class BasicContent extends StatelessWidget {
aspectRatio: 1.6,
child: BlurHash(hash: 'LEHV6nWB2yk8pyo0adR*.7kCMdnj'),
),
imageUrl: 'https://blurha.sh/assets/images/img1.jpg',
imageUrl: 'https://blurha.sh/12c2aca29ea896a628be.jpg',
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just updated this URL since the old https://blurha.sh/assets/images/img1.jpg didn't point to a resource anymore. You just got back a 404.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant