fix: improve api error handling for dashboards and folders (#111831)

pkg/api/apierrors/dashboard.go

@@ -17,8 +17,9 @@ import (
 
 // ToDashboardErrorResponse returns a different response status according to the dashboard error type
 func ToDashboardErrorResponse(ctx context.Context, pluginStore pluginstore.Store, err error) response.Response {
+	// --- Dashboard errors ---
 	var dashboardErr dashboardaccess.DashboardErr
-	if ok := errors.As(err, &dashboardErr); ok {
+	if errors.As(err, &dashboardErr) {
 		if body := dashboardErr.Body(); body != nil {
 			return response.JSON(dashboardErr.StatusCode, body)
 		}
@@ -28,28 +29,32 @@ func ToDashboardErrorResponse(ctx context.Context, pluginStore pluginstore.Store
 		return response.Error(dashboardErr.StatusCode, dashboardErr.Error(), nil)
 	}
 
+	// --- 400 Bad Request ---
 	if errors.Is(err, dashboards.ErrFolderNotFound) {
 		return response.Error(http.StatusBadRequest, err.Error(), nil)
 	}
 
 	var pluginErr dashboards.UpdatePluginDashboardError
-	if ok := errors.As(err, &pluginErr); ok {
+	if errors.As(err, &pluginErr) {
 		message := fmt.Sprintf("The dashboard belongs to plugin %s.", pluginErr.PluginId)
 		// look up plugin name
 		if plugin, exists := pluginStore.Plugin(ctx, pluginErr.PluginId); exists {
 			message = fmt.Sprintf("The dashboard belongs to plugin %s.", plugin.Name)
 		}
+		// --- 412 Precondition Failed ---
 		return response.JSON(http.StatusPreconditionFailed, util.DynMap{"status": "plugin-dashboard", "message": message})
 	}
 
+	// --- 413 Payload Too Large ---
 	if apierrors.IsRequestEntityTooLargeError(err) {
 		return response.Error(http.StatusRequestEntityTooLarge, fmt.Sprintf("Dashboard is too large, max is %d MB", apiserver.MaxRequestBodyBytes/1024/1024), err)
 	}
 
+	// --- Kubernetes status errors ---
 	var statusErr *apierrors.StatusError
 	if errors.As(err, &statusErr) {
 		return response.Error(int(statusErr.ErrStatus.Code), statusErr.ErrStatus.Message, err)
 	}
 
-	return response.Error(http.StatusInternalServerError, "Failed to save dashboard", err)
+	return response.ErrOrFallback(http.StatusInternalServerError, fmt.Sprintf("Dashboard API error: %s", err.Error()), err)
 }

pkg/api/apierrors/dashboard_test.go

@@ -0,0 +1,128 @@
+package apierrors
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/grafana/grafana/pkg/api/response"
+	"github.com/grafana/grafana/pkg/plugins"
+	"github.com/grafana/grafana/pkg/services/apiserver"
+	"github.com/grafana/grafana/pkg/services/dashboards"
+	"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
+	"github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore"
+	"github.com/grafana/grafana/pkg/util"
+)
+
+type fakePluginStore struct {
+	pluginstore.Store
+	plugins map[string]pluginstore.Plugin
+}
+
+func (f *fakePluginStore) Plugin(_ context.Context, id string) (pluginstore.Plugin, bool) {
+	p, ok := f.plugins[id]
+	return p, ok
+}
+
+func TestToDashboardErrorResponse(t *testing.T) {
+	pluginStoreWithPlugin := &fakePluginStore{
+		plugins: map[string]pluginstore.Plugin{
+			"test-plugin": {JSONData: plugins.JSONData{Name: "Test Plugin"}},
+		},
+	}
+	pluginStoreWithoutPlugin := &fakePluginStore{
+		plugins: map[string]pluginstore.Plugin{},
+	}
+
+	tests := []struct {
+		name        string
+		pluginStore pluginstore.Store
+		input       error
+		want        response.Response
+	}{
+		// --- 400 Bad Request ---
+		{
+			name:        "dashboard error with a bad-request status",
+			pluginStore: pluginStoreWithoutPlugin,
+			input:       dashboardaccess.DashboardErr{Reason: "Bad Request", StatusCode: http.StatusBadRequest},
+			want:        response.Error(http.StatusBadRequest, "Bad Request", nil),
+		},
+		// --- 403 Forbidden ---
+		{
+			name:        "dashboard error with a forbidden status",
+			pluginStore: pluginStoreWithoutPlugin,
+			input:       &k8sErrors.StatusError{ErrStatus: metav1.Status{Code: http.StatusForbidden, Message: "access denied"}},
+			want:        response.Error(http.StatusForbidden, "access denied", &k8sErrors.StatusError{ErrStatus: metav1.Status{Code: http.StatusForbidden, Message: "access denied"}}),
+		},
+		// --- 404 Not Found ---
+		{
+			name:        "folder not found error",
+			pluginStore: pluginStoreWithoutPlugin,
+			input:       dashboards.ErrFolderNotFound,
+			want:        response.Error(http.StatusBadRequest, dashboards.ErrFolderNotFound.Error(), nil),
+		},
+		{
+			name:        "dashboard error with a non-bad-request status",
+			pluginStore: pluginStoreWithoutPlugin,
+			input:       dashboardaccess.DashboardErr{Reason: "Not Found", StatusCode: http.StatusNotFound},
+			want:        response.Error(http.StatusNotFound, "Not Found", dashboardaccess.DashboardErr{Reason: "Not Found", StatusCode: http.StatusNotFound}),
+		},
+		{
+			name:        "plugin dashboard error where plugin is found",
+			pluginStore: pluginStoreWithPlugin,
+			input:       dashboards.UpdatePluginDashboardError{PluginId: "test-plugin"},
+			want:        response.JSON(http.StatusPreconditionFailed, util.DynMap{"status": "plugin-dashboard", "message": "The dashboard belongs to plugin Test Plugin."}),
+		},
+		// --- 412 Precondition Failed ---
+		{
+			name:        "plugin dashboard error where plugin is not found",
+			pluginStore: pluginStoreWithoutPlugin,
+			input:       dashboards.UpdatePluginDashboardError{PluginId: "unknown-plugin"},
+			want:        response.JSON(http.StatusPreconditionFailed, util.DynMap{"status": "plugin-dashboard", "message": "The dashboard belongs to plugin unknown-plugin."}),
+		},
+		// --- 413 Payload Too Large ---
+		{
+			name:        "request entity too large error",
+			pluginStore: pluginStoreWithoutPlugin,
+			input:       k8sErrors.NewRequestEntityTooLargeError("request is too large"),
+			want:        response.Error(http.StatusRequestEntityTooLarge, fmt.Sprintf("Dashboard is too large, max is %d MB", apiserver.MaxRequestBodyBytes/1024/1024), k8sErrors.NewRequestEntityTooLargeError("request is too large")),
+		},
+		// --- Kubernetes status errors ---
+		{
+			name:        "kubernetes status error",
+			pluginStore: pluginStoreWithoutPlugin,
+			input: &k8sErrors.StatusError{
+				ErrStatus: metav1.Status{
+					Code:    412,
+					Message: "the dashboard has been changed by someone else",
+				},
+			},
+			want: response.Error(412, "the dashboard has been changed by someone else", &k8sErrors.StatusError{
+				ErrStatus: metav1.Status{
+					Code:    412,
+					Message: "the dashboard has been changed by someone else",
+				},
+			}),
+		},
+		// --- 500 Internal Server Error ---
+		{
+			name:        "fallback error for an unknown error",
+			pluginStore: pluginStoreWithoutPlugin,
+			input:       errors.New("an unexpected error"),
+			want:        response.Error(http.StatusInternalServerError, "Dashboard API error: an unexpected error", errors.New("an unexpected error")),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			res := ToDashboardErrorResponse(context.Background(), tt.pluginStore, tt.input)
+			require.Equal(t, tt.want, res)
+		})
+	}
+}

pkg/api/apierrors/folder.go

@@ -3,6 +3,7 @@ package apierrors
 import (
 	"encoding/json"
 	"errors"
+	"fmt"
 	"net/http"
 
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
@@ -17,45 +18,49 @@ import (
 
 // ToFolderErrorResponse returns a different response status according to the folder error type
 func ToFolderErrorResponse(err error) response.Response {
+	// --- Dashboard errors ---
 	var dashboardErr dashboardaccess.DashboardErr
 	if ok := errors.As(err, &dashboardErr); ok {
 		return response.Error(dashboardErr.StatusCode, err.Error(), err)
 	}
 
+	// --- 400 Bad Request ---
 	if errors.Is(err, dashboards.ErrFolderTitleEmpty) ||
 		errors.Is(err, dashboards.ErrDashboardTypeMismatch) ||
 		errors.Is(err, dashboards.ErrDashboardInvalidUid) ||
-		errors.Is(err, dashboards.ErrDashboardUidTooLong) {
+		errors.Is(err, dashboards.ErrDashboardUidTooLong) ||
+		errors.Is(err, folder.ErrFolderCannotBeParentOfItself) {
 		return response.Error(http.StatusBadRequest, err.Error(), nil)
 	}
 
+	// --- 403 Forbidden ---
 	if errors.Is(err, dashboards.ErrFolderAccessDenied) {
 		return response.Error(http.StatusForbidden, "Access denied", err)
 	}
 
+	// --- 404 Not Found ---
 	if errors.Is(err, dashboards.ErrFolderNotFound) {
 		return response.JSON(http.StatusNotFound, util.DynMap{"status": "not-found", "message": dashboards.ErrFolderNotFound.Error()})
 	}
 
+	// --- 409 Conflict ---
 	if errors.Is(err, dashboards.ErrFolderWithSameUIDExists) {
 		return response.Error(http.StatusConflict, err.Error(), nil)
 	}
 
+	// --- 412 Precondition Failed ---
 	if errors.Is(err, dashboards.ErrFolderVersionMismatch) ||
 		k8sErrors.IsAlreadyExists(err) {
 		return response.JSON(http.StatusPreconditionFailed, util.DynMap{"status": "version-mismatch", "message": dashboards.ErrFolderVersionMismatch.Error()})
 	}
 
-	if errors.Is(err, folder.ErrMaximumDepthReached) {
+	// --- Kubernetes status errors ---
-		return response.JSON(http.StatusBadRequest, util.DynMap{"messageId": "folder.maximum-depth-reached", "message": folder.ErrMaximumDepthReached.Error()})
-	}
-
 	var statusErr *k8sErrors.StatusError
 	if errors.As(err, &statusErr) {
 		return response.Error(int(statusErr.ErrStatus.Code), statusErr.ErrStatus.Message, err)
 	}
 
-	return response.ErrOrFallback(http.StatusInternalServerError, "Folder API error", err)
+	return response.ErrOrFallback(http.StatusInternalServerError, fmt.Sprintf("Folder API error: %s", err.Error()), err)
 }
 
 func ToFolderStatusError(err error) k8sErrors.StatusError {

pkg/api/apierrors/folder_test.go

@@ -21,61 +21,109 @@ func TestToFolderErrorResponse(t *testing.T) {
 		input error
 		want  response.Response
 	}{
+		// --- 400 Bad Request ---
 		{
 			name:  "dashboard error",
-			input: dashboardaccess.DashboardErr{StatusCode: 400, Reason: "Dashboard Error", Status: "error"},
+			input: dashboardaccess.DashboardErr{StatusCode: http.StatusBadRequest, Reason: "Dashboard Error", Status: "error"},
-			want:  response.Error(400, "Dashboard Error", dashboardaccess.DashboardErr{StatusCode: 400, Reason: "Dashboard Error", Status: "error"}),
+			want:  response.Error(http.StatusBadRequest, "Dashboard Error", dashboardaccess.DashboardErr{StatusCode: http.StatusBadRequest, Reason: "Dashboard Error", Status: "error"}),
+		},
+		{
+			name:  "maximum depth reached",
+			input: folder.ErrMaximumDepthReached.Errorf("Maximum nested folder depth reached"),
+			want:  response.Err(folder.ErrMaximumDepthReached.Errorf("Maximum nested folder depth reached")),
+		},
+		{
+			name:  "bad request errors",
+			input: folder.ErrBadRequest.Errorf("Bad request error"),
+			want:  response.Err(folder.ErrBadRequest.Errorf("Bad request error")),
+		},
+		{
+			name:  "conflict error",
+			input: folder.ErrConflict.Errorf("Conflict error"),
+			want:  response.Err(folder.ErrConflict.Errorf("Conflict error")),
+		},
+		{
+			name:  "circular reference error",
+			input: folder.ErrCircularReference.Errorf("Circular reference detected"),
+			want:  response.Err(folder.ErrCircularReference.Errorf("Circular reference detected")),
+		},
+
+		{
+			name:  "folder not empty error",
+			input: folder.ErrFolderNotEmpty.Errorf("Folder cannot be deleted: folder is not empty"),
+			want:  response.Err(folder.ErrFolderNotEmpty.Errorf("Folder cannot be deleted: folder is not empty")),
 		},
 		{
 			name:  "folder title empty",
 			input: dashboards.ErrFolderTitleEmpty,
-			want:  response.Error(400, "folder title cannot be empty", nil),
+			want:  response.Error(http.StatusBadRequest, "folder title cannot be empty", nil),
 		},
 		{
 			name:  "dashboard type mismatch",
 			input: dashboards.ErrDashboardTypeMismatch,
-			want:  response.Error(400, "Dashboard cannot be changed to a folder", dashboards.ErrDashboardTypeMismatch),
+			want:  response.Error(http.StatusBadRequest, "Dashboard cannot be changed to a folder", dashboards.ErrDashboardTypeMismatch),
 		},
 		{
 			name:  "dashboard invalid uid",
 			input: dashboards.ErrDashboardInvalidUid,
-			want:  response.Error(400, "uid contains illegal characters", dashboards.ErrDashboardInvalidUid),
+			want:  response.Error(http.StatusBadRequest, "uid contains illegal characters", dashboards.ErrDashboardInvalidUid),
 		},
 		{
 			name:  "dashboard uid too long",
 			input: dashboards.ErrDashboardUidTooLong,
-			want:  response.Error(400, "uid too long, max 40 characters", dashboards.ErrDashboardUidTooLong),
+			want:  response.Error(http.StatusBadRequest, "uid too long, max 40 characters", dashboards.ErrDashboardUidTooLong),
 		},
+		{
+			name:  "folder cannot be parent of itself",
+			input: folder.ErrFolderCannotBeParentOfItself,
+			want:  response.Error(http.StatusBadRequest, folder.ErrFolderCannotBeParentOfItself.Error(), nil),
+		},
+		// --- 403 Forbidden ---
 		{
 			name:  "folder access denied",
 			input: dashboards.ErrFolderAccessDenied,
 			want:  response.Error(http.StatusForbidden, "Access denied", dashboards.ErrFolderAccessDenied),
 		},
+		// --- 404 Not Found ---
 		{
 			name:  "folder not found",
 			input: dashboards.ErrFolderNotFound,
 			want:  response.JSON(http.StatusNotFound, util.DynMap{"status": "not-found", "message": dashboards.ErrFolderNotFound.Error()}),
 		},
+		// --- 409 Conflict ---
 		{
 			name:  "folder with same uid exists",
 			input: dashboards.ErrFolderWithSameUIDExists,
 			want:  response.Error(http.StatusConflict, dashboards.ErrFolderWithSameUIDExists.Error(), nil),
 		},
+		// --- 412 Precondition Failed ---
 		{
 			name:  "folder version mismatch",
 			input: dashboards.ErrFolderVersionMismatch,
 			want:  response.JSON(http.StatusPreconditionFailed, util.DynMap{"status": "version-mismatch", "message": dashboards.ErrFolderVersionMismatch.Error()}),
 		},
+		// --- 500 Internal Server Error ---
 		{
-			name:  "folder max depth reached",
+			name:  "target registry srv conflict error",
-			input: folder.ErrMaximumDepthReached,
+			input: folder.ErrTargetRegistrySrvConflict.Errorf("Target registry service conflict"),
-			want:  response.JSON(http.StatusBadRequest, util.DynMap{"messageId": "folder.maximum-depth-reached", "message": folder.ErrMaximumDepthReached.Error()}),
+			want:  response.Err(folder.ErrTargetRegistrySrvConflict.Errorf("Target registry service conflict")),
 		},
 		{
-			name:  "fallback error",
+			name:  "internal error",
-			input: errors.New("some error"),
+			input: folder.ErrInternal.Errorf("Internal error"),
-			want:  response.ErrOrFallback(http.StatusInternalServerError, "Folder API error", errors.New("some error")),
+			want:  response.Err(folder.ErrInternal.Errorf("Internal error")),
 		},
+		{
+			name:  "database error",
+			input: folder.ErrDatabaseError.Errorf("Database error"),
+			want:  response.Err(folder.ErrDatabaseError.Errorf("Database error")),
+		},
+		{
+			name:  "fallback error for an unknown error",
+			input: errors.New("an unexpected error"),
+			want:  response.Error(http.StatusInternalServerError, "Folder API error: an unexpected error", errors.New("an unexpected error")),
+		},
+		// --- Kubernetes status errors ---
 		{
 			name: "kubernetes status error",
 			input: &k8sErrors.StatusError{

pkg/registry/apis/folders/validate.go

@@ -122,7 +122,7 @@ func validateOnUpdate(ctx context.Context,
 
 	// if by moving a folder we exceed the max depth, return an error
 	if len(info.Items)+1 >= maxDepth {
-		return folder.ErrMaximumDepthReached
+		return folder.ErrMaximumDepthReached.Errorf("maximum folder depth reached")
 	}
 	return nil
 }
@@ -146,7 +146,7 @@ func validateOnDelete(ctx context.Context,
 
 	for _, v := range resp.Stats {
 		if v.Count > 0 {
-			return folder.ErrFolderNotEmpty
+			return folder.ErrFolderNotEmpty.Errorf("folder is not empty, contains %d resources", v.Count)
 		}
 	}
 	return nil

pkg/services/folder/folderimpl/folder.go

@@ -1401,13 +1401,13 @@ func (s *Service) validateParent(ctx context.Context, orgID int64, parentUID str
 
 	// Create folder under itself is not allowed
 	if parentUID == UID {
-		return folder.ErrCircularReference
+		return folder.ErrCircularReference.Errorf("circular reference detected")
 	}
 
 	// check there is no circular reference
 	for _, ancestor := range ancestors {
 		if ancestor.UID == UID {
-			return folder.ErrCircularReference
+			return folder.ErrCircularReference.Errorf("circular reference detected")
 		}
 	}
 

pkg/services/folder/folderimpl/sqlstore.go

@@ -417,7 +417,7 @@ func (ss *FolderStoreImpl) GetHeight(ctx context.Context, foldrUID string, orgID
 			ele := queue[0]
 			queue = queue[1:]
 			if parentUID != nil && *parentUID == ele {
-				return 0, folder.ErrCircularReference
+				return 0, folder.ErrCircularReference.Errorf("circular reference detected")
 			}
 			folders, err := ss.GetChildren(ctx, folder.GetChildrenQuery{UID: ele, OrgID: orgID})
 			if err != nil {

pkg/services/folder/folderimpl/sqlstore_test.go

@@ -840,7 +840,7 @@ func TestIntegrationGetHeight(t *testing.T) {
 
 	t.Run("should failed when the parent folder exist in the subtree", func(t *testing.T) {
 		_, err = folderStore.GetHeight(context.Background(), parent.UID, orgID, &subTree[0])
-		require.Error(t, err, folder.ErrCircularReference)
+		require.Error(t, err, folder.ErrCircularReference.Errorf("circular reference detected"))
 	})
 }
 

pkg/services/folder/folderimpl/unifiedstore.go

@@ -298,7 +298,7 @@ func (ss *FolderUnifiedStoreImpl) GetHeight(ctx context.Context, foldrUID string
 			ele := queue[0]
 			queue = queue[1:]
 			if parentUID != nil && *parentUID == ele {
-				return 0, folder.ErrCircularReference
+				return 0, folder.ErrCircularReference.Errorf("circular reference detected")
 			}
 			folders, err := ss.GetChildren(ctx, folder.GetChildrenQuery{UID: ele, OrgID: orgID})
 			if err != nil {