From b25ee94eeed81a60dd93aa116f6f556ea8308e56 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Thu, 9 Jul 2026 20:33:03 -0700 Subject: [PATCH 1/2] [listen] improve onError API --- packages/listen/CHANGELOG.md | 5 ++ packages/listen/lib/src/listen.dart | 40 ++++++--- packages/listen/pubspec.yaml | 2 +- packages/listen/test/listen_test.dart | 119 +++++++++++++++++++++----- 4 files changed, 132 insertions(+), 34 deletions(-) diff --git a/packages/listen/CHANGELOG.md b/packages/listen/CHANGELOG.md index 72c1578..7ca068c 100644 --- a/packages/listen/CHANGELOG.md +++ b/packages/listen/CHANGELOG.md @@ -1,3 +1,8 @@ +## 1.0.0-beta.3 + +- Updates `ErrorCallback` to pass the raw error object and original `StackTrace`. +- Adds an optional `ErrorContext` parameter to `ErrorCallback` to distinguish listener errors from assertion errors. + ## 1.0.0-beta.2 - Fixes dart doc example directive. diff --git a/packages/listen/lib/src/listen.dart b/packages/listen/lib/src/listen.dart index eba6226..9ffd9cc 100644 --- a/packages/listen/lib/src/listen.dart +++ b/packages/listen/lib/src/listen.dart @@ -10,8 +10,21 @@ import 'package:meta/meta.dart'; /// Signature of callbacks that have no arguments and return no data. typedef VoidCallback = void Function(); -/// Signature of callbacks that are called when an error is thrown by a listener. -typedef ErrorCallback = void Function(String message, StackTrace? stackTrace); +/// Identifies the context in which an error was reported to [Listenable.onError]. +enum ErrorContext { + /// The error was thrown by a listener callback during [Listenable] notification. + listener, + + /// The error was reported by a debug assertion or lifecycle check (e.g. using a disposed notifier). + assertion, +} + +/// Signature of callbacks that are called when an error is reported by a listener or assertion. +typedef ErrorCallback = void Function( + Object error, + StackTrace? stackTrace, { + ErrorContext? context, +}); /// Signature of callbacks that are called when an object is created. /// @@ -58,11 +71,15 @@ abstract class Listenable { /// {@example /example/lib/listenable_merge.dart} factory Listenable.merge(Iterable listenables) = _MergingListenable; - /// Error callback that is called when an error is thrown by a listener. + /// Error callback that is called when an error is thrown by a listener or assertion. /// - /// By default, errors are thrown as [StateError]. - static ErrorCallback onError = (String message, StackTrace? stackTrace) { - throw StateError(message); + /// By default, errors are rethrown preserving their original [StackTrace]. + static ErrorCallback onError = ( + Object error, + StackTrace? stackTrace, { + ErrorContext? context, + }) { + Error.throwWithStackTrace(error, stackTrace ?? StackTrace.current); }; /// Called when a new object is created. @@ -171,10 +188,13 @@ mixin class ChangeNotifier implements Listenable { assert(() { if (notifier._debugDisposed) { Listenable.onError( - 'A ${notifier.runtimeType} was used after being disposed.\n' - 'Once you have called dispose() on a ${notifier.runtimeType}, it ' - 'can no longer be used.', + StateError( + 'A ${notifier.runtimeType} was used after being disposed.\n' + 'Once you have called dispose() on a ${notifier.runtimeType}, it ' + 'can no longer be used.', + ), StackTrace.current, + context: ErrorContext.assertion, ); } return true; @@ -424,7 +444,7 @@ mixin class ChangeNotifier implements Listenable { try { _listeners[i]?.call(); } catch (exception, stack) { - Listenable.onError(exception.toString(), stack); + Listenable.onError(exception, stack, context: ErrorContext.listener); } } diff --git a/packages/listen/pubspec.yaml b/packages/listen/pubspec.yaml index e5b51f7..b2f8460 100644 --- a/packages/listen/pubspec.yaml +++ b/packages/listen/pubspec.yaml @@ -5,7 +5,7 @@ name: listen description: A package to notify state changes to interested listeners in pure Dart. repository: https://github.com/flutter/core-packages/tree/main/packages/listen issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+listen%22 -version: 1.0.0-beta.2 +version: 1.0.0-beta.3 environment: sdk: ^3.10.0 diff --git a/packages/listen/test/listen_test.dart b/packages/listen/test/listen_test.dart index 8acc632..4f8ca0f 100644 --- a/packages/listen/test/listen_test.dart +++ b/packages/listen/test/listen_test.dart @@ -54,18 +54,21 @@ class Counter with ChangeNotifier { void main() { final ErrorCallback originalOnError = Listenable.onError; - String? lastErrorMessage; + Object? lastError; + ErrorContext? lastContext; setUp(() { - lastErrorMessage = null; - Listenable.onError = (String message, StackTrace? stackTrace) { - lastErrorMessage = message; + lastError = null; + lastContext = null; + Listenable.onError = (Object error, StackTrace? stackTrace, {ErrorContext? context}) { + lastError = error; + lastContext = context; }; }); tearDown(() { Listenable.onError = originalOnError; - if (lastErrorMessage != null) { - throw StateError('Unexpected error in test: $lastErrorMessage'); + if (lastError != null) { + throw StateError('Unexpected error in test: $lastError'); } }); @@ -81,11 +84,13 @@ void main() { test.notify(); - expect(lastErrorMessage, contains('dispose()')); + expect(lastError, isA()); + expect(lastContext, ErrorContext.listener); + expect(lastError.toString(), contains('dispose()')); // Make sure it crashes during dispose call. expect(callbackDidFinish, isFalse); test.dispose(); - lastErrorMessage = null; + lastError = null; }); test('ChangeNotifier', () { @@ -110,9 +115,9 @@ void main() { final test = TestNotifier(); final ErrorCallback original = Listenable.onError; - final List errorMessages = []; - Listenable.onError = (String message, StackTrace? stackTrace) { - errorMessages.add(message); + final List errors = []; + Listenable.onError = (Object error, StackTrace? stackTrace, {ErrorContext? context}) { + errors.add(error); }; addTearDown(() { Listenable.onError = original; @@ -167,7 +172,7 @@ void main() { test.addListener(badListener); test.notify(); expect(log, ['listener', 'listener2', 'listener1', 'badListener']); - expect(errorMessages.removeAt(0), contains('Invalid argument')); + expect(errors.removeAt(0), isA()); log.clear(); test.addListener(listener1); @@ -177,11 +182,79 @@ void main() { test.addListener(listener2); test.notify(); expect(log, ['badListener', 'listener1', 'listener2']); - expect(errorMessages.removeAt(0), contains('Invalid argument')); + expect(errors.removeAt(0), isA()); log.clear(); test.dispose(); }); + test('Listenable.onError preserves runtime exception types', () { + final List errors = []; + final ErrorCallback original = Listenable.onError; + Listenable.onError = (Object error, StackTrace? stackTrace, {ErrorContext? context}) { + errors.add(error); + }; + addTearDown(() { + Listenable.onError = original; + }); + + final notifier = TestNotifier(); + notifier.addListener(() { + throw const FormatException('custom exception'); + }); + notifier.notify(); + + expect(errors.single, isA()); + expect((errors.single as FormatException).message, 'custom exception'); + }); + + test('Listenable.onError default implementation rethrows and preserves StackTrace', () { + final stack = StackTrace.fromString('test_stack_trace_marker'); + try { + originalOnError(const FormatException('bad format'), stack); + fail('Expected originalOnError to throw'); + } catch (e, s) { + expect(e, isA()); + expect(s.toString(), 'test_stack_trace_marker'); + } + }); + + test('Listenable.onError reports correct ErrorContext across all failure types', () { + final List contexts = []; + final ErrorCallback original = Listenable.onError; + Listenable.onError = (Object error, StackTrace? stackTrace, {ErrorContext? context}) { + contexts.add(context); + }; + addTearDown(() { + Listenable.onError = original; + }); + + final notifier = TestNotifier(); + notifier.addListener(() { + throw Exception('listener exception'); + }); + notifier.notify(); + expect(contexts.single, ErrorContext.listener); + contexts.clear(); + + notifier.dispose(); + + notifier.dispose(); + expect(contexts.single, ErrorContext.assertion); + contexts.clear(); + + notifier.notify(); + expect(contexts.single, ErrorContext.assertion); + contexts.clear(); + + notifier.addListener(() {}); + expect(contexts.single, ErrorContext.assertion); + contexts.clear(); + + ChangeNotifier.debugAssertNotDisposed(notifier); + expect(contexts.single, ErrorContext.assertion); + contexts.clear(); + }); + test('ChangeNotifier with mutating listener', () { final test = TestNotifier(); final log = []; @@ -388,16 +461,16 @@ void main() { source.dispose(); source.addListener(() {}); - expect(lastErrorMessage, contains('TestNotifier was used after being disposed.')); - lastErrorMessage = null; + expect(lastError.toString(), contains('TestNotifier was used after being disposed.')); + lastError = null; source.dispose(); - expect(lastErrorMessage, contains('TestNotifier was used after being disposed.')); - lastErrorMessage = null; + expect(lastError.toString(), contains('TestNotifier was used after being disposed.')); + lastError = null; source.notify(); - expect(lastErrorMessage, contains('TestNotifier was used after being disposed.')); - lastErrorMessage = null; + expect(lastError.toString(), contains('TestNotifier was used after being disposed.')); + lastError = null; }); test('Can remove listener on a disposed ChangeNotifier', () { @@ -517,8 +590,8 @@ void main() { testNotifier.dispose(); - expect(lastErrorMessage, contains('TestNotifier was used after being disposed.')); - lastErrorMessage = null; + expect(lastError.toString(), contains('TestNotifier was used after being disposed.')); + lastError = null; }); test('Calling debugAssertNotDisposed works as intended', () { @@ -528,8 +601,8 @@ void main() { ChangeNotifier.debugAssertNotDisposed(testNotifier); - expect(lastErrorMessage, contains('TestNotifier was used after being disposed.')); - lastErrorMessage = null; + expect(lastError.toString(), contains('TestNotifier was used after being disposed.')); + lastError = null; }); test('notifyListener can be called recursively', () { From c0530b845657120d044bd19cab338452103ecdb2 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Thu, 9 Jul 2026 20:43:26 -0700 Subject: [PATCH 2/2] foramt --- packages/listen/lib/src/listen.dart | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/listen/lib/src/listen.dart b/packages/listen/lib/src/listen.dart index 9ffd9cc..825296b 100644 --- a/packages/listen/lib/src/listen.dart +++ b/packages/listen/lib/src/listen.dart @@ -20,11 +20,8 @@ enum ErrorContext { } /// Signature of callbacks that are called when an error is reported by a listener or assertion. -typedef ErrorCallback = void Function( - Object error, - StackTrace? stackTrace, { - ErrorContext? context, -}); +typedef ErrorCallback = + void Function(Object error, StackTrace? stackTrace, {ErrorContext? context}); /// Signature of callbacks that are called when an object is created. /// @@ -74,11 +71,7 @@ abstract class Listenable { /// Error callback that is called when an error is thrown by a listener or assertion. /// /// By default, errors are rethrown preserving their original [StackTrace]. - static ErrorCallback onError = ( - Object error, - StackTrace? stackTrace, { - ErrorContext? context, - }) { + static ErrorCallback onError = (Object error, StackTrace? stackTrace, {ErrorContext? context}) { Error.throwWithStackTrace(error, stackTrace ?? StackTrace.current); };