Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: emitting a state in a closed Cubit throws an error #4165

Open
ra48ad opened this issue May 7, 2024 · 9 comments
Open

fix: emitting a state in a closed Cubit throws an error #4165

ra48ad opened this issue May 7, 2024 · 9 comments
Labels
discussion Open discussion for a specific topic enhancement candidate Candidate for enhancement but additional research is needed feedback wanted Looking for feedback from the community pkg:bloc This issue is related to the bloc package

Comments

@ra48ad
Copy link

ra48ad commented May 7, 2024

Description
Trying to emit a state when the cubit is closed (can happen for example in async functions) leads to an Error being thrown. This behaviour may be intentional, but it is - however - different from a Bloc's behaviour the emit is called.

Steps To Reproduce

  1. Create a new Cubit
  2. Add an async function that waits for 5 seconds, then emits a new state.
  3. Create a new instance of this Cubit in your main method and call the function in 2) and then close the Cubit.
  4. You should get an Error thrown that says "Cannot emit new states after calling close"

Expected Behavior
Calling emit on a closed Cubit ignores the new state and throws no error (Just like a Bloc would)

Screenshots

Additional Context

@ra48ad ra48ad added the bug Something isn't working label May 7, 2024
@felangel
Copy link
Owner

Hi @ra48ad 👋
Thanks for opening an issue!

Are you able to share a link to a minimal reproduction sample illustrating the problem/inconsistency you're seeing? It'd be much easier to help if you could share a repro, thanks!

@felangel felangel added question Further information is requested waiting for response Waiting for follow up needs repro info The issue is missing a reproduction sample and/or steps and removed bug Something isn't working labels May 10, 2024
@ra48ad
Copy link
Author

ra48ad commented May 13, 2024

Hi @ra48ad 👋 Thanks for opening an issue!

Are you able to share a link to a minimal reproduction sample illustrating the problem/inconsistency you're seeing? It'd be much easier to help if you could share a repro, thanks!

Thank you for your reply!

Sure, here are two comparable samples for a Cubit and a Bloc:

Cubit

class CounterCubit extends Cubit<int> {
  CounterCubit() : super(0);

  // delay emit to simulate async operation
  void increment() => Future.delayed(
        Duration(seconds: 3),
        () => emit(state + 1),
      );
}

void main(List<String> args) {
  final cubit = CounterCubit();
  cubit.increment();
  cubit.close();
  // prints 0, exception [StateError] thrown (bloc_base.dart line 97)
  print(cubit.state);
}

Bloc

enum CounterEvent { increment }

class CounterBloc extends Bloc<CounterEvent, int> {
  CounterBloc() : super(0) {
    on<CounterEvent>(_onCounterEvent);
  }

  Future<void> _onCounterEvent(CounterEvent event, Emitter<int> emit) async {
    if (event == CounterEvent.increment) {
      // delay emit to simulate async operation
      return Future.delayed(
        Duration(seconds: 3),
        () => emit(state + 1),
      );
    }
  }
}

void main(List<String> args) {
  final bloc = CounterBloc();
  bloc.add(CounterEvent.increment);
  bloc.close();
  // prints 0, no exception thrown because of isClosed check (bloc.dart line 202)
  print(bloc.state);
}

As you can see, there is a (small) difference in behaviour of both, even though the code is very similar.
In a real world app, it could be sometimes easier to use a Cubit instead of a Bloc, but this behaviour leads to writing extra isClosed checks in the Cubit to prevent the exception.

What do you think? is there anyway to prevent the exception without adding extra checks in the Cubit?

@ra48ad ra48ad changed the title fix: adding a state to a Cubit throws an error fix: adding a state to a closed Cubit throws an error May 13, 2024
@ra48ad ra48ad changed the title fix: adding a state to a closed Cubit throws an error fix: emitting a state in a closed Cubit throws an error May 13, 2024
@bambinoua
Copy link

bambinoua commented May 14, 2024

Why is that throw StateError in bloc_base.dart line 97 needed? As for me the isClosed should be handled as in Bloc (bloc.dart line 202) and it is not interesting to know either stream closed or no. We basically know that if the bloc is initialized then its stream is active, if bloc is closed (due to widget dispose) then any new events should be just discarded and I don't want to see error.

For example, I have a case where I downlooad the image and get its size. It is a long operation which I can break. Now I get a StateError because of widget is already disposed but emit is still trying to send new state. To avoid this I need to add additional check on isClosed in my code. But actually, I repeat, I am not interesting when and why the bloc was closed. I know exactly that it will be closed on widget dispose.

@felangel
Copy link
Owner

This behavior was introduced to be consistent with how StreamController works in Dart. I'm open to adjusting the behavior and welcome all feedback from the community 👍

@felangel felangel added enhancement candidate Candidate for enhancement but additional research is needed feedback wanted Looking for feedback from the community pkg:bloc This issue is related to the bloc package discussion Open discussion for a specific topic and removed needs repro info The issue is missing a reproduction sample and/or steps question Further information is requested waiting for response Waiting for follow up labels May 16, 2024
@ra48ad
Copy link
Author

ra48ad commented May 17, 2024

Thank you for your kind reply! @felangel

I understand this point of view and it makes sense. however, since I started using Cubits instead of Blocs in big parts of my apps, this behaviour have been a pain for me and my team. I am glad you are considering a change.

Would it be ok if I make a Pull Request with the change for you to review it?
My suggestion would be to simply return if the cubit is closed (in BlocBase), just like Bloc. What do you think?

@karvulf
Copy link

karvulf commented Jul 26, 2024

I faced also some issues about that behavior because the bloc is still running even though it is closed which causes issues in other parts of my app.
My idea would be to set a mode for the bloc so that the old behavior won't be overwritten but it would be also possible to say that after closing the bloc, all running events will be canceled.
Or there is a method (cancelRunningEvents) which allows to cancel all running events, this would also solve my case but it is not a solution which works automatically.

What do you think about this? @ra48ad

@ra48ad
Copy link
Author

ra48ad commented Aug 7, 2024

Thank you for your suggestion @karvulf
I actually think this is a very good way to implement it without breaking anything since bloc is widely used.

Curious to know what @felangel thinks about this.

@xoliq0v
Copy link

xoliq0v commented Nov 17, 2024

Hi! 👋

I see the issue you’re pointing out. The current behavior indeed results in an error when attempting to emit a new state from a closed Cubit, especially in asynchronous functions. This behavior might be different from Bloc, where calling emit on a closed instance simply ignores the state change without throwing an error.

Steps to Address This:
1. Consistency Check: If consistency between Bloc and Cubit is the goal, the Cubit’s behavior could be modified to ignore any emit calls made after it’s closed, similar to Bloc.
2. Documentation Update: If the current behavior is intentional, we might consider adding documentation to clarify that Cubit will throw an error if an attempt to emit is made after closure, which would help avoid confusion.

I’d be happy to help with an update or workaround if needed. Let me know if there are any specific directions to explore!

Thanks!

@karvulf
Copy link

karvulf commented Nov 17, 2024

What still could happen if only the emitted states are ignored that the rest of the method that handles the event, would still call functions that could result into an error. It would be nice if after the close all events are stopped or canceled. In this case also the emit would be stopped but I am not sure if Dart allows to cancel asynchronous functions @xoliq0v

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Open discussion for a specific topic enhancement candidate Candidate for enhancement but additional research is needed feedback wanted Looking for feedback from the community pkg:bloc This issue is related to the bloc package
Projects
None yet
Development

No branches or pull requests

5 participants