Give the webhook handler a timeout (closes #38)
The webhook handler's client had no timeout, and slog calls the handler inside the log call, so a server that accepted the connection and never answered stopped that log call for good, and every later one. A request still running after 5 seconds, reading the answer included, now fails with a timeout error. The handler also reads the answer to the end before closing it, so the connection is reused for the next record. Two new tests point the handler at a server that never answers and at one that sends its status and then stalls the answer, and check that Handle returns a timeout error within the timeout. The README states the timeout. Model: opus-5-5
This commit is contained in:
@@ -5,6 +5,7 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"log/slog"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
@@ -160,3 +161,103 @@ func TestWebhookHandlerReturnsErrorOnRedirect(t *testing.T) {
|
||||
t.Fatalf("Handle returned %v, want an error for the redirect", err)
|
||||
}
|
||||
}
|
||||
|
||||
// A server that accepts the request and never answers must not hold up
|
||||
// the log call: Handle gives up after webhookTimeout and says why.
|
||||
func TestWebhookHandlerTimesOutOnServerThatNeverAnswers(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
testEnded := make(chan struct{})
|
||||
|
||||
server := httptest.NewServer(http.HandlerFunc(
|
||||
func(_ http.ResponseWriter, _ *http.Request) {
|
||||
<-testEnded
|
||||
},
|
||||
))
|
||||
|
||||
// server.Close waits for running requests, so the server's handler
|
||||
// is released first.
|
||||
defer func() {
|
||||
close(testEnded)
|
||||
server.Close()
|
||||
}()
|
||||
|
||||
handler, err := NewWebhookHandler(server.URL)
|
||||
if err != nil {
|
||||
t.Fatalf("NewWebhookHandler: %v", err)
|
||||
}
|
||||
|
||||
// Handle runs in a goroutine so that a lost timeout fails the test
|
||||
// instead of hanging the test run.
|
||||
handleErr := make(chan error, 1)
|
||||
|
||||
go func() {
|
||||
handleErr <- handler.Handle(context.Background(), errorTestRecord())
|
||||
}()
|
||||
|
||||
select {
|
||||
case err = <-handleErr:
|
||||
case <-time.After(webhookTimeout + time.Second):
|
||||
t.Fatal("Handle did not return within webhookTimeout")
|
||||
}
|
||||
|
||||
var netErr net.Error
|
||||
if !errors.As(err, &netErr) || !netErr.Timeout() {
|
||||
t.Fatalf("Handle returned %v, want a timeout error", err)
|
||||
}
|
||||
}
|
||||
|
||||
// A server that sends a 2xx status and then never finishes the answer
|
||||
// must not hold up the log call either: reading the answer counts toward
|
||||
// webhookTimeout, and Handle returns the read's error.
|
||||
func TestWebhookHandlerTimesOutOnServerThatStallsTheAnswer(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
testEnded := make(chan struct{})
|
||||
|
||||
server := httptest.NewServer(http.HandlerFunc(
|
||||
func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
|
||||
// Flush sends the status now; the answer stays unfinished
|
||||
// until the test ends.
|
||||
err := http.NewResponseController(w).Flush()
|
||||
if err != nil {
|
||||
t.Errorf("flush the status: %v", err)
|
||||
}
|
||||
|
||||
<-testEnded
|
||||
},
|
||||
))
|
||||
|
||||
// server.Close waits for running requests, so the server's handler
|
||||
// is released first.
|
||||
defer func() {
|
||||
close(testEnded)
|
||||
server.Close()
|
||||
}()
|
||||
|
||||
handler, err := NewWebhookHandler(server.URL)
|
||||
if err != nil {
|
||||
t.Fatalf("NewWebhookHandler: %v", err)
|
||||
}
|
||||
|
||||
// Handle runs in a goroutine so that a lost timeout fails the test
|
||||
// instead of hanging the test run.
|
||||
handleErr := make(chan error, 1)
|
||||
|
||||
go func() {
|
||||
handleErr <- handler.Handle(context.Background(), errorTestRecord())
|
||||
}()
|
||||
|
||||
select {
|
||||
case err = <-handleErr:
|
||||
case <-time.After(webhookTimeout + time.Second):
|
||||
t.Fatal("Handle did not return within webhookTimeout")
|
||||
}
|
||||
|
||||
var netErr net.Error
|
||||
if !errors.As(err, &netErr) || !netErr.Timeout() {
|
||||
t.Fatalf("Handle returned %v, want a timeout error", err)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user