UserStorage: Improve error handling (#104025)

packages/grafana-runtime/src/utils/userStorage.test.tsx

@@ -79,13 +79,16 @@ describe('userStorage', () => {
 
     it('creates a new user storage if it does not exist', async () => {
       request.mockReturnValueOnce(Promise.reject({ status: 404 } as FetchError));
+      request.mockReturnValueOnce(Promise.resolve({ status: 200 } as FetchResponse));
       const storage = usePluginUserStorage();
       await storage.setItem('key', 'value');
-      expect(request).toHaveBeenCalledWith({
-        url: '/apis/userstorage.grafana.app/v0alpha1/namespaces/default/user-storage/plugin-id:abc',
-        method: 'GET',
-        showErrorAlert: false,
-      });
+      expect(request).toHaveBeenCalledWith(
+        expect.objectContaining({
+          url: '/apis/userstorage.grafana.app/v0alpha1/namespaces/default/user-storage/plugin-id:abc',
+          method: 'GET',
+          showErrorAlert: false,
+        })
+      );
       expect(request).toHaveBeenCalledWith(
         expect.objectContaining({
           url: '/apis/userstorage.grafana.app/v0alpha1/namespaces/default/user-storage/',
@@ -98,6 +101,17 @@ describe('userStorage', () => {
           },
         })
       );
+      expect(localStorage.setItem).not.toHaveBeenCalled();
+    });
+
+    it('falls back to localStorage if the user storage fails to be created', async () => {
+      // Get fails with not found
+      request.mockReturnValueOnce(Promise.reject({ status: 404 } as FetchError));
+      // Create fails with forbidden
+      request.mockReturnValueOnce(Promise.reject({ status: 403 } as FetchError));
+      const storage = usePluginUserStorage();
+      await storage.setItem('key', 'value');
+      expect(localStorage.setItem).toHaveBeenCalledWith('plugin-id:abc:key', 'value');
     });
 
     it('updates the user storage if it exists', async () => {
@@ -109,11 +123,13 @@ describe('userStorage', () => {
       );
       const storage = usePluginUserStorage();
       await storage.setItem('key', 'new-value');
-      expect(request).toHaveBeenCalledWith({
-        url: '/apis/userstorage.grafana.app/v0alpha1/namespaces/default/user-storage/plugin-id:abc',
-        method: 'GET',
-        showErrorAlert: false,
-      });
+      expect(request).toHaveBeenCalledWith(
+        expect.objectContaining({
+          url: '/apis/userstorage.grafana.app/v0alpha1/namespaces/default/user-storage/plugin-id:abc',
+          method: 'GET',
+          showErrorAlert: false,
+        })
+      );
       expect(request).toHaveBeenCalledWith(
         expect.objectContaining({
           url: '/apis/userstorage.grafana.app/v0alpha1/namespaces/default/user-storage/plugin-id:abc',

packages/grafana-runtime/src/utils/userStorage.tsx

@@ -27,6 +27,7 @@ async function apiRequest<T>(requestOptions: RequestOptions) {
         ...requestOptions,
         url: baseURL + requestOptions.url,
         data: requestOptions.body,
+        showErrorAlert: false,
       })
     );
     return { data: responseData, meta };
@@ -60,17 +61,19 @@ export class UserStorage {
     const userStorage = await apiRequest<{ spec: UserStorageSpec }>({
       url: `/${this.resourceName}`,
       method: 'GET',
-      showErrorAlert: false,
+      manageError: (error) => {
+        if (get(error, 'status') === 404) {
+          this.storageSpec = null;
+          return { error: null };
+        }
+        return { error };
+      },
     });
     if ('error' in userStorage) {
-      if (get(userStorage, 'error.status') !== 404) {
-        console.error('Failed to get user storage', userStorage.error);
-      }
-      // No user storage found, return null
-      this.storageSpec = null;
-    } else {
-      this.storageSpec = userStorage.data.spec;
+      return userStorage.error;
     }
+    this.storageSpec = userStorage.data.spec;
+    return;
   }
 
   async getItem(key: string): Promise<string | null> {
@@ -96,7 +99,12 @@ export class UserStorage {
 
     const newData = { data: { [key]: value } };
     // Ensure this.storageSpec is initialized
-    await this.init();
+    const error = await this.init();
+    if (error) {
+      // Fallback to localStorage
+      localStorage.setItem(`${this.resourceName}:${key}`, value);
+      return;
+    }
 
     if (!this.storageSpec) {
       // No user storage found, create a new one
@@ -107,6 +115,11 @@ export class UserStorage {
           metadata: { name: this.resourceName, labels: { user: this.userUID, service: this.service } },
           spec: newData,
         },
+        manageError: (error) => {
+          // Fallback to localStorage
+          localStorage.setItem(`${this.resourceName}:${key}`, value);
+          return { error };
+        },
       });
       this.storageSpec = newData;
       return;
@@ -119,6 +132,11 @@ export class UserStorage {
       url: `/${this.resourceName}`,
       method: 'PATCH',
       body: { spec: newData },
+      manageError: (error) => {
+        // Fallback to localStorage
+        localStorage.setItem(`${this.resourceName}:${key}`, value);
+        return { error };
+      },
     });
   }
 }

pkg/registry/apis/userstorage/register.go

@@ -94,7 +94,7 @@ func (b *UserStorageAPIBuilder) GetAuthorizer() authorizer.Authorizer {
 			switch attr.GetVerb() {
 			case "create":
 				// Create requests are validated later since we don't have access to the resource name
-				return authorizer.DecisionNoOpinion, "", nil
+				return authorizer.DecisionAllow, "", nil
 			case "get", "delete", "patch", "update":
 				// Only allow the user to access their own settings
 				if !compareResourceNameAndUserUID(attr.GetName(), u) {

pkg/registry/apis/userstorage/register_test.go

@@ -43,7 +43,7 @@ func TestAuthorizer(t *testing.T) {
 			requesterID: "123",
 			objectName:  "",
 			verb:        "create",
-			decision:    authorizer.DecisionNoOpinion,
+			decision:    authorizer.DecisionAllow,
 		},
 		{
 			name:        "forbidden action",