Skip to content

Commit

Permalink
refactor: check if the remote exists just before reading it (#1713)
Browse files Browse the repository at this point in the history
* refactor: check if the remote exists in the read to avoid doing it in offline mode

* fix: timeout error was not working

* fix: use cached copy if available
  • Loading branch information
vmaerten authored Sep 7, 2024
1 parent a233b52 commit c77c8a4
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 12 deletions.
24 changes: 14 additions & 10 deletions taskfile/node_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import (
// An HTTPNode is a node that reads a Taskfile from a remote location via HTTP.
type HTTPNode struct {
*BaseNode
URL *url.URL
URL *url.URL
logger *logger.Logger
timeout time.Duration
}

func NewHTTPNode(
Expand All @@ -36,18 +38,12 @@ func NewHTTPNode(
if url.Scheme == "http" && !insecure {
return nil, &errors.TaskfileNotSecureError{URI: entrypoint}
}
ctx, cf := context.WithTimeout(context.Background(), timeout)
defer cf()
url, err = RemoteExists(ctx, l, url)
if err != nil {
return nil, err
}
if errors.Is(ctx.Err(), context.DeadlineExceeded) {
return nil, &errors.TaskfileNetworkTimeoutError{URI: url.String(), Timeout: timeout}
}

return &HTTPNode{
BaseNode: base,
URL: url,
timeout: timeout,
logger: l,
}, nil
}

Expand All @@ -60,13 +56,21 @@ func (node *HTTPNode) Remote() bool {
}

func (node *HTTPNode) Read(ctx context.Context) ([]byte, error) {
url, err := RemoteExists(ctx, node.logger, node.URL, node.timeout)
if err != nil {
return nil, err
}
node.URL = url
req, err := http.NewRequest("GET", node.URL.String(), nil)
if err != nil {
return nil, errors.TaskfileFetchFailedError{URI: node.URL.String()}
}

resp, err := http.DefaultClient.Do(req.WithContext(ctx))
if err != nil {
if errors.Is(err, context.DeadlineExceeded) {
return nil, &errors.TaskfileNetworkTimeoutError{URI: node.URL.String(), Timeout: node.timeout}
}
return nil, errors.TaskfileFetchFailedError{URI: node.URL.String()}
}
defer resp.Body.Close()
Expand Down
3 changes: 2 additions & 1 deletion taskfile/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,9 @@ func (r *Reader) readNode(node Node) (*ast.Taskfile, error) {

// Read the file
b, err = node.Read(ctx)
var taskfileNetworkTimeoutError *errors.TaskfileNetworkTimeoutError
// If we timed out then we likely have a network issue
if node.Remote() && errors.Is(ctx.Err(), context.DeadlineExceeded) {
if node.Remote() && errors.As(err, &taskfileNetworkTimeoutError) {
// If a download was requested, then we can't use a cached copy
if r.download {
return nil, &errors.TaskfileNetworkTimeoutError{URI: node.Location(), Timeout: r.timeout}
Expand Down
6 changes: 5 additions & 1 deletion taskfile/taskfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"path/filepath"
"slices"
"strings"
"time"

"github.com/go-task/task/v3/errors"
"github.com/go-task/task/v3/internal/filepathext"
Expand Down Expand Up @@ -40,7 +41,7 @@ var (
// at the given URL with any of the default Taskfile files names. If any of
// these match a file, the first matching path will be returned. If no files are
// found, an error will be returned.
func RemoteExists(ctx context.Context, l *logger.Logger, u *url.URL) (*url.URL, error) {
func RemoteExists(ctx context.Context, l *logger.Logger, u *url.URL, timeout time.Duration) (*url.URL, error) {
// Create a new HEAD request for the given URL to check if the resource exists
req, err := http.NewRequest("HEAD", u.String(), nil)
if err != nil {
Expand All @@ -50,6 +51,9 @@ func RemoteExists(ctx context.Context, l *logger.Logger, u *url.URL) (*url.URL,
// Request the given URL
resp, err := http.DefaultClient.Do(req.WithContext(ctx))
if err != nil {
if errors.Is(ctx.Err(), context.DeadlineExceeded) {
return nil, &errors.TaskfileNetworkTimeoutError{URI: u.String(), Timeout: timeout}
}
return nil, errors.TaskfileFetchFailedError{URI: u.String()}
}
defer resp.Body.Close()
Expand Down

0 comments on commit c77c8a4

Please sign in to comment.