felangel / bloc

A predictable state management library that helps implement the BLoC design pattern
https://bloclibrary.dev
MIT License
11.56k stars 3.37k forks source link

feat(bloc): add support for optional `StackTrace` on error handlers for `Emitter.onEach`/`Emitter.forEach` #4134

Closed narcodico closed 3 months ago

narcodico commented 3 months ago

Description

The error handler signature on Emitter's methods is void/State Function(Object error, StackTrace stackTrace)?. There are situations when using a custom error/exception/failure that could potentially encapsulate the stack, deems the StackTrace parameter useless. To better align with some other APIs like Future.catchError, Completer.completeError and even the core catch signature which accepts an optional StackTrace, I would like to see the StackTrace converted to an optional parameter.

Desired Solution

Make the StackTrace parameter optional and enable using onError in a cleaner way, e.g.:

await emit.onEach(
      _repository.watchData(),
      onData: (data) => add(DataChanged(data)),
      onError: (failure) => add(DataFailed(failure)),
    );
felangel commented 3 months ago

Thanks for opening an issue!

The issue with this approach is the signature of onError will just become Function (completely untyped). I understand it's consistent with existing APIs like Future.catchError and Completer.completeError but those APIs are likely the way they are because they pre-date a lot of modern Dart language features so I'm a bit torn about making this change since in some ways it feels like a regression in terms of API from a type-safety standpoint.

narcodico commented 3 months ago

I was exploring the possibility of simplifying the signature, but you're right. The nearest supertype of both void Function(Object) and void Function(Object, StackTrace) is Function, but you end up loosing type-safety.