grpc / grpc-dart

The Dart language implementation of gRPC.
https://pub.dev/packages/grpc
Apache License 2.0
861 stars 271 forks source link

Add onConnectionStateChanged on ClientConnection #563

Closed Ludonope closed 2 years ago

Ludonope commented 2 years ago

This PR adds a callback on ClientConnection to get updates when the ConnectionState changes.

Usage:

final conn = await clientChannel.getConnection();
conn.onConnectionStateChanged = (ConnectionState prevState, ConnectionState state) {
  // ... do whatever.
}

This is a first solution for issue #428. It's not perfect yet and could be improved upon, I'm open to critics/suggestions.

👀 Why is it important?

A modern app (particularly GUI) should be able to provide feedback to user related to the state of the connection. We don't want the user to wait until they interact to tell them that "oh actually the connection is down", we need to tell them as soon as possible.

🛠 Possible improvements

  1. Maybe we could set that callback as a parameter of the ClientChannel ? That way we wouldn't have to force the creation of the connection just to set it.

  2. Is the previous state necessary ? I put it there as I feel like it's important, going from idle to connected is not the same as going from transientFailure to connected. But we could also leave that to the user to store the previous state 🤔

linux-foundation-easycla[bot] commented 2 years ago

CLA Signed

The committers listed above are authorized under a signed CLA.

Cobinja commented 2 years ago

If this is merged then do not merge #565.

mraleph commented 2 years ago

Thanks for contribution @Ludonope!

I think stream based API proposed by @Cobinja is more in line with Dart style (it also allows for multiple listeners out of the box, etc). Let's try to land #565 instead of this one.

Ludonope commented 2 years ago

Yeah makes more sense with stream!