Skip to content

Commit

Permalink
feat(client): add timeout to fetchResource to prevent hangs (#2322)
Browse files Browse the repository at this point in the history
  • Loading branch information
jyyi1 authored Jan 13, 2025
1 parent b48dd2c commit 19aab4e
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
8 changes: 7 additions & 1 deletion client/go/outline/fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,22 @@ package outline
import (
"io"
"net/http"
"time"

"github.com/Jigsaw-Code/outline-apps/client/go/outline/platerrors"
)

const fetchTimeout = 10 * time.Second

// fetchResource fetches a resource from the given URL.
//
// The function makes an HTTP GET request to the specified URL and returns the response body as a
// string. If the request fails or the server returns a non-2xx status code, an error is returned.
func fetchResource(url string) (string, error) {
resp, err := http.Get(url)
client := &http.Client{
Timeout: fetchTimeout,
}
resp, err := client.Get(url)
if err != nil {
return "", platerrors.PlatformError{
Code: platerrors.FetchConfigFailed,
Expand Down
27 changes: 27 additions & 0 deletions client/go/outline/fetch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"net/http"
"net/http/httptest"
"testing"
"time"

"github.com/Jigsaw-Code/outline-apps/client/go/outline/platerrors"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -100,3 +101,29 @@ func TestFetchResource_BodyReadError(t *testing.T) {
require.Equal(t, platerrors.FetchConfigFailed, perr.Code)
require.Error(t, perr.Cause)
}

func TestFetchResource_Timeout(t *testing.T) {
const (
MaxFetchWaitTime = 12 * time.Second
ServerDelay = 20 * time.Second
)

testDone := make(chan bool)
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
select {
case <-time.After(ServerDelay):
w.WriteHeader(http.StatusNoContent)
case <-testDone:
}
}))
defer server.Close()

start := time.Now()
content, err := fetchResource(server.URL)
duration := time.Since(start)
testDone <- true

require.LessOrEqual(t, duration, MaxFetchWaitTime, "fetchResource should time out in 10s")
require.Error(t, err, "fetchResource should return a non-nil timeout error")
require.Empty(t, content)
}

0 comments on commit 19aab4e

Please sign in to comment.