feat: add support for idle connection timeouts and automatic reconnection #16

estensen · 2025-03-04T15:23:03Z

⚠️ Abandoning this in favor of #19

Like http.Client we don't set timeouts by default, which can lead to connection leaks and silent disconnections in long-running applications.

This PR allows IdleTimout to be set to configure the timeout duration. Connections that are broken are also automatically reconnected.

To keep things backwards compatible IdleTimout is set to 0 and is disabled by default.

Added test to simluate network and will add it to CI next:

> FIBER_API_KEY="XXX" RUN_RECONNECTION_TEST=true go test -v -run TestReconnection
RUN_RECONNECTION_TEST=true go test -v -run TestReconnection
=== RUN   TestReconnection
    smoke_test.go:31: ========== CONNECTION SETUP ==========
    smoke_test.go:61: Waiting for initial transactions before testing reconnection...
    smoke_test.go:75: Received initial tx 1: 0xbaf0d73d5c424c7a00796938ae6148adf6fa3fde7e80a617776f8c6b98b3cecf
    smoke_test.go:77: Received sufficient initial transactions
    smoke_test.go:86: Current connection state before disconnect: READY
    smoke_test.go:89: Simulating network interruption...
    smoke_test.go:94: Waiting for reconnection...
Stream error, reconnecting: rpc error: code = Canceled desc = grpc: the client connection is closing
Subscription error, retrying in 1.46767385s: rpc error: code = Canceled desc = grpc: the client connection is closing
Subscription error, retrying in 2.24161844s: rpc error: code = Canceled desc = grpc: the client connection is closing
Subscription error, retrying in 2.640983165s: rpc error: code = Canceled desc = grpc: the client connection is closing
    smoke_test.go:119: RECONNECTION DETECTED after 21.863080708s - Transaction received after disconnect
    smoke_test.go:121: Current connection state after reconnect: READY
    smoke_test.go:125: Post-reconnect tx: 0x7c65daf613dc42da4db0252c19e25baaac1a4fee3bd5a6976446224a5d5e63df
    smoke_test.go:125: Post-reconnect tx: 0x8e26e2bd6796dd4953cc11fc1fc11183082f7ccda4bb61ca0f5efec044947e47
    smoke_test.go:125: Post-reconnect tx: 0xf355b293674e8670eeb736c5a271bbf40064cc21fa47d0f5d609a8a4c230658c
    smoke_test.go:130: Successfully confirmed reconnection with 3 transactions
    smoke_test.go:149: SUCCESS: Client reconnected after 21.863080708s
--- PASS: TestReconnection (22.62s)
Stream error, reconnecting: rpc error: code = Canceled desc = grpc: the client connection is closing
PASS
ok      github.com/chainbound/fiber-go  22.869s

mempirate · 2025-03-04T15:41:19Z

@estensen can we double check that this will be compatible with the Fiber grpc server? https://github.com/grpc/grpc-go/blob/master/Documentation/keepalive.md

estensen · 2025-03-05T09:31:02Z

@estensen can we double check that this will be compatible with the Fiber grpc server? https://github.com/grpc/grpc-go/blob/master/Documentation/keepalive.md

To add more context here the concern here is that the server might disconnect clients that send keepalive too often
https://github.com/grpc/proposal/blob/master/A8-client-side-keepalive.md#server-enforcement

I've reviewed the Fiber server and Tonic and can't see that it will disconnect clients because of keepalives.
Will create a small program and smoke-test this before merging.

estensen · 2025-03-05T16:08:51Z

streams.go

+
+			// Use exponential backoff with jitter
+			sleepTime := backoff + time.Duration(rand.Int63n(int64(backoff/2)))
+			fmt.Printf("Subscription error, retrying in %v: %v\n", sleepTime, err)


We should probably implement a logger instead of this. But can we live with it?

Have gotten feedback that people don't like it, so let's rm it for now and do a logger later

mempirate

Nice job, some questions and nits

mempirate · 2025-03-05T16:25:39Z

client.go

+				if err == nil {
+					// Successfully created new connection
+					c.conn = newConn
+					c.client = api.NewAPIClient(newConn)


Will this also reconnect any existing streams that are open? Or should we do that manually?

mempirate · 2025-03-05T16:26:59Z

streams.go

+
+	// Use exponential backoff with jitter
+	sleepTime := *backoff + time.Duration(rand.Int63n(int64(*backoff/2)))
+	fmt.Printf("%s subscription error, retrying in %v: %v\n", subscriptionName, sleepTime, err)


Suggested change

fmt.Printf("%s subscription error, retrying in %v: %v\n", subscriptionName, sleepTime, err)

mempirate · 2025-03-05T16:27:16Z

streams.go

+					break
+				}
+
+				fmt.Printf("Stream error, reconnecting: %v\n", err)


Suggested change

fmt.Printf("Stream error, reconnecting: %v\n", err)

mempirate · 2025-03-05T16:27:25Z

streams.go

-				time.Sleep(time.Second * 2)
-				continue outer
+				if err == io.EOF {
+					fmt.Println("Stream completed")


Suggested change

fmt.Println("Stream completed")

mempirate · 2025-03-05T16:27:37Z

streams.go

-			if err := tx.UnmarshalBinary(proto.RlpTransaction); err != nil {
-				continue outer
+			if err := tx.UnmarshalBinary(msg.RlpTransaction); err != nil {
+				fmt.Printf("Error decoding transaction: %v\n", err)


Suggested change

fmt.Printf("Error decoding transaction: %v\n", err)

mempirate · 2025-03-05T16:28:52Z

streams.go

-				time.Sleep(time.Second * 2)
-				continue outer
+				if err == io.EOF {
+					fmt.Println("Stream completed")


Suggested change

fmt.Println("Stream completed")

mempirate · 2025-03-05T16:28:59Z

streams.go

+					break
+				}
+
+				fmt.Printf("Block stream error, reconnecting: %v\n", err)


Suggested change

fmt.Printf("Block stream error, reconnecting: %v\n", err)

mempirate · 2025-03-05T16:29:08Z

streams.go

+			}
+
+			if decodeErr != nil {
+				fmt.Printf("Failed to decode execution payload: %v\n", decodeErr)


Suggested change

fmt.Printf("Failed to decode execution payload: %v\n", decodeErr)

mempirate · 2025-03-05T16:29:24Z

streams.go

-				time.Sleep(time.Second * 2)
-				continue outer
+				if err == io.EOF {
+					fmt.Println("Stream completed")


Suggested change

fmt.Println("Stream completed")

mempirate · 2025-03-05T16:29:31Z

streams.go

+					break
+				}
+
+				fmt.Printf("Beacon block stream error, reconnecting: %v\n", err)


Suggested change

fmt.Printf("Beacon block stream error, reconnecting: %v\n", err)

chore: add test CI

estensen added 4 commits March 4, 2025 15:48

feat: add support for idle connection timeouts

7aa6ce9

feat: extend multiplexer client to be configurable

7c2b9c3

chore: document custom client config

0d6ed02

chore: document that can't be lower than 10s

87d0b3a

estensen requested a review from mempirate March 4, 2025 15:23

chore: automatically reconnect broken connections

25dfb41

estensen changed the title ~~feat: add support for idle connection timeouts~~ feat: add support for idle connection timeouts and automatic reconnection Mar 5, 2025

chore: simplify custom client example

9433b18

estensen force-pushed the idle-timeout branch from 54b143f to 9433b18 Compare March 5, 2025 15:44

chore: check connection more frequently

82a90f7

estensen commented Mar 5, 2025

View reviewed changes

estensen added 2 commits March 5, 2025 17:13

chore: extract backoff logic

c27b96c

chore: remove unnecessary pointer

d654014

mempirate requested changes Mar 5, 2025

View reviewed changes

estensen added 2 commits March 5, 2025 17:31

chore: add test CI

5194157

chore: run reconnection test in CI

2ef16ce

estensen mentioned this pull request Mar 5, 2025

chore: add test CI #18

Merged

estensen and others added 2 commits March 5, 2025 17:49

chore: add reconnection test

bcbe4cb

Merge pull request #18 from chainbound/test

32a0ea6

chore: add test CI

estensen marked this pull request as draft March 5, 2025 20:24

estensen closed this Mar 31, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for idle connection timeouts and automatic reconnection #16

feat: add support for idle connection timeouts and automatic reconnection #16

estensen commented Mar 4, 2025 •

edited

Loading

mempirate commented Mar 4, 2025

estensen commented Mar 5, 2025

estensen Mar 5, 2025

mempirate Mar 5, 2025

mempirate left a comment

mempirate Mar 5, 2025

mempirate Mar 5, 2025

mempirate Mar 5, 2025

mempirate Mar 5, 2025

mempirate Mar 5, 2025

mempirate Mar 5, 2025

mempirate Mar 5, 2025

mempirate Mar 5, 2025

mempirate Mar 5, 2025

mempirate Mar 5, 2025

feat: add support for idle connection timeouts and automatic reconnection #16

feat: add support for idle connection timeouts and automatic reconnection #16

Conversation

estensen commented Mar 4, 2025 • edited Loading

mempirate commented Mar 4, 2025

estensen commented Mar 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mempirate left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

estensen commented Mar 4, 2025 •

edited

Loading