context.DeadlineExceeded is ambiguous.

[castaneai]

Hi, rueidis is working great with my product, thanks!

I found that Client.Do sometimes returns context.DeadlineExceeded when there is a bad TCP connection. But the first argument ctx is not cancelled.
90a405b hides os.ErrDeadlineExceeded by context.DeadlineExceeded, which obscures context.DeadlineExceeded and confuses us.
The i/o timeout has nothing to do with context.Context. Could you please consider reverting to the previous behavior?

[rueian]

Hi @castaneai, good to know rueidis works for you.

Yes, we can revert it. We did that because, at that time, the two errors were equivalent: The only conn.SetDeadline was when you had a deadline in your context. So you would only get io timeout when your context deadline was exceeded.

However, later, we added backgroundPing which will abort all pending and ongoing requests with context.DeadlineExceeded and drop the connection if it fails to respond in time (10s by the default ConnWriteTimeout). It indeed led to confusion.

While we are working on the fix #672 to revert errors back to io timeouts, it is still worth revisiting your ConnWriteTimeout or if you have severe head of line blocking on a connection since they will result in context.DeadlineExceeded errors in the current version.

[rueian]

You can try the fix early with the new alpha version: v1.0.50-alpha.5

[castaneai]

@rueian Thanks for the quick fix!

[castaneai]

We reproduced a bad TCP connection using the docker network disconnect command.
fix is working fine, thanks @rueian !

	copt := rueidis.ClientOption{
		InitAddress:      []string{"redis:6379"},
		DisableCache:     true,
		ConnWriteTimeout: 3 * time.Second,
	}
	rc, err := rueidis.NewClient(copt)
	if err != nil {
		log.Fatalf("failed to connect to redis: %+v", err)
	}

	ctx := context.Background()
	ticker := time.NewTicker(1000 * time.Millisecond)
	defer ticker.Stop()
	for range ticker.C {
		cmd := rc.B().Set().Key("test").Value("value").Build()
		resp := rc.Do(ctx, cmd)
		if err := resp.NonRedisError(); err != nil {
			log.Printf("%+v", err)
			continue
		}
	}

docker network disconnect <network> <container>

# rueidis v1.0.49
client-1  | 2024/11/15 05:37:17 context deadline exceeded

# rueidis v1.0.50-alpha.5
client-1  | 2024/11/15 05:41:36 i/o timeout

[castaneai]

@rueian Sorry, I overlooked one thing.
If the context deadline is earlier than the ConnWriteTimeout, then context.DeadlineExceeded should be returned. #672 returns os.ErrDeadlineExceeded for all timeouts, but I guess I need to be able to distinguish whether the timeout is context-derived or ConnWriteTimeout-derived.

[rueian]

@castaneai, yes, good catch. So we have v1.0.50-alpha.6 now.

[castaneai]

@rueian Thank you! The distinction between the two timeouts works fine.

# v1.0.50-alpha.6 (context deadline < ConnWriteTimeout)
client-1  | 2024/11/15 07:25:34 context deadline exceeded

# v1.0.50-alpha.6 (context deadline > ConnWriteTimeout)
client-1  | 2024/11/15 07:27:33 i/o timeout