rrousselGit / state_notifier

ValueNotifier, but outside Flutter and with some extra perks
MIT License
311 stars 28 forks source link

Throw error of try/catch instead of a Error() when set state #51

Closed tbm98 closed 3 years ago

codecov[bot] commented 3 years ago

Codecov Report

Merging #51 (00db549) into master (4822bc5) will increase coverage by 4.04%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #51      +/-   ##
==========================================
+ Coverage   95.04%   99.08%   +4.04%     
==========================================
  Files           5        2       -3     
  Lines         464      328     -136     
==========================================
- Hits          441      325     -116     
+ Misses         23        3      -20     
Impacted Files Coverage Δ
packages/state_notifier/lib/state_notifier.dart 96.38% <ø> (-0.05%) :arrow_down:
...kages/state_notifier/test/state_notifier_test.dart 100.00% <100.00%> (ø)
example/lib/my_state_notifier.dart
...ter_state_notifier/lib/flutter_state_notifier.dart
example/lib/main.dart
rrousselGit commented 3 years ago

Hello!

What is the use-case? I'm all for changing Error to something more specific. But I don't think throwing the original exception is a good call (on top of that, there can be more than one)

That would mean the exception thrown by state = would changed based on the number of listeners and what they do. I don't like the idea of coupling those logics.

tbm98 commented 3 years ago

I think throw original exceptions is helpful to trace/ identify problems more quickly. Currently, it only throws Error() without information and developers don't know why it happens.

tbm98 commented 3 years ago

instance is this error: river_pod/issues/730

it only show

======== Exception caught by gesture ===============================================================
The following Error was thrown while handling a gesture:
Instance of 'Error'
rrousselGit commented 3 years ago

But if it's about tracing, we can improve tracing without necessarily rethrowing the exception.

We could for example do:

List<Object> errors;
List<StackTrace> stackTraces;

if (didThrow) {
  throw NotifyListenerError(errors, stackTraces);
}

(with a toString override to print the errors)

This achieves the same effect, but without making the behaviour of set = changed based on the number of listeners.

tbm98 commented 3 years ago

Oh. I didn't know that can has multiple exception