Skip to content

Add reportErrors to ImageStreamListener#180327

Merged
auto-submit[bot] merged 18 commits into
flutter:masterfrom
koji-1009:fix/image_exception
Apr 29, 2026
Merged

Add reportErrors to ImageStreamListener#180327
auto-submit[bot] merged 18 commits into
flutter:masterfrom
koji-1009:fix/image_exception

Conversation

@koji-1009

@koji-1009 koji-1009 commented Dec 27, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

fix #81931

Run the following app and quickly tap the FAB multiple times to output an exception to the console. However, N/A continues to be displayed on the UI, and errorBuilder is functioning correctly.

import 'package:flutter/material.dart';

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(home: MyHomePage());
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key});

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  var counter = 0;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: Text('Flutter Demo Home Page')),
      body: Center(
        child: Column(
          spacing: 16,
          mainAxisSize: MainAxisSize.min,
          children: [
            MyImage(
              url: switch (counter % 3) {
                0 => 'AAABBBCCC',
                1 => 'https://example.com/nothing',
                2 => 'https://google.com',
                _ => 'AAABBBCCC',
              },
            ),
          ],
        ),
      ),
      floatingActionButton: FloatingActionButton(
        onPressed: () {
          setState(() {
            counter++;
          });
        },
        child: Text(counter.toString()),
      ),
    );
  }
}

class MyImage extends StatelessWidget {
  const MyImage({super.key, required this.url});

  final String url;

  @override
  Widget build(BuildContext context) {
    return Image.network(
      url,
      errorBuilder: (context, exception, stackTrace) {
        return Text('N/A');
      },
    );
  }
}

Root Cause

ImageStreamCompleter outlives the Image widget because ImageCache holds keepAlive handles that prevent disposal even after the widget is gone. When the widget is disposed or rebuilt with a different URL, its listener is removed, but the completer continues to live. Errors arriving after that point have no onError handler and fall through to FlutterError.reportError.

The completer has no memory of whether an error handler was ever present, so it cannot distinguish between "no one ever handled errors" and "someone handled errors but is now gone." This is the root cause.

A previous fix (addEphemeralErrorListener in _stopListeningToStream) covers the dispose path, but not the rebuild path where _updateSourceStream removes the listener directly. It also has a limitation when a cached image is delivered first (_currentImage != null), causing addEphemeralErrorListener to skip registration and leaving subsequent network errors unhandled. Another fix inferred error-handling intent from onError != null, but in debug mode onError is always set for _debugBuildErrorWidget, making it impossible to distinguish real error handlers from debug-only re-throwers.

Fix

Added reportErrors flag to ImageStreamListener. The Image widget sets this to false only when errorBuilder is provided. ImageStreamCompleter remembers that such a listener was once registered, and skips reporting errors to FlutterError.onError after all listeners are removed.

This separates "error handling intent" (reportErrors) from "error callback presence" (onError != null), so kDebugMode behavior no longer interferes with error reporting logic.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

@github-actions github-actions Bot added the framework flutter/packages/flutter repository. See also f: labels. label Dec 27, 2025
@koji-1009 koji-1009 force-pushed the fix/image_exception branch 2 times, most recently from 56e68e5 to 65a91aa Compare December 27, 2025 05:44
koji-1009
koji-1009 commented Dec 27, 2025
View reviewed changes
Comment thread packages/flutter/test/widgets/image_test.dart
});

testWidgets(
'errorBuilder prevents FlutterError report only if errorBuilder is non-null when widget is disposed',

@koji-1009 koji-1009 Dec 27, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've updated this test to assert that errors are silenced when the widget is disposed.

If the behavior asserted by the original test satisfies the intended design, then issue #81931 should be closed as "working as intended".

@koji-1009 koji-1009 marked this pull request as ready for review December 27, 2025 07:24
@justinmc justinmc self-requested a review January 20, 2026 23:32
justinmc
justinmc reviewed Feb 9, 2026
View reviewed changes

@justinmc justinmc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the clear description of the solution. I don't know the original intention of this code but I suspect your approach is right. I left a few minor comments.

Comment thread packages/flutter/lib/src/painting/image_stream.dart Outdated
Comment on lines 754 to 757

@justinmc justinmc Feb 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These docs are no longer strictly true, could you update them?

@koji-1009 koji-1009 Feb 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I updated the docs.
1e0fe08 be604b0

Comment thread packages/flutter/test/painting/image_provider_and_image_cache_test.dart Outdated
Comment on lines +60 to +66
final listener = ImageStreamListener(
(ImageInfo info, bool syncCall) {},
onError: (Object exception, StackTrace? stackTrace) {
throw exception;
},
);
stream.addListener(listener);

@justinmc justinmc Feb 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess this turns the listener into an "active" listener?

Comment thread packages/flutter/test/painting/image_provider_and_image_cache_test.dart Outdated

expect(imageCache.statusForKey(provider).untracked, true);
expect(imageCache.pendingImageCount, 0);
stream.removeListener(listener);

@justinmc justinmc Feb 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it would be better to do this in an addTeardown just after your call to addListener, to avoid leaks.

@justinmc justinmc Feb 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same for the other tests below.

@koji-1009 koji-1009 Feb 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review. I fixed in 5fe7eeb.

@justinmc justinmc self-requested a review February 10, 2026 23:25
justinmc
justinmc reviewed Feb 26, 2026
View reviewed changes

@justinmc justinmc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry now I'm second guessing myself on this PR, because I worry about swallowing errors that someone might care about. Why is the current behavior of reporting the error with FlutterErrorDetails.silent still too much? I guess it's just showing up in the real world (though debug mode) too frequently during normal development? Would it help if we added to the error message to explain that the situation more?

@koji-1009

koji-1009 commented Feb 26, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback. I understand the concern about swallowing errors.

The silent flag is unfortunately not sufficient in practice. Firebase Crashlytics's official docs recommend FlutterError.onError = FirebaseCrashlytics.instance.recordFlutterFatalError, which treats all FlutterError.onError as fatal, ignoring the silent flag entirely.

This follows naturally from the onError documentation, which suggests reporting all errors to server as a use case. The silent flag only affects the default handler's behavior in release mode, so when developers override onError with a custom handler, silent errors become indistinguishable unless they specifically check for it.

I'd like to propose a more conservative approach. Instead of my current implementation, track whether an onError listener has ever been registered (_hasHadActiveErrorListener):

  • Flag is true + no current listeners → error was previously handled, silence it
  • Flag is false → no one ever handled errors → report to FlutterError.onError as before

This limits the change to Image.network with errorBuilder only. Without errorBuilder, behavior is unchanged.

What do you think? I'll update the PR if this direction looks good.

For context: in one of the projects I'm involved in, this code path is consistently the top error in Crashlytics dashboards despite being handled by errorBuilder, and it dilutes the crash-free rate.

@koji-1009 koji-1009 requested a review from justinmc February 26, 2026 09:20
@justinmc

justinmc commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

I ran your example code and looked at the Image and ImageStreamListener code a bit more. I think that the current behavior makes sense in the context of ImageStreamListener alone. Your change (the original one currently in this branch) blends the concern of Image into ImageStreamListener.

Where do you expect to implement your new idea with _hasHadActiveErrorListener, is that in Image or ImageStreamListener?

What if we dispose the ImageStreamListener when Image is disposed and then ImageStreamListener throws away errors if they come back after it has been disposed? That makes sense to me both from the self-contained perspective of Image and ImageStreamListener.

@koji-1009

koji-1009 commented Feb 27, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the thoughtful feedback. I've reworked the approach based on your concerns.

Regarding swallowing errors:

The previous approach silenced all errors when no active listeners remained, which was too broad. The new approach adds an explicit preventErrorReporting flag to ImageStreamListener. Only listeners that opt in (i.e., when Image.errorBuilder is provided) prevent error reporting after removal. Listeners without this flag still report errors to FlutterError.onError as before. This preserves the default behavior and only suppresses errors where the developer has explicitly declared an error handler.

Regarding the disposal approach:

There is actually an existing mechanism that takes a similar approach: _stopListeningToStream adds an addEphemeralErrorListener to suppress errors after the widget is disposed. This covers the dispose path.

However, when the image URL changes (e.g. rapid user interaction), _updateSourceStream removes the old listener directly without going through _stopListeningToStream, so the ephemeral listener is never added. This is the path that causes the bug in the reproduction code.

preventErrorReporting covers both paths because the flag is recorded when the listener is first added via addListener, regardless of how it is later removed.

ref: #167783

@koji-1009 koji-1009 changed the title Fix ImageStream reporting errors globally when no active listeners exist Add preventErrorReporting to ImageStreamListener Feb 27, 2026
justinmc
justinmc previously approved these changes Apr 9, 2026
View reviewed changes

@justinmc justinmc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with some minor comments. I think this is the best approach I can think of for this problem. Sorry for the delayed review here!

Comment thread packages/flutter/lib/src/painting/image_stream.dart Outdated
Comment on lines +220 to +222
/// If the image stream has no error listeners (listeners with an [onError]
/// callback), the error is silently ignored. This prevents unnecessary
/// error noise when errors occur after the widget has been disposed.

@justinmc justinmc Apr 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this no longer true and should reference preventErrorReporting?

@koji-1009 koji-1009 Apr 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated the doc to reflect the current behavior and reference reportErrors.

09ac5d0

Comment thread packages/flutter/lib/src/painting/image_stream.dart Outdated
///
/// Defaults to false. The [Image] widget sets this to true when an
/// [Image.errorBuilder] is provided.
final bool preventErrorReporting;

@justinmc justinmc Apr 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: Maybe reportErrors per the styleguide? https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md#avoid-double-negatives-in-apis

@justinmc justinmc Apr 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Or is reportErrorsAfterRemoval more descriptive? Though it's longer. Up to you!

@koji-1009 koji-1009 Apr 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I renamed it to reportErrors and updated the docs!

09ac5d0

Comment thread packages/flutter/lib/src/painting/image_stream.dart Outdated
Comment on lines +560 to +563
// Track that an error-preventing listener was once registered.
if (listener.preventErrorReporting) {
_hadErrorListener = true;
}

@justinmc justinmc Apr 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess there's no valid use case for removing a preventErrorReporting listener and wanting errors to be reported? In the case of the Image widget it makes sense as-is. Not sure if anything else might use this in this way.

@koji-1009 koji-1009 Apr 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I also couldn't think of a use case beyond Image. Since reportErrors: false is an opt-in flag, I don't see a scenario where the same completer would later need to opt back in.

@dkwingsmt dkwingsmt requested review from dkwingsmt and justinmc April 9, 2026 23:33
@auto-submit

auto-submit Bot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

auto label is removed for flutter/flutter/180327, Failed to enqueue flutter/flutter/180327 with HTTP 400: Pull request Required status check "Merge Queue Guard" is expected..

@dkwingsmt dkwingsmt added autosubmit Merge PR when tree becomes green via auto submit App CICD Run CI/CD labels Apr 28, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 28, 2026
@auto-submit

auto-submit Bot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/180327, because - The status or check suite Linux analyze has failed. Please fix the issues identified (or deflake) before re-applying this label.

@koji-1009 koji-1009 dismissed stale reviews from dkwingsmt and justinmc via f0b0277 April 28, 2026 21:05
@github-actions github-actions Bot removed the CICD Run CI/CD label Apr 28, 2026
@koji-1009

koji-1009 commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

Sorry, I fixed the code format.

@dkwingsmt dkwingsmt added the CICD Run CI/CD label Apr 28, 2026
@koji-1009 koji-1009 requested review from dkwingsmt and justinmc April 28, 2026 21:16
dkwingsmt
dkwingsmt approved these changes Apr 28, 2026
View reviewed changes

@dkwingsmt dkwingsmt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re LGTM

justinmc
justinmc approved these changes Apr 29, 2026
View reviewed changes

@justinmc justinmc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re LGTM, will autosubmit.

@justinmc justinmc added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 29, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue Apr 29, 2026
Merged via the queue into flutter:master with commit 0254afb Apr 29, 2026
88 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 29, 2026
@koji-1009 koji-1009 deleted the fix/image_exception branch April 30, 2026 04:41
This was referenced Apr 30, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Merged
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request May 1, 2026
@engine-flutter-autoroll
Roll Flutter from 81bc3d69535f to 707dbc0420a3 (85 revisions) (#11630)
a8e3cce 
Roll Flutter from 81bc3d69535f to 707dbc0420a3 (85 revisions)

flutter/flutter@81bc3d6...707dbc0

2026-05-01 [email protected] Removing TODOs from the WebParagraph code and addressing technical debts. (flutter/flutter#185168)
2026-05-01 [email protected] Ensure that vulkan_interface.h gets included before vk_mem_alloc.h (flutter/flutter#185777)
2026-05-01 [email protected] [flutter_tools] Bump dwds dependency to v27.1.1 (flutter/flutter#185357)
2026-05-01 [email protected] Roll Dart SDK from 6d4a319cbdac to 9aa7097f2e3e (3 revisions) (flutter/flutter#185888)
2026-05-01 [email protected] Roll Skia from fa1dcb289709 to 7ac6d42f2fd0 (1 revision) (flutter/flutter#185887)
2026-05-01 [email protected] Roll Skia from 54cc00adde38 to fa1dcb289709 (3 revisions) (flutter/flutter#185880)
2026-05-01 [email protected] [iOS] Migrate to FlutterFMLTaskRunner(s) (flutter/flutter#185815)
2026-05-01 [email protected] Remove material imports from navigator_on_did_remove_page_test and scrollable_in_overlay_test (flutter/flutter#182546)
2026-05-01 [email protected] Roll Skia from 2e279266f06a to 54cc00adde38 (3 revisions) (flutter/flutter#185872)
2026-05-01 [email protected] dev: Remove unused parameters (flutter/flutter#185345)
2026-05-01 [email protected] Roll Fuchsia Linux SDK from HN5VYzftnf_B8T-n9... to VnzuUefDQR0UhQ1L1... (flutter/flutter#185870)
2026-05-01 [email protected] Use g_free when using glib memory allocation (flutter/flutter#185519)
2026-05-01 [email protected] Roll Dart SDK from d30df3428f2e to 6d4a319cbdac (2 revisions) (flutter/flutter#185862)
2026-05-01 [email protected] Remove trivial test utility cross-imports from material and cupertino… (flutter/flutter#184295)
2026-05-01 [email protected] Inline test callback painter in tab scaffold test (flutter/flutter#184851)
2026-05-01 [email protected] [a11y] Add support for high contrast themes in the a11y assessments app  (flutter/flutter#185801)
2026-05-01 [email protected] [a11y assessment app] Use default color for banner (flutter/flutter#185804)
2026-04-30 [email protected] Added name to AUTHORS (flutter/flutter#185586)
2026-04-30 [email protected] Remove semantics_tester import from raw_material_button_test.dart (flutter/flutter#184806)
2026-04-30 [email protected] Remove semantics_tester import from user_accounts_drawer_header_test.dart (flutter/flutter#184809)
2026-04-30 [email protected] Roll Skia from 7b88c5c281e5 to 2e279266f06a (5 revisions) (flutter/flutter#185854)
2026-04-30 [email protected] Handle symmetric rectangular and elliptical round super ellipses in the uber SDF renderer  (flutter/flutter#185695)
2026-04-30 [email protected] Match on process name before killing for SwiftPM (flutter/flutter#185774)
2026-04-30 [email protected] Sync CHANGELOG.md from stable (flutter/flutter#185838)
2026-04-30 [email protected] Roll Dart SDK from 25910e31a6d2 to d30df3428f2e (5 revisions) (flutter/flutter#185839)
2026-04-30 [email protected] Check cross imports test subfolders (flutter/flutter#185493)
2026-04-30 [email protected] test: inline TestCallbackPainter in cupertino/picker_test.dart (flutter/flutter#185398)
2026-04-30 [email protected] Update customer testing version (flutter/flutter#185830)
2026-04-30 [email protected] Adapt the Metal shader library output list for debug versus release mode (flutter/flutter#185798)
2026-04-30 [email protected] [Impeller] Port a recent Vulkan swapchain fence waiting fix to the AHB version of the swapchain (flutter/flutter#185763)
2026-04-30 [email protected] Roll Skia from 26a59aa71eff to 7b88c5c281e5 (1 revision) (flutter/flutter#185821)
2026-04-30 [email protected] Roll Skia from 6b4167b4e204 to 26a59aa71eff (4 revisions) (flutter/flutter#185808)
2026-04-30 [email protected] [a11y] Mark SemanticsNode dirty when customSemanticsActions change  (flutter/flutter#185707)
2026-04-30 [email protected] Roll Skia from 1bd2f1cc2746 to 6b4167b4e204 (8 revisions) (flutter/flutter#185799)
2026-04-30 [email protected] [iOS] Extract FlutterVSyncClient from vsync_waiter_ios (flutter/flutter#185737)
2026-04-30 [email protected] Roll Fuchsia Linux SDK from nnv8-SSam6yE8dw4z... to HN5VYzftnf_B8T-n9... (flutter/flutter#185782)
2026-04-29 [email protected] [iOS] Soften TaskRunner.postTask(delay:task:) delay check (flutter/flutter#185729)
2026-04-29 [email protected] Add reportErrors to ImageStreamListener (flutter/flutter#180327)
2026-04-29 [email protected] Roll Skia from f5c781c083c7 to 1bd2f1cc2746 (5 revisions) (flutter/flutter#185761)
2026-04-29 [email protected] Update merge semantics logic to merge sibling nodes (flutter/flutter#183745)
2026-04-29 [email protected] Roll Packages from ba80f8f to cde5b36 (12 revisions) (flutter/flutter#185748)
2026-04-29 [email protected] examples: Remove unused parameters (flutter/flutter#185346)
2026-04-29 [email protected] Update TickerMode.getValuesNotifier to handle initialization during State.dispose (flutter/flutter#185248)
2026-04-29 [email protected] Update triage links (flutter/flutter#185714)
2026-04-29 [email protected] Add support for high contrast and color inversion on Android (flutter/flutter#182263)
2026-04-29 [email protected] Roll Skia from 05251260fda6 to f5c781c083c7 (2 revisions) (flutter/flutter#185743)
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@justinmc justinmc justinmc approved these changes

@dkwingsmt dkwingsmt dkwingsmt approved these changes

Assignees

No one assigned

Labels

CICD Run CI/CD framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Image.network throws uncaught exception when providing valid url without an image

3 participants

@koji-1009 @justinmc @dkwingsmt