Skip to content

Commit 4cfba27

Browse files
committed
Refactor net.Error.Temporary
Signed-off-by: noor <[email protected]>
1 parent 09f2719 commit 4cfba27

File tree

4 files changed

+8
-68
lines changed

4 files changed

+8
-68
lines changed

loadbalancer/healthchecker.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,7 @@ func doActiveHealthCheck(rt http.RoundTripper, backend string) state {
215215
resp, err := rt.RoundTrip(req)
216216
if err != nil {
217217
perr, ok := err.(net.Error)
218-
//lint:ignore SA1019 Temporary is deprecated in Go 1.18, but keep it for now (https://github.com/zalando/skipper/issues/1992)
219-
if ok && !perr.Temporary() {
218+
if ok && !perr.Timeout() {
220219
log.Infof("Backend %v connection refused -> mark as dead", backend)
221220
return dead
222221
} else if ok {

proxy/proxy.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -966,8 +966,7 @@ func (p *Proxy) makeBackendRequest(ctx *context, requestContext stdlibcontext.Co
966966
status = http.StatusServiceUnavailable
967967
}
968968
p.tracing.setTag(ctx.proxySpan, HTTPStatusCodeTag, uint16(status))
969-
//lint:ignore SA1019 Temporary is deprecated in Go 1.18, but keep it for now (https://github.com/zalando/skipper/issues/1992)
970-
return nil, &proxyError{err: fmt.Errorf("net.Error during backend roundtrip to %s: timeout=%v temporary='%v': %w", req.URL.Host, nerr.Timeout(), nerr.Temporary(), err), code: status}
969+
return nil, &proxyError{err: fmt.Errorf("net.Error during backend roundtrip to %s: timeout=%v : %w", req.URL.Host, nerr.Timeout(), err), code: status}
971970
}
972971

973972
return nil, &proxyError{err: fmt.Errorf("unexpected error from Go stdlib net/http package during roundtrip: %w", err)}

queuelistener/listener.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,7 @@ func (l *listener) listenExternal() {
230230
c, err = l.externalListener.Accept()
231231
if err != nil {
232232
// based on net/http.Server.Serve():
233-
//lint:ignore SA1019 Temporary is deprecated in Go 1.18, but keep it for now (https://github.com/zalando/skipper/issues/1992)
234-
if nerr, ok := err.(net.Error); ok && nerr.Temporary() {
233+
if nerr, ok := err.(net.Error); ok && nerr.Timeout() {
235234
delay = bounce(delay)
236235
l.options.Log.Errorf(
237236
"queue listener: accept error: %v, retrying in %v",

queuelistener/listener_test.go

+5-62
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"testing"
1111
"time"
1212

13-
"github.com/zalando/skipper/logging/loggingtest"
1413
"github.com/zalando/skipper/metrics/metricstest"
1514
)
1615

@@ -21,22 +20,13 @@ type testConnection struct {
2120

2221
type testListener struct {
2322
sync.Mutex
24-
closed bool
25-
failNextTemporary bool
26-
fail bool
27-
connsBeforeFail int
28-
addr net.Addr
29-
conns chan *testConnection
23+
closed bool
24+
fail bool
25+
connsBeforeFail int
26+
addr net.Addr
27+
conns chan *testConnection
3028
}
3129

32-
type testError struct{}
33-
34-
var errTemporary testError
35-
36-
func (err testError) Error() string { return "test error" }
37-
func (err testError) Timeout() bool { return false }
38-
func (err testError) Temporary() bool { return true }
39-
4030
func (c *testConnection) Read([]byte) (int, error) { return 0, nil }
4131
func (c *testConnection) Write([]byte) (int, error) { return 0, nil }
4232
func (c *testConnection) LocalAddr() net.Addr { return nil }
@@ -59,10 +49,6 @@ func (c *testConnection) isClosed() bool {
5949
}
6050

6151
func (l *testListener) Accept() (net.Conn, error) {
62-
if l.failNextTemporary {
63-
l.failNextTemporary = false
64-
return nil, errTemporary
65-
}
6652

6753
if l.fail {
6854
return nil, errors.New("listener error")
@@ -353,28 +339,6 @@ func TestInterface(t *testing.T) {
353339
}
354340
})
355341

356-
t.Run("wrapped listener returns temporary error, logs and retries", func(t *testing.T) {
357-
log := loggingtest.New()
358-
l, err := listenWith(&testListener{failNextTemporary: true}, Options{
359-
Log: log,
360-
})
361-
362-
if err != nil {
363-
t.Fatal(err)
364-
}
365-
366-
defer l.Close()
367-
conn, err := l.Accept()
368-
if err != nil {
369-
t.Fatal(err)
370-
}
371-
372-
defer conn.Close()
373-
if err := log.WaitFor(errTemporary.Error(), 120*time.Millisecond); err != nil {
374-
t.Error("failed to log temporary error")
375-
}
376-
})
377-
378342
t.Run("wrapped permanently fails, returns queued connections and the error", func(t *testing.T) {
379343
m := &metricstest.MockMetrics{}
380344
l, err := listenWith(&testListener{connsBeforeFail: 3}, Options{
@@ -982,27 +946,6 @@ func TestTeardown(t *testing.T) {
982946
}
983947

984948
func TestMonitoring(t *testing.T) {
985-
t.Run("logs the temporary errors", func(t *testing.T) {
986-
log := loggingtest.New()
987-
l, err := listenWith(&testListener{failNextTemporary: true}, Options{
988-
Log: log,
989-
})
990-
991-
if err != nil {
992-
t.Fatal(err)
993-
}
994-
995-
defer l.Close()
996-
conn, err := l.Accept()
997-
if err != nil {
998-
t.Fatal(err)
999-
}
1000-
1001-
defer conn.Close()
1002-
if err := log.WaitFor(errTemporary.Error(), 120*time.Millisecond); err != nil {
1003-
t.Error("failed to log temporary error")
1004-
}
1005-
})
1006949

1007950
t.Run("updates the gauges for the concurrency and the queue size, measures accept latency", func(t *testing.T) {
1008951
m := &metricstest.MockMetrics{}

0 commit comments

Comments
 (0)