brianegan / flutter_redux

A library that connects Widgets to a Redux Store
MIT License
1.65k stars 219 forks source link

`onWillChange` and `onDidChange` errors are swallowed #226

Closed atoka93 closed 2 years ago

atoka93 commented 2 years ago

Hi Brian! 🙂

Depending on a InheritedWidget in the onWillChange method, which should log an exception, doesn't.

I discovered the issue by depending on an inherited widget, but the following code example also results in the same behavior:

  @override
  void onWillChange(
    final _ViewModel? previousViewModel,
    final _ViewModel viewModel,
  ) {
    throw Exception('error');
  }

The error is thrown here: https://github.com/brianegan/flutter_redux/blob/848cf12b68b2fa97f3e39a173086ad9567b22805/lib/flutter_redux.dart#L569 but I think it is not handled by the stream.

brianegan commented 2 years ago

Heya @atoka93 :) Nice to hear from ya!

Ah, you're right... Good catch. The error is properly handled by the stream: In other words, the error thrown here does in fact reach the onError callback listening to the stream. However, in this case, the StreamBuilder is listening to the Stream, and I'm only paying attention to a specific variable to throw an error. A small change should make this work properly!

atoka93 commented 2 years ago

I'm not that familiar with Dart streams, was never required to work with them extensively and never took the time to familiarize myself with them for the fun of it. That's also the reason I didn't provide a pull request with the issue. 😬 But I could start filling that gap in my knowledge and open a pull request later this week if you'd like

brianegan commented 2 years ago

Yah no worries, this is a pretty ugly combo of Stream + Widget layer, quick fix since I know where it's at. Please try this PR and let me know if it works for ya:

https://github.com/brianegan/flutter_redux/pull/227

brianegan commented 2 years ago

Fixed by #227