From 1a944c547dd9f9fc5c214aa7bd47273467e73ff4 Mon Sep 17 00:00:00 2001 From: Thibault Le Ouay Ducasse Date: Tue, 20 Oct 2026 09:26:45 +0200 Subject: [PATCH] fix pr --- apps/checker/checker/tcp.go | 22 +++++++++++--- apps/checker/checker/tcp_test.go | 35 +++++++++++++++++++--- apps/checker/handlers/dns.go | 18 +++++++++-- apps/checker/handlers/dns_internal_test.go | 24 +++++++++++++++ apps/checker/handlers/tcp.go | 4 +-- apps/checker/pkg/job/tcp_job.go | 2 +- 6 files changed, 92 insertions(+), 13 deletions(-) diff --git a/apps/checker/checker/tcp.go b/apps/checker/checker/tcp.go index d1a36cfef..2b7fc5ca5 100644 --- a/apps/checker/checker/tcp.go +++ b/apps/checker/checker/tcp.go @@ -1,6 +1,8 @@ package checker import ( + "context" + "errors" "fmt" "net" "strings" @@ -31,14 +33,26 @@ type TCPResponse struct { Error uint8 `json:"error,omitempty"` } -func PingTCP(timeout int, url string) (TCPResponseTiming, error) { +// tcpDefaultTimeout is the dial timeout in milliseconds when none is given. +const tcpDefaultTimeout = 45_000 + +// PingTCP dials url and reports how long the connection took. timeoutMs is in +// milliseconds, like every monitor timeout; the dial also stops when ctx ends. +func PingTCP(ctx context.Context, timeoutMs int64, url string) (TCPResponseTiming, error) { + if timeoutMs <= 0 { + timeoutMs = tcpDefaultTimeout + } + + dialer := net.Dialer{Timeout: time.Duration(timeoutMs) * time.Millisecond} + start := time.Now().UTC().UnixMilli() - conn, err := net.DialTimeout("tcp", url, time.Duration(timeout)*time.Second) + conn, err := dialer.DialContext(ctx, "tcp", url) stop := time.Now().UTC().UnixMilli() if err != nil { - if e := err.(*net.OpError).Timeout(); e { - return TCPResponseTiming{}, fmt.Errorf("timeout after %d ms", timeout*1000) + var netErr net.Error + if errors.As(err, &netErr) && netErr.Timeout() { + return TCPResponseTiming{}, fmt.Errorf("timeout after %d ms", timeoutMs) } if strings.Contains(err.Error(), "connection refused") { return TCPResponseTiming{}, fmt.Errorf("connection refused") diff --git a/apps/checker/checker/tcp_test.go b/apps/checker/checker/tcp_test.go index 5f2d135dd..1f607552e 100644 --- a/apps/checker/checker/tcp_test.go +++ b/apps/checker/checker/tcp_test.go @@ -1,7 +1,9 @@ package checker_test import ( + "context" "testing" + "time" "github.com/openstatushq/openstatus/apps/checker/checker" ) @@ -9,7 +11,7 @@ import ( func TestPingTcp(t *testing.T) { type args struct { url string - timeout int + timeout int64 } tests := []struct { name string @@ -17,12 +19,12 @@ func TestPingTcp(t *testing.T) { want checker.TCPResponseTiming wantErr bool }{ - {name: "will failed", args: args{url: "error", timeout: 60}, wantErr: true}, - {name: "will be ok", args: args{url: "openstat.us:443", timeout: 60}, wantErr: false}, + {name: "will failed", args: args{url: "error", timeout: 5000}, wantErr: true}, + {name: "will be ok", args: args{url: "openstat.us:443", timeout: 5000}, wantErr: false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := checker.PingTCP(tt.args.timeout, tt.args.url) + got, err := checker.PingTCP(context.Background(), tt.args.timeout, tt.args.url) if (err != nil) != tt.wantErr { t.Errorf("PingTcp() error = %v, wantErr %v", err, tt.wantErr) return @@ -39,3 +41,28 @@ func TestPingTcp(t *testing.T) { }) } } + +func TestPingTcpTimeoutIsMilliseconds(t *testing.T) { + // 192.0.2.0/24 (TEST-NET-1) is never routed, so the dial hangs until the timeout. + start := time.Now() + _, err := checker.PingTCP(context.Background(), 500, "192.0.2.1:80") + if err == nil { + t.Fatal("PingTCP() expected an error dialing TEST-NET-1") + } + if elapsed := time.Since(start); elapsed > 5*time.Second { + t.Fatalf("PingTCP() took %s, a 500ms timeout was read as seconds", elapsed) + } +} + +func TestPingTcpStopsWhenContextEnds(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + + start := time.Now() + if _, err := checker.PingTCP(ctx, 60_000, "192.0.2.1:80"); err == nil { + t.Fatal("PingTCP() expected an error once the context ended") + } + if elapsed := time.Since(start); elapsed > 5*time.Second { + t.Fatalf("PingTCP() ignored the context, took %s", elapsed) + } +} diff --git a/apps/checker/handlers/dns.go b/apps/checker/handlers/dns.go index 8790190e6..26a06789e 100644 --- a/apps/checker/handlers/dns.go +++ b/apps/checker/handlers/dns.go @@ -1,6 +1,7 @@ package handlers import ( + "context" "encoding/json" "fmt" "net/http" @@ -142,8 +143,10 @@ func (h Handler) DNSHandler(c *gin.Context) { op := func() (*checker.DnsResponse, error) { called++ log.Ctx(ctx).Debug().Msgf("performing dns check for %s (attempt %d/%d)", req.URI, called, retry) + attemptCtx, cancel := dnsAttemptContext(ctx, req.Timeout) + defer cancel() start := time.Now().UTC().UnixMilli() - response, err := checker.Dns(ctx, req.URI) + response, err := checker.Dns(attemptCtx, req.URI) latency = time.Now().UTC().UnixMilli() - start if err != nil { @@ -312,8 +315,10 @@ func (h Handler) DNSHandlerRegion(c *gin.Context) { op := func() (*checker.DnsResponse, error) { called++ log.Ctx(ctx).Debug().Msgf("performing dns check for %s (attempt %d/%d)", req.URI, called, retry) + attemptCtx, cancel := dnsAttemptContext(ctx, req.Timeout) + defer cancel() start := time.Now().UTC().UnixMilli() - response, err := checker.Dns(ctx, req.URI) + response, err := checker.Dns(attemptCtx, req.URI) latency = time.Now().UTC().UnixMilli() - start if err != nil { @@ -399,3 +404,12 @@ func EvaluateDNSAssertions(rawAssertions []json.RawMessage, response *checker.Dn } return true, nil } + +// dnsAttemptContext bounds one lookup attempt by the monitor timeout +// (milliseconds), so a stalled resolver cannot outlive the retry budget. +func dnsAttemptContext(ctx context.Context, timeoutMs int64) (context.Context, context.CancelFunc) { + if timeoutMs <= 0 { + return context.WithCancel(ctx) + } + return context.WithTimeout(ctx, time.Duration(timeoutMs)*time.Millisecond) +} diff --git a/apps/checker/handlers/dns_internal_test.go b/apps/checker/handlers/dns_internal_test.go index 0d3b64c33..6765f35fa 100644 --- a/apps/checker/handlers/dns_internal_test.go +++ b/apps/checker/handlers/dns_internal_test.go @@ -1,9 +1,11 @@ package handlers import ( + "context" "encoding/json" "reflect" "testing" + "time" ) func TestTinybirdEventRecordsIsString(t *testing.T) { @@ -82,3 +84,25 @@ func TestDNSResponseHTTPShapeKeepsObject(t *testing.T) { t.Errorf("HTTP records is not a JSON object: %s", raw["records"]) } } + +func TestDNSAttemptContextAppliesMonitorTimeout(t *testing.T) { + ctx, cancel := dnsAttemptContext(context.Background(), 1500) + defer cancel() + + deadline, ok := ctx.Deadline() + if !ok { + t.Fatal("dnsAttemptContext() set no deadline for a positive timeout") + } + if left := time.Until(deadline); left <= time.Second || left > 1500*time.Millisecond { + t.Fatalf("dnsAttemptContext() deadline in %s, want about 1.5s", left) + } +} + +func TestDNSAttemptContextWithoutTimeout(t *testing.T) { + ctx, cancel := dnsAttemptContext(context.Background(), 0) + defer cancel() + + if _, ok := ctx.Deadline(); ok { + t.Fatal("dnsAttemptContext() set a deadline for a zero timeout") + } +} diff --git a/apps/checker/handlers/tcp.go b/apps/checker/handlers/tcp.go index 9249b581e..ed1a7cfce 100644 --- a/apps/checker/handlers/tcp.go +++ b/apps/checker/handlers/tcp.go @@ -109,7 +109,7 @@ func (h Handler) TCPHandler(c *gin.Context) { } op := func() (struct{}, error) { - res, err := checker.PingTCP(int(req.Timeout), req.URI) + res, err := checker.PingTCP(ctx, req.Timeout, req.URI) if err != nil { return struct{}{}, fmt.Errorf("unable to check tcp %s", err) @@ -298,7 +298,7 @@ func (h Handler) TCPHandlerRegion(c *gin.Context) { op := func() (struct{}, error) { called++ timestamp := time.Now().UTC().UnixMilli() - res, err := checker.PingTCP(int(req.Timeout), req.URI) + res, err := checker.PingTCP(ctx, req.Timeout, req.URI) if err != nil { return struct{}{}, fmt.Errorf("unable to check tcp %s", err) diff --git a/apps/checker/pkg/job/tcp_job.go b/apps/checker/pkg/job/tcp_job.go index a4891bffd..60ef46d69 100644 --- a/apps/checker/pkg/job/tcp_job.go +++ b/apps/checker/pkg/job/tcp_job.go @@ -54,7 +54,7 @@ func (jobRunner) TCPJob(ctx context.Context, monitor *v1.TCPMonitor, region stri op := func() (*TCPPrivateRegionData, error) { called++ - res, err := checker.PingTCP(int(monitor.Timeout), monitor.Uri) + res, err := checker.PingTCP(ctx, monitor.Timeout, monitor.Uri) if err != nil { if called < int(retry) { return nil, fmt.Errorf("TCP connection failed: %w", err) -- 2.54.0