Proposal: renaming and consolidation of the connection handlers

[alovak]

Background:

In an effort to simplify our interface and offer a more intuitive experience to developers, we are proposing a renaming and consolidation of our connection handlers.

Proposed Changes:

In addition to the existing OnConnect and OnClose (sync) handlers, we propose the following modifications:

1. Rename ConnectionEstablishedHandler to OnConnected.
2. Rename ConnectionClosedHandler to OnClosed.
3. Invoke all OnClosed handlers regardless of how the connection was closed (by us, by the server, or due to a read/write error).

Motivation:

• Consistent Naming: The new naming convention is both intuitive and in line with common practices, providing a more streamlined experience for developers.

• Unified Closure Handling with OnClosed:

  • Historical Context: Originally, the assumption was that OnClosed handlers would only be triggered when a connection was closed by the server or due to read/write errors. This allowed connection pools to handle unexpected disconnects and initiate reconnections.
  • Shortcoming: This approach had a limitation. There are scenarios where a connection might be deliberately closed outside the pool (by us) – for instance, when an unhealthy connection is detected and a reconnection is desired. In the current implementation, if we invoke connection.Close, the handlers set up by the connection pool are not executed and it wouldn't initiate a reconnection.
  • Solution: By ensuring OnClosed handlers are triggered regardless of the cause of closure, we address this oversight. This provides more consistent behavior, ensuring that any necessary cleanup or reconnection logic is always executed. It makes the connection's lifecycle more predictable, offering more control and flexibility to developers.

This revision aims to eliminate unexpected behaviors and provide a more consistent and reliable connection management experience.

Changes, except 3rd point, will be backward compatible, we will support old names for some time.

Changed Callbacks

OnConnect (sync):

• Triggered when the connection process is initiated.
• Synchronous in nature. If this handler faces an error, the Connect method will return that error, preventing the connection from being established.
• Provides immediate feedback during the onset of connection.

• OnConnected (async):

  • Invoked after a connection has been successfully established.
  • Asynchronous, allowing for post-connection logic without blocking the core connection process.
  • Notifies that the connection is active and ready.

• OnClose (sync):

  • Triggered before initiating the disconnection process.
  • Synchronous in nature. If an error arises in this handler, the Close method will return that error, and the connection will not be closed.
  • Prepares the system for disconnection, allowing for necessary cleanup or final message sending such as sign-off.

• OnClosed (async):

  • Invoked after the connection is successfully closed.
  • Asynchronous, facilitating post-closure logic.
  • Confirms that the connection has ended and its resources are released.

[adamdecaf]

Would you pass a cause to each handler so the receiver could ignore certain conditions? (e.g. ignore when the Pool closes a connection?)

[alovak]

I'm not sure @adamdecaf. From what I see from the production code, there is no need for such. But I don't also know how to do it and maybe how to use it later (the use case is not clear to me). Can you show me an example?

Is it something like

select {
    case <-ctx.Done():
      log.WithError(context.Cause(ctx)).Warn("canceled")
      return err

[adamdecaf]

I didn't mean context.Cause, but an enum referencing where the close came from.