public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH proxmox-backup] EncryptionKeysView: Fix API2 error alert messages
@ 2026-04-28 15:09 Arthur Bied-Charreton
  2026-04-29  9:57 ` Daniel Kral
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Arthur Bied-Charreton @ 2026-04-28 15:09 UTC (permalink / raw)
  To: pbs-devel

The errors returned by the encryption-keys and tape-encryption-keys
endpoints are not plain strings, so the alerts displayed on API2 errors
like failed permission checks end up being empty and therefore not very
helpful.

Extract the error messages to display them correctly in alerts.

Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 www/config/EncryptionKeysView.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/www/config/EncryptionKeysView.js b/www/config/EncryptionKeysView.js
index 0f9367c7..bb327814 100644
--- a/www/config/EncryptionKeysView.js
+++ b/www/config/EncryptionKeysView.js
@@ -168,7 +168,7 @@ Ext.define('PBS.config.EncryptionKeysView', {
                     });
                 }
             } catch (error) {
-                Ext.Msg.alert(gettext('Error'), error);
+                Ext.Msg.alert(gettext('Error'), error.result?.message || gettext('Unknown error'));
             }
 
             try {
@@ -181,7 +181,7 @@ Ext.define('PBS.config.EncryptionKeysView', {
                     });
                 }
             } catch (error) {
-                Ext.Msg.alert(gettext('Error'), error);
+                Ext.Msg.alert(gettext('Error'), error.result?.message || gettext('Unknown error'));
             }
 
             let store = view.getStore().rstore;
-- 
2.47.3




^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH proxmox-backup] EncryptionKeysView: Fix API2 error alert messages
  2026-04-28 15:09 [PATCH proxmox-backup] EncryptionKeysView: Fix API2 error alert messages Arthur Bied-Charreton
@ 2026-04-29  9:57 ` Daniel Kral
  2026-04-29  9:57 ` Thomas Lamprecht
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Daniel Kral @ 2026-04-29  9:57 UTC (permalink / raw)
  To: Arthur Bied-Charreton, pbs-devel

On Tue Apr 28, 2026 at 5:09 PM CEST, Arthur Bied-Charreton wrote:
> The errors returned by the encryption-keys and tape-encryption-keys
> endpoints are not plain strings, so the alerts displayed on API2 errors
> like failed permission checks end up being empty and therefore not very
> helpful.
>
> Extract the error messages to display them correctly in alerts.
>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
>  www/config/EncryptionKeysView.js | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/www/config/EncryptionKeysView.js b/www/config/EncryptionKeysView.js
> index 0f9367c7..bb327814 100644
> --- a/www/config/EncryptionKeysView.js
> +++ b/www/config/EncryptionKeysView.js
> @@ -168,7 +168,7 @@ Ext.define('PBS.config.EncryptionKeysView', {
>                      });
>                  }
>              } catch (error) {
> -                Ext.Msg.alert(gettext('Error'), error);
> +                Ext.Msg.alert(gettext('Error'), error.result?.message || gettext('Unknown error'));
>              }
>  
>              try {
> @@ -181,7 +181,7 @@ Ext.define('PBS.config.EncryptionKeysView', {
>                      });
>                  }
>              } catch (error) {
> -                Ext.Msg.alert(gettext('Error'), error);
> +                Ext.Msg.alert(gettext('Error'), error.result?.message || gettext('Unknown error'));
>              }
>  
>              let store = view.getStore().rstore;

Usually we use `error.htmlStatus` for the error message, but this should
be fine here as well.

This fixes the issue for me as well, so consider as:

Reviewed-by: Daniel Kral <d.kral@proxmox.com>
Tested-by: Daniel Kral <d.kral@proxmox.com>




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH proxmox-backup] EncryptionKeysView: Fix API2 error alert messages
  2026-04-28 15:09 [PATCH proxmox-backup] EncryptionKeysView: Fix API2 error alert messages Arthur Bied-Charreton
  2026-04-29  9:57 ` Daniel Kral
@ 2026-04-29  9:57 ` Thomas Lamprecht
  2026-04-29  9:59 ` Christian Ebner
  2026-04-29 10:05 ` Arthur Bied-Charreton
  3 siblings, 0 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2026-04-29  9:57 UTC (permalink / raw)
  To: Arthur Bied-Charreton, pbs-devel

Code change is fine, but the metadata needs polish before this can go in,
it's only the subject that should improve though:

* Subject prefix should be the product area, not a JS class name. For
  this view we use `ui: encryption keys: ...`, see e.g. ffe55e23e,
  67235b5fb. Lowercase, two-level path, separated by colons.

* Lowercase imperative after the colon: "Fix" -> "fix".

* The text after the prefix should describe what visibly changes for
  the user, not the mechanism. "Fix API2 error alert messages" hides
  the symptom; something like

      ui: encryption keys: show actual error text in API alerts instead of empty popup

  reads on its own in `git log` and tells a future git bisect(or)
  and changelog writers what this really changed without having
  to look at the full commit message or even the code changes.

See the proxmox commit-message conventions on the dev wiki and
`git log --oneline www/` for examples in this tree.




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH proxmox-backup] EncryptionKeysView: Fix API2 error alert messages
  2026-04-28 15:09 [PATCH proxmox-backup] EncryptionKeysView: Fix API2 error alert messages Arthur Bied-Charreton
  2026-04-29  9:57 ` Daniel Kral
  2026-04-29  9:57 ` Thomas Lamprecht
@ 2026-04-29  9:59 ` Christian Ebner
  2026-04-29 10:05 ` Arthur Bied-Charreton
  3 siblings, 0 replies; 5+ messages in thread
From: Christian Ebner @ 2026-04-29  9:59 UTC (permalink / raw)
  To: pbs-devel

On 4/28/26 5:08 PM, Arthur Bied-Charreton wrote:
> The errors returned by the encryption-keys and tape-encryption-keys
> endpoints are not plain strings, so the alerts displayed on API2 errors
> like failed permission checks end up being empty and therefore not very
> helpful.
> 
> Extract the error messages to display them correctly in alerts.
> 
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---

Thanks for the patch, definitely fine as stop-gap for now. But I think I 
have to improve the error handling here in general. This should probably 
include setting the error mask if neither permissions for tape or sync 
keys are given to the user, and less invasive show the missing 
permissions for other combinations.




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH proxmox-backup] EncryptionKeysView: Fix API2 error alert messages
  2026-04-28 15:09 [PATCH proxmox-backup] EncryptionKeysView: Fix API2 error alert messages Arthur Bied-Charreton
                   ` (2 preceding siblings ...)
  2026-04-29  9:59 ` Christian Ebner
@ 2026-04-29 10:05 ` Arthur Bied-Charreton
  3 siblings, 0 replies; 5+ messages in thread
From: Arthur Bied-Charreton @ 2026-04-29 10:05 UTC (permalink / raw)
  To: pbs-devel

On Tue, Apr 28, 2026 at 05:09:50PM +0200, Arthur Bied-Charreton wrote:
> The errors returned by the encryption-keys and tape-encryption-keys
> endpoints are not plain strings, so the alerts displayed on API2 errors
> like failed permission checks end up being empty and therefore not very
> helpful.
> 
> Extract the error messages to display them correctly in alerts.
> 
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
>  www/config/EncryptionKeysView.js | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/www/config/EncryptionKeysView.js b/www/config/EncryptionKeysView.js
> index 0f9367c7..bb327814 100644
> --- a/www/config/EncryptionKeysView.js
> +++ b/www/config/EncryptionKeysView.js
> @@ -168,7 +168,7 @@ Ext.define('PBS.config.EncryptionKeysView', {
>                      });
>                  }
>              } catch (error) {
> -                Ext.Msg.alert(gettext('Error'), error);
> +                Ext.Msg.alert(gettext('Error'), error.result?.message || gettext('Unknown error'));
>              }
>  
>              try {
> @@ -181,7 +181,7 @@ Ext.define('PBS.config.EncryptionKeysView', {
>                      });
>                  }
>              } catch (error) {
> -                Ext.Msg.alert(gettext('Error'), error);
> +                Ext.Msg.alert(gettext('Error'), error.result?.message || gettext('Unknown error'));
>              }
>  
>              let store = view.getStore().rstore;
> -- 
> 2.47.3
> 
> 
> 
> 
Thanks @Daniel, @Thomas and @Christian for the feedback! Sent v2:

https://lore.proxmox.com/pbs-devel/20260429100257.155092-1-a.bied-charreton@proxmox.com




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-29 10:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-28 15:09 [PATCH proxmox-backup] EncryptionKeysView: Fix API2 error alert messages Arthur Bied-Charreton
2026-04-29  9:57 ` Daniel Kral
2026-04-29  9:57 ` Thomas Lamprecht
2026-04-29  9:59 ` Christian Ebner
2026-04-29 10:05 ` Arthur Bied-Charreton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal