feat: prefer remote taskfiles over cached ones (#1345)

* feat: prefer remote taskfiles over cached ones

* feat: implemented cache on network timeout

* feat: --download always downloads, but never executes tasks

* feat: --timeout flag

* fix: bug with timeout error handling

* chore: changelog

CHANGELOG.md

@@ -2,9 +2,16 @@
 
 ## Unreleased
 
+- The
+  [Remote Taskfiles experiment](https://taskfile.dev/experiments/remote-taskfiles)
+  now prefers remote files over cached ones by default (#1317, #1345 by @pd93).
+- Added `--timeout` flag to the
+  [Remote Taskfiles experiment](https://taskfile.dev/experiments/remote-taskfiles)
+  (#1317, #1345 by @pd93).
 - Fix bug where dynamic `vars:` and `env:` were being executed when they should
   actually be skipped by `platforms:` (#1273, #1377 by @andreynering).
-- Fix `schema.json` to make `silent` valid in `cmds` that use `for` (#1385, #1386 by @iainvm).
+- Fix `schema.json` to make `silent` valid in `cmds` that use `for` (#1385,
+  #1386 by @iainvm).
 - Add new `--no-status` flag to skip expensive status checks when running
   `task --list --json` (#1348, #1368 by @amancevice).
 
@@ -12,7 +19,7 @@
 
 - Enabled the `--yes` flag for the
   [Remote Taskfiles experiment](https://taskfile.dev/experiments/remote-taskfiles)
-  (#1344 by @pd93).
+  (#1317, #1344 by @pd93).
 - Add ability to set `watch: true` in a task to automatically run it in watch
   mode (#231, #1361 by @andreynering).
 - Fixed a bug on the watch mode where paths that contained `.git` (like
@@ -26,8 +33,8 @@
   exists to detect recursive calls, but will be removed in favor of a better
   algorithm soon (#1321, #1332).
 - Fixed templating on descriptions on `task --list` (#1343 by @blackjid).
-- Fixed a bug where precondition errors were incorrectly being printed when
-  task execution was aborted (#1337, #1338 by @sylv-io).
+- Fixed a bug where precondition errors were incorrectly being printed when task
+  execution was aborted (#1337, #1338 by @sylv-io).
 
 ## v3.30.1 - 2023-09-14
 

cmd/task/task.go

@@ -75,6 +75,7 @@ var flags struct {
 	experiments bool
 	download    bool
 	offline     bool
+	timeout     time.Duration
 }
 
 func main() {
@@ -150,6 +151,7 @@ func run() error {
 	if experiments.RemoteTaskfiles {
 		pflag.BoolVar(&flags.download, "download", false, "Downloads a cached version of a remote Taskfile.")
 		pflag.BoolVar(&flags.offline, "offline", false, "Forces Task to only use local or cached Taskfiles.")
+		pflag.DurationVar(&flags.timeout, "timeout", time.Second*10, "Timeout for downloading remote Taskfiles.")
 	}
 
 	pflag.Parse()
@@ -235,6 +237,7 @@ func run() error {
 		Insecure:    flags.insecure,
 		Download:    flags.download,
 		Offline:     flags.offline,
+		Timeout:     flags.timeout,
 		Watch:       flags.watch,
 		Verbose:     flags.verbose,
 		Silent:      flags.silent,
@@ -270,6 +273,12 @@ func run() error {
 		return err
 	}
 
+	// If the download flag is specified, we should stop execution as soon as
+	// taskfile is downloaded
+	if flags.download {
+		return nil
+	}
+
 	if listOptions.ShouldListTasks() {
 		foundTasks, err := e.ListTasks(listOptions)
 		if err != nil {
@@ -298,9 +307,7 @@ func run() error {
 	}
 
 	// If there are no calls, run the default task instead
-	// Unless the download flag is specified, in which case we want to download
-	// the Taskfile and do nothing else
-	if len(calls) == 0 && !flags.download {
+	if len(calls) == 0 {
 		calls = append(calls, taskfile.Call{Task: "default", Direct: true})
 	}
 

docs/docs/experiments/remote_taskfiles.md

@@ -74,16 +74,17 @@ you are doing.
 
 ## Caching & Running Offline
 
-If for whatever reason, you don't have access to the internet, but you still
-need to be able to run your tasks, you are able to use the `--download` flag to
-store a cached copy of the remote Taskfile.
+Whenever you run a remote Taskfile, the latest copy will be downloaded from the
+internet and cached locally. If for whatever reason, you lose access to the
+internet, you will still be able to run your tasks by specifying the `--offline`
+flag. This will tell Task to use the latest cached version of the file instead
+of trying to download it. You are able to use the `--download` flag to update
+the cached version of the remote files without running any tasks.
 
-<!-- TODO: The following behavior may change -->
-
-If Task detects that you have a local copy of the remote Taskfile, it will use
-your local copy instead of downloading the remote file. You can force Task to
-work offline by using the `--offline` flag. This will prevent Task from making
-any calls to remote sources.
+By default, Task will timeout requests to download remote files after 10 seconds
+and look for a cached copy instead. This timeout can be configured by setting
+the `--timeout` flag and specifying a duration. For example, `--timeout 5s` will
+set the timeout to 5 seconds.
 
 <!-- prettier-ignore-start -->
 [remote-taskfiles-experiment]: https://github.com/go-task/task/issues/1317

errors/errors.go

@@ -18,6 +18,7 @@ const (
 	CodeTaskfileNotSecure
 	CodeTaskfileCacheNotFound
 	CodeTaskfileVersionNotDefined
+	CodeTaskfileNetworkTimeout
 )
 
 // Task related exit codes

errors/errors_taskfile.go

@@ -3,6 +3,7 @@ package errors
 import (
 	"fmt"
 	"net/http"
+	"time"
 )
 
 // TaskfileNotFoundError is returned when no appropriate Taskfile is found when
@@ -137,3 +138,26 @@ func (err *TaskfileVersionNotDefined) Error() string {
 func (err *TaskfileVersionNotDefined) Code() int {
 	return CodeTaskfileVersionNotDefined
 }
+
+// TaskfileNetworkTimeout is returned when the user attempts to use a remote
+// Taskfile but a network connection could not be established within the timeout.
+type TaskfileNetworkTimeout struct {
+	URI          string
+	Timeout      time.Duration
+	CheckedCache bool
+}
+
+func (err *TaskfileNetworkTimeout) Error() string {
+	var cacheText string
+	if err.CheckedCache {
+		cacheText = " and no offline copy was found in the cache"
+	}
+	return fmt.Sprintf(
+		`task: Network connection timed out after %s while attempting to download Taskfile %q%s`,
+		err.Timeout, err.URI, cacheText,
+	)
+}
+
+func (err *TaskfileNetworkTimeout) Code() int {
+	return CodeTaskfileNetworkTimeout
+}

setup.go

@@ -91,6 +91,7 @@ func (e *Executor) readTaskfile() error {
 		e.Insecure,
 		e.Download,
 		e.Offline,
+		e.Timeout,
 		e.TempDir,
 		e.Logger,
 	)

task.go

@@ -46,6 +46,7 @@ type Executor struct {
 	Insecure    bool
 	Download    bool
 	Offline     bool
+	Timeout     time.Duration
 	Watch       bool
 	Verbose     bool
 	Silent      bool

taskfile/read/taskfile.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"time"
 
 	"gopkg.in/yaml.v3"
 
@@ -37,6 +38,7 @@ func readTaskfile(
 	node Node,
 	download,
 	offline bool,
+	timeout time.Duration,
 	tempDir string,
 	l *logger.Logger,
 ) (*taskfile.Taskfile, error) {
@@ -51,35 +53,44 @@ func readTaskfile(
 		}
 	}
 
-	// If the file is remote, check if we have a cached copy
-	// If we're told to download, skip the cache
-	if node.Remote() && !download {
-		if b, err = cache.read(node); !errors.Is(err, os.ErrNotExist) && err != nil {
-			return nil, err
-		}
-
-		if b != nil {
-			l.VerboseOutf(logger.Magenta, "task: [%s] Fetched cached copy\n", node.Location())
-		}
-	}
-
-	// If the file is remote, we found nothing in the cache and we're not
-	// allowed to download it then we can't do anything.
-	if node.Remote() && b == nil && offline {
-		if b == nil && offline {
+	// If the file is remote and we're in offline mode, check if we have a cached copy
+	if node.Remote() && offline {
+		if b, err = cache.read(node); errors.Is(err, os.ErrNotExist) {
 			return nil, &errors.TaskfileCacheNotFound{URI: node.Location()}
-		}
-	}
-
-	// If we still don't have a copy, get the file in the usual way
-	if b == nil {
-		b, err = node.Read(context.Background())
-		if err != nil {
+		} else if err != nil {
 			return nil, err
 		}
+		l.VerboseOutf(logger.Magenta, "task: [%s] Fetched cached copy\n", node.Location())
+
+	} else {
+
+		downloaded := false
+		ctx, cf := context.WithTimeout(context.Background(), timeout)
+		defer cf()
+
+		// Read the file
+		b, err = node.Read(ctx)
+		// If we timed out then we likely have a network issue
+		if node.Remote() && errors.Is(ctx.Err(), context.DeadlineExceeded) {
+			// If a download was requested, then we can't use a cached copy
+			if download {
+				return nil, &errors.TaskfileNetworkTimeout{URI: node.Location(), Timeout: timeout}
+			}
+			// Search for any cached copies
+			if b, err = cache.read(node); errors.Is(err, os.ErrNotExist) {
+				return nil, &errors.TaskfileNetworkTimeout{URI: node.Location(), Timeout: timeout, CheckedCache: true}
+			} else if err != nil {
+				return nil, err
+			}
+			l.VerboseOutf(logger.Magenta, "task: [%s] Network timeout. Fetched cached copy\n", node.Location())
+		} else if err != nil {
+			return nil, err
+		} else {
+			downloaded = true
+		}
 
 		// If the node was remote, we need to check the checksum
-		if node.Remote() {
+		if node.Remote() && downloaded {
 			l.VerboseOutf(logger.Magenta, "task: [%s] Fetched remote copy\n", node.Location())
 
 			// Get the checksums
@@ -102,24 +113,21 @@ func readTaskfile(
 				}
 			}
 
-			// If the hash has changed (or is new), store it in the cache
+			// If the hash has changed (or is new)
 			if checksum != cachedChecksum {
+				// Store the checksum
 				if err := cache.writeChecksum(node, checksum); err != nil {
 					return nil, err
 				}
+				// Cache the file
+				l.VerboseOutf(logger.Magenta, "task: [%s] Caching downloaded file\n", node.Location())
+				if err = cache.write(node, b); err != nil {
+					return nil, err
+				}
 			}
 		}
 	}
 
-	// If the file is remote and we need to cache it
-	if node.Remote() && download {
-		l.VerboseOutf(logger.Magenta, "task: [%s] Caching downloaded file\n", node.Location())
-		// Cache the file for later
-		if err = cache.write(node, b); err != nil {
-			return nil, err
-		}
-	}
-
 	var t taskfile.Taskfile
 	if err := yaml.Unmarshal(b, &t); err != nil {
 		return nil, &errors.TaskfileInvalidError{URI: filepathext.TryAbsToRel(node.Location()), Err: err}
@@ -137,12 +145,13 @@ func Taskfile(
 	insecure bool,
 	download bool,
 	offline bool,
+	timeout time.Duration,
 	tempDir string,
 	l *logger.Logger,
 ) (*taskfile.Taskfile, error) {
 	var _taskfile func(Node) (*taskfile.Taskfile, error)
 	_taskfile = func(node Node) (*taskfile.Taskfile, error) {
-		t, err := readTaskfile(node, download, offline, tempDir, l)
+		t, err := readTaskfile(node, download, offline, timeout, tempDir, l)
 		if err != nil {
 			return nil, err
 		}