Return errors instead of accumulating diagnostics

acceptance/bundle/deploy/wal/failed-plan-no-wal/output.txt

@@ -16,8 +16,6 @@ HTTP Status: 403 Forbidden
 API error_code: INJECTED
 API message: Fault injected by test.
 
-Error: planning failed
-
 
 Exit code: 1
 
@@ -33,8 +31,6 @@ HTTP Status: 403 Forbidden
 API error_code: INJECTED
 API message: Fault injected by test.
 
-Error: planning failed
-
 
 Exit code: 1
 

acceptance/bundle/lifecycle/prevent-destroy/out.direct.txt

@@ -1,7 +1,8 @@
 
 >>> musterr [CLI] bundle destroy --auto-approve
 Error: resources.pipelines.my_pipelines has lifecycle.prevent_destroy set, but the plan calls for this resource to be recreated or destroyed. To avoid this error, disable lifecycle.prevent_destroy for resources.pipelines.my_pipelines
-resources.schemas.my_schema has lifecycle.prevent_destroy set, but the plan calls for this resource to be recreated or destroyed. To avoid this error, disable lifecycle.prevent_destroy for resources.schemas.my_schema
+
+Error: resources.schemas.my_schema has lifecycle.prevent_destroy set, but the plan calls for this resource to be recreated or destroyed. To avoid this error, disable lifecycle.prevent_destroy for resources.schemas.my_schema
 
 
 >>> errcode [CLI] bundle plan
@@ -23,7 +24,8 @@ Plan: 2 to add, 0 to change, 2 to delete, 0 unchanged
 >>> musterr [CLI] bundle deploy
 Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/prevent-destroy/default/files...
 Error: resources.pipelines.my_pipelines has lifecycle.prevent_destroy set, but the plan calls for this resource to be recreated or destroyed. To avoid this error, disable lifecycle.prevent_destroy for resources.pipelines.my_pipelines
-resources.schemas.my_schema has lifecycle.prevent_destroy set, but the plan calls for this resource to be recreated or destroyed. To avoid this error, disable lifecycle.prevent_destroy for resources.schemas.my_schema
+
+Error: resources.schemas.my_schema has lifecycle.prevent_destroy set, but the plan calls for this resource to be recreated or destroyed. To avoid this error, disable lifecycle.prevent_destroy for resources.schemas.my_schema
 
 
 >>> errcode [CLI] bundle plan

acceptance/bundle/resource_deps/bad_ref_string_to_int/out.plan.direct.txt

@@ -2,5 +2,3 @@
 >>> musterr [CLI] bundle plan
 Error: cannot plan resources.jobs.foo: cannot update tasks[0].run_job_task.job_id with value of "${resources.jobs.bar.name}": cannot set (*jobs.JobSettings).tasks[0].run_job_task.job_id to string ("job bar"): cannot parse "job bar" as int64: strconv.ParseInt: parsing "job bar": invalid syntax
 
-Error: planning failed
-

acceptance/bundle/resource_deps/non_existent_field/out.deploy.direct.txt

@@ -1,5 +1,3 @@
 Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
 Error: cannot plan resources.volumes.foo: cannot resolve "${resources.volumes.bar.non_existent}": schema mismatch: non_existent: field "non_existent" not found in catalog.CreateVolumeRequestContent; non_existent: field "non_existent" not found in catalog.VolumeInfo
 
-Error: planning failed
-

acceptance/bundle/resource_deps/non_existent_field/out.plan.direct.txt

@@ -1,4 +1,2 @@
 Error: cannot plan resources.volumes.foo: cannot resolve "${resources.volumes.bar.non_existent}": schema mismatch: non_existent: field "non_existent" not found in catalog.CreateVolumeRequestContent; non_existent: field "non_existent" not found in catalog.VolumeInfo
 
-Error: planning failed
-

acceptance/bundle/resources/catalogs/auto-approve/output.txt

@@ -39,8 +39,6 @@ test-file-[UNIQUE_NAME].txt -> dbfs:/Volumes/test-catalog-[UNIQUE_NAME]/test-sch
 Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/[UNIQUE_NAME]/files...
 Error: cannot plan resources.schemas.foo: cannot resolve "${resources.catalogs.bar.name}": internal error: resources.catalogs.bar: action is "delete" missing new_state
 
-Error: planning failed
-
 
 === Test cleanup
 >>> [CLI] bundle destroy --auto-approve

acceptance/bundle/resources/permissions/pipelines/504/plan/output.txt

@@ -26,6 +26,4 @@ HTTP Status: 504 Gateway Timeout
 API error_code: INJECTED
 API message: Fault injected by test.
 
-Error: planning failed
-
 3 {"method": "GET", "path": "/api/2.0/permissions/pipelines/[UUID]"}

bundle/apps/validate.go

@@ -9,6 +9,7 @@ import (
 	"github.com/databricks/cli/bundle/config/resources"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
+	"github.com/databricks/cli/libs/logdiag"
 	"github.com/databricks/databricks-sdk-go/service/apps"
 )
 
@@ -17,7 +18,7 @@ var resourceReferencePattern = regexp.MustCompile(`\$\{resources\.(\w+)\.([^.]+)
 
 type validate struct{}
 
-func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
+func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) error {
 	var diags diag.Diagnostics
 	usedSourceCodePaths := make(map[string]string)
 
@@ -52,10 +53,10 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics
 		}
 		usedSourceCodePaths[app.SourceCodePath] = key
 
-		diags = diags.Extend(warnForAppResourcePermissions(b, key, app))
+		warnForAppResourcePermissions(ctx, b, key, app)
 	}
 
-	return diags
+	return logdiag.Flush(ctx, diags)
 }
 
 // appResourceRef extracts resource references from an app resource entry.
@@ -130,9 +131,7 @@ func hasAppSPInPermissions(b *bundle.Bundle, resourcePath, appKey string) bool {
 // Without the SP in the permission list, the second deploy will overwrite the
 // app-granted permission on the resource.
 // See https://github.com/databricks/cli/issues/4309
-func warnForAppResourcePermissions(b *bundle.Bundle, appKey string, app *resources.App) diag.Diagnostics {
-	var diags diag.Diagnostics
-
+func warnForAppResourcePermissions(ctx context.Context, b *bundle.Bundle, appKey string, app *resources.App) {
 	for _, ar := range app.Resources {
 		ref, ok := appResourceRef(ar)
 		if !ok {
@@ -155,7 +154,7 @@ func warnForAppResourcePermissions(b *bundle.Bundle, appKey string, app *resourc
 		}
 
 		appPath := "resources.apps." + appKey
-		diags = append(diags, diag.Diagnostic{
+		logdiag.LogDiag(ctx, diag.Diagnostic{
 			Severity: diag.Warning,
 			Summary:  fmt.Sprintf("app %q references %s %q which has permissions set. To prevent permission override after deploying the app, please add the app service principal to the %s permissions", appKey, refType, resourceKey, refType),
 			Detail: fmt.Sprintf(
@@ -176,8 +175,6 @@ func warnForAppResourcePermissions(b *bundle.Bundle, appKey string, app *resourc
 			Locations: b.Config.GetLocations(appPath),
 		})
 	}
-
-	return diags
 }
 
 func (v *validate) Name() string {

bundle/artifacts/build.go

@@ -29,7 +29,7 @@ func (m *build) Name() string {
 	return "artifacts.Build"
 }
 
-func (m *build) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
+func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error {
 	cacheDir, err := b.LocalStateDir(ctx)
 	if err != nil {
 		logdiag.LogDiag(ctx, diag.Diagnostic{
@@ -44,23 +44,19 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 		if a.BuildCommand != "" {
 			err := doBuild(ctx, artifactName, a)
 			if err != nil {
-				logdiag.LogError(ctx, err)
-				break
+				return err
 			}
 
 			// We need to expand glob reference after build mutator is applied because
 			// if we do it before, any files that are generated by build command will
 			// not be included into artifact.Files and thus will not be uploaded.
 			// We only do it if BuildCommand was specified because otherwise it should have been done already by artifacts.Prepare()
-			bundle.ApplyContext(ctx, b, expandGlobs{name: artifactName})
+			if err := bundle.ApplyContext(ctx, b, expandGlobs{name: artifactName}); err != nil {
+				return err
+			}
 
 			// After bundle.ApplyContext is called, all of b.Config is recreated and all pointers are invalidated (!)
 			a = b.Config.Artifacts[artifactName]
-
-			if logdiag.HasError(ctx) {
-				break
-			}
-
 		}
 
 		if a.Type == "whl" && a.DynamicVersion && cacheDir != "" {
@@ -71,9 +67,6 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 				if patchedWheel != "" {
 					a.Files[ind].Patched = patchedWheel
 				}
-				if logdiag.HasError(ctx) {
-					break
-				}
 			}
 		}
 	}

bundle/artifacts/expand_globs.go

@@ -9,6 +9,7 @@ import (
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/log"
+	"github.com/databricks/cli/libs/logdiag"
 	"github.com/databricks/cli/libs/patchwheel"
 )
 
@@ -39,7 +40,7 @@ func (e expandGlobs) Name() string {
 	return "expandGlobs"
 }
 
-func (e expandGlobs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
+func (e expandGlobs) Apply(ctx context.Context, b *bundle.Bundle) error {
 	// Base path for this mutator.
 	// This path is set with the list of expanded globs when done.
 	base := dyn.NewPath(
@@ -117,8 +118,8 @@ func (e expandGlobs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnosti
 		return dyn.SetByPath(rootv, base, dyn.V(output))
 	})
 	if err != nil {
-		diags = diags.Extend(diag.FromErr(err))
+		return err
 	}
 
-	return diags
+	return logdiag.Flush(ctx, diags)
 }

bundle/artifacts/prepare.go

@@ -15,7 +15,6 @@ import (
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/log"
-	"github.com/databricks/cli/libs/logdiag"
 	"github.com/databricks/cli/libs/python"
 )
 
@@ -29,23 +28,22 @@ func (m *prepare) Name() string {
 	return "artifacts.Prepare"
 }
 
-func (m *prepare) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
+func (m *prepare) Apply(ctx context.Context, b *bundle.Bundle) error {
 	err := InsertPythonArtifact(ctx, b)
 	if err != nil {
-		return diag.FromErr(err)
+		return err
 	}
 
 	for _, artifactName := range slices.Sorted(maps.Keys(b.Config.Artifacts)) {
 		artifact := b.Config.Artifacts[artifactName]
 		if artifact == nil {
 			l := b.Config.GetLocation("artifacts." + artifactName)
-			logdiag.LogDiag(ctx, diag.Diagnostic{
+			return diag.Diagnostic{
 				Severity:  diag.Error,
 				Summary:   "Artifact not properly configured",
 				Detail:    "please specify artifact properties",
 				Locations: []dyn.Location{l},
-			})
-			continue
+			}
 		}
 		b.Metrics.AddBoolValue(metrics.ArtifactBuildCommandIsSet, artifact.BuildCommand != "")
 		b.Metrics.AddBoolValue(metrics.ArtifactFilesIsSet, len(artifact.Files) != 0)
@@ -85,15 +83,13 @@ func (m *prepare) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics
 		}
 
 		if artifact.BuildCommand == "" && len(artifact.Files) == 0 {
-			logdiag.LogError(ctx, errors.New("misconfigured artifact: please specify 'build' or 'files' property"))
+			return errors.New("misconfigured artifact: please specify 'build' or 'files' property")
 		}
 
 		if len(artifact.Files) > 0 && artifact.BuildCommand == "" {
-			bundle.ApplyContext(ctx, b, expandGlobs{name: artifactName})
-		}
-
-		if logdiag.HasError(ctx) {
-			break
+			if err := bundle.ApplyContext(ctx, b, expandGlobs{name: artifactName}); err != nil {
+				return err
+			}
 		}
 	}
 

bundle/artifacts/upload.go

@@ -20,10 +20,10 @@ func (m *cleanUp) Name() string {
 	return "artifacts.CleanUp"
 }
 
-func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
-	client, uploadPath, diags := libraries.GetFilerForLibrariesCleanup(ctx, b)
-	if diags.HasError() {
-		return diags
+func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) error {
+	client, uploadPath, err := libraries.GetFilerForLibrariesCleanup(ctx, b)
+	if err != nil {
+		return err
 	}
 
 	skipArtifactsCleanup := b.Config.Experimental != nil && b.Config.Experimental.SkipArtifactCleanup
@@ -38,7 +38,7 @@ func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics
 		}
 	}
 
-	err := client.Mkdir(ctx, libraries.InternalDirName)
+	err = client.Mkdir(ctx, libraries.InternalDirName)
 	if err != nil {
 		return diag.Errorf("unable to create directory for %s: %v", uploadPath, err)
 	}

bundle/bundle.go

@@ -189,47 +189,33 @@ func Load(ctx context.Context, path string) (*Bundle, error) {
 }
 
 // MustLoad returns a bundle configuration.
-// The errors are recorded by logdiag, check with logdiag.HasError().
-func MustLoad(ctx context.Context) *Bundle {
+func MustLoad(ctx context.Context) (*Bundle, error) {
 	root, err := mustGetRoot(ctx)
 	if err != nil {
-		logdiag.LogError(ctx, err)
-		return nil
+		return nil, err
 	}
 
 	logdiag.SetRoot(ctx, root)
 
-	b, err := Load(ctx, root)
-	if err != nil {
-		logdiag.LogError(ctx, err)
-		return nil
-	}
-	return b
+	return Load(ctx, root)
 }
 
 // TryLoad returns a bundle configuration if there is one, but doesn't fail if there isn't one.
-// The errors are recorded by logdiag, check with logdiag.HasError().
 // It returns a `nil` bundle if a bundle was not found.
-func TryLoad(ctx context.Context) *Bundle {
+func TryLoad(ctx context.Context) (*Bundle, error) {
 	root, err := tryGetRoot(ctx)
 	if err != nil {
-		logdiag.LogError(ctx, err)
-		return nil
+		return nil, err
 	}
 
 	// No root is fine in this function.
 	if root == "" {
-		return nil
+		return nil, nil
 	}
 
 	logdiag.SetRoot(ctx, root)
 
-	b, err := Load(ctx, root)
-	if err != nil {
-		logdiag.LogError(ctx, err)
-		return nil
-	}
-	return b
+	return Load(ctx, root)
 }
 
 func (b *Bundle) initClientOnce(ctx context.Context) {

bundle/bundle_test.go

@@ -22,17 +22,23 @@ import (
 func mustLoad(t *testing.T) (*Bundle, []diag.Diagnostic) {
 	ctx := logdiag.InitContext(t.Context())
 	logdiag.SetCollect(ctx, true)
-	b := MustLoad(ctx)
+	b, err := MustLoad(ctx)
 	diags := logdiag.FlushCollected(ctx)
+	if err != nil {
+		diags = append(diags, diag.DiagnosticFromError(err))
+	}
 	return b, diags
 }
 
 // tryLoad calls TryLoad and returns the bundle and collected diagnostics
 func tryLoad(t *testing.T) (*Bundle, []diag.Diagnostic) {
 	ctx := logdiag.InitContext(t.Context())
 	logdiag.SetCollect(ctx, true)
-	b := TryLoad(ctx)
+	b, err := TryLoad(ctx)
 	diags := logdiag.FlushCollected(ctx)
+	if err != nil {
+		diags = append(diags, diag.DiagnosticFromError(err))
+	}
 	return b, diags
 }