Retry workspace get-status on transient failures during sync

.nextchanges/bundles/get-status-retries.md

@@ -0,0 +1 @@
+* `databricks bundle deploy`, `databricks bundle sync` and `databricks sync` now retry the workspace path check (`workspace/get-status`) when it fails with a transient server error (5xx or 408) or an HTTP/2 `REFUSED_STREAM`, instead of failing. ([#7044](https://github.com/databricks/cli/pull/7044))

acceptance/cmd/sync/get-status-504/hello.py

@@ -0,0 +1 @@
+print("hello")

acceptance/cmd/sync/get-status-504/out.test.toml

@@ -0,0 +1 @@
+Cloud = false

acceptance/cmd/sync/get-status-504/output.txt

@@ -0,0 +1,4 @@
+Warn: retrying after 504 Gateway Timeout from GET /api/2.0/workspace/get-status
+Action: PUT: hello.py
+Uploaded hello.py
+Initial Sync Complete

acceptance/cmd/sync/get-status-504/script

@@ -0,0 +1,22 @@
+# .gitignore excludes itself and .databricks/ so that hello.py is the only synced file.
+mkdir .git
+cat > .gitignore << EOF2
+.databricks/
+.gitignore
+EOF2
+
+# Use an ephemeral remote directory so concurrent test runs do not interfere.
+remote_dir="/Users/$CURRENT_USER_NAME/sync-getstatus-504-$UNIQUE_NAME"
+
+cleanup() {
+  MSYS_NO_PATHCONV=1 $CLI workspace delete --recursive "$remote_dir" 2>/dev/null || true
+  rm -rf .git .gitignore .databricks
+}
+trap cleanup EXIT
+
+# Inject a single 504 on the first workspace get-status, the call that validates the sync
+# target (issue #6472). The read has no side effects, so it is retried and the sync succeeds.
+fault.py "GET /api/2.0/workspace/get-status" 504 0 1
+
+# MSYS_NO_PATHCONV=1 prevents Git Bash on Windows from converting /Users/... to C:/Program Files/Git/Users/...
+MSYS_NO_PATHCONV=1 $CLI sync . "$remote_dir"

acceptance/cmd/sync/get-status-504/test.toml

@@ -0,0 +1 @@
+Env.DATABRICKS_BUNDLE_RETRY_INTERVAL_MS = "100"

acceptance/cmd/sync/output.txt

@@ -1,6 +1,6 @@
 
 >>> [CLI] sync . /Users/[USERNAME]
-Action: PUT: .gitignore, dryrun-missing-remote/hello.py, dryrun-missing-remote/out.test.toml, dryrun-missing-remote/output.txt, dryrun/out.test.toml, dryrun/output.txt, project-folder/app.py, project-folder/app.yaml, project-folder/query.sql, upload-504/hello.py, upload-504/out.test.toml, upload-504/output.txt
+Action: PUT: .gitignore, dryrun-missing-remote/hello.py, dryrun-missing-remote/out.test.toml, dryrun-missing-remote/output.txt, dryrun/out.test.toml, dryrun/output.txt, get-status-504/hello.py, get-status-504/out.test.toml, get-status-504/output.txt, project-folder/app.py, project-folder/app.yaml, project-folder/query.sql, upload-504/hello.py, upload-504/out.test.toml, upload-504/output.txt
 Initial Sync Complete
 Uploaded .gitignore
 Uploaded dryrun
@@ -10,6 +10,10 @@ Uploaded dryrun-missing-remote/out.test.toml
 Uploaded dryrun-missing-remote/output.txt
 Uploaded dryrun/out.test.toml
 Uploaded dryrun/output.txt
+Uploaded get-status-504
+Uploaded get-status-504/hello.py
+Uploaded get-status-504/out.test.toml
+Uploaded get-status-504/output.txt
 Uploaded project-folder
 Uploaded project-folder/app.py
 Uploaded project-folder/app.yaml

libs/retry/retry.go

@@ -6,7 +6,9 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"net/url"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/databricks/cli/libs/env"
@@ -50,6 +52,21 @@ func IsTransient(ctx context.Context, err error) bool {
 	return apiErr.StatusCode >= 500 || apiErr.StatusCode == http.StatusRequestTimeout
 }
 
+// IsTransientRead returns true for errors worth retrying on an idempotent read: the transient
+// server errors IsTransient covers, plus the HTTP/2 REFUSED_STREAM in issue #6472, where the
+// server refused the stream before processing the request so the read is safe to replay. The
+// SDK already retries the transport errors it recognizes (connection reset/refused, i/o timeout,
+// TLS handshake) for its full RetryTimeout, so retrying every *url.Error would restart that
+// multi-minute loop; REFUSED_STREAM is the gap it leaves. net/http bundles unexported http2
+// types, so we match the message, as the SDK does for its own transient url errors.
+func IsTransientRead(ctx context.Context, err error) bool {
+	if IsTransient(ctx, err) {
+		return true
+	}
+	_, ok := errors.AsType[*url.Error](err)
+	return ok && strings.Contains(err.Error(), "REFUSED_STREAM")
+}
+
 // With retries fn while check returns true for the error, up to MaxRetries times.
 func With[T any](ctx context.Context, check func(context.Context, error) bool, fn func() (T, error)) (T, error) {
 	wait := interval(ctx)

libs/retry/retry_test.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"net/url"
 	"testing"
 	"time"
 
@@ -26,6 +27,27 @@ func TestIsTransient(t *testing.T) {
 	assert.False(t, IsTransient(ctx, errors.New("boom")))
 }
 
+func TestIsTransientRead(t *testing.T) {
+	ctx := t.Context()
+	// Everything IsTransient covers is transient for a read too.
+	assert.True(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusInternalServerError}))
+	assert.True(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusRequestTimeout}))
+	// REFUSED_STREAM is the transport failure the SDK leaves unretried, so we replay the read (issue #6472).
+	refusedStream := &url.Error{Op: "Get", URL: "https://example.test/api/2.0/workspace/get-status", Err: errors.New("stream error: stream ID 1; REFUSED_STREAM; received from peer")}
+	assert.True(t, IsTransientRead(ctx, refusedStream))
+	// Transport errors the SDK already retries for its full RetryTimeout must not be retried again.
+	connRefused := &url.Error{Op: "Get", URL: "https://example.test/api/2.0/workspace/get-status", Err: errors.New("dial tcp 10.0.0.1:443: connect: connection refused")}
+	assert.False(t, IsTransientRead(ctx, connRefused))
+	// A cancelled request is a *url.Error too, but not REFUSED_STREAM, so it fails fast rather than logging a retry.
+	cancelled := &url.Error{Op: "Get", URL: "https://example.test/api/2.0/workspace/get-status", Err: context.Canceled}
+	assert.False(t, IsTransientRead(ctx, cancelled))
+	// Not transient: retried by the SDK, client errors, and plain non-url errors.
+	assert.False(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusServiceUnavailable}))
+	assert.False(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusNotFound}))
+	assert.False(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusForbidden}))
+	assert.False(t, IsTransientRead(ctx, errors.New("boom")))
+}
+
 // testContext shortens the pause between attempts.
 func testContext(t *testing.T) context.Context {
 	return env.Set(t.Context(), IntervalMsVariable, "1")

libs/sync/path.go

@@ -7,12 +7,22 @@ import (
 	"strings"
 
 	"github.com/databricks/cli/libs/log"
+	"github.com/databricks/cli/libs/retry"
 	"github.com/databricks/databricks-sdk-go"
 	"github.com/databricks/databricks-sdk-go/apierr"
 	"github.com/databricks/databricks-sdk-go/service/iam"
 	"github.com/databricks/databricks-sdk-go/service/workspace"
 )
 
+// getStatusWithRetry returns the workspace object at remotePath, retrying transient failures.
+// The read has no side effects, so a transient server error or a refused HTTP/2 stream
+// (https://github.com/databricks/cli/issues/6472) is safe to replay instead of failing the sync.
+func getStatusWithRetry(ctx context.Context, wsc *databricks.WorkspaceClient, remotePath string) (*workspace.ObjectInfo, error) {
+	return retry.With(ctx, retry.IsTransientRead, func() (*workspace.ObjectInfo, error) {
+		return wsc.Workspace.GetStatusByPath(ctx, remotePath)
+	})
+}
+
 func repoPathForPath(me *iam.User, remotePath string) string {
 	base := path.Clean("/Repos/" + me.UserName)
 	remotePath = path.Clean(remotePath)
@@ -40,7 +50,7 @@ func EnsureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClie
 	// Ensure that the remote path exists.
 	// If it is a repo, it has to exist.
 	// If it is a workspace path, it may not exist.
-	info, err := wsc.Workspace.GetStatusByPath(ctx, remotePath)
+	info, err := getStatusWithRetry(ctx, wsc, remotePath)
 	if err != nil {
 		// We only deal with 404s below.
 		if !apierr.IsMissing(err) {
@@ -50,7 +60,7 @@ func EnsureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClie
 		// If the path is nested under a repo, the repo has to exist.
 		if strings.HasPrefix(remotePath, "/Repos/") {
 			repoPath := repoPathForPath(me, remotePath)
-			_, err = wsc.Workspace.GetStatusByPath(ctx, repoPath)
+			_, err = getStatusWithRetry(ctx, wsc, repoPath)
 			if err != nil && apierr.IsMissing(err) {
 				return fmt.Errorf("%s does not exist; please create it first", repoPath)
 			}
@@ -66,7 +76,7 @@ func EnsureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClie
 		if err != nil {
 			return fmt.Errorf("unable to create directory at %s: %w", remotePath, err)
 		}
-		info, err = wsc.Workspace.GetStatusByPath(ctx, remotePath)
+		info, err = getStatusWithRetry(ctx, wsc, remotePath)
 		if err != nil {
 			return err
 		}