[pve-devel] [PATCH widget-toolkit] improve error handling when adding webauthn entries

[pve-devel] [PATCH widget-toolkit] improve error handling when adding webauthn entries

[Wolfgang Bumiller]

There are cases where we directly throw a string error
(particularly, when navigator.credentials.create() fails,
for example when trying to register the same WA device to
the same user twice), which would end up with the WA window
simply not closing before.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/window/AddWebauthn.js | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/window/AddWebauthn.js b/src/window/AddWebauthn.js
index f4a0b10..77d0914 100644
--- a/src/window/AddWebauthn.js
+++ b/src/window/AddWebauthn.js
@@ -141,7 +141,12 @@ Ext.define('Proxmox.window.AddWebauthn', {
 		    params,
 		});
 	    } catch (response) {
-		let error = response.result.message;
+		let error = response;
+		if (typeof error !== "string") {
+		    // in case it came from an api request:
+		    error = error.result.message;
+		}
+
 		console.error(error); // for debugging if it's not displayable...
 		Ext.Msg.alert(gettext('Error'), error);
 	    }
-- 
2.30.2

Re: [pve-devel] [PATCH widget-toolkit] improve error handling when adding webauthn entries

[Thomas Lamprecht]

On 17.11.21 09:35, Wolfgang Bumiller wrote:
> There are cases where we directly throw a string error
> (particularly, when navigator.credentials.create() fails,
> for example when trying to register the same WA device to
> the same user twice), which would end up with the WA window
> simply not closing before.
> 
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>  src/window/AddWebauthn.js | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/window/AddWebauthn.js b/src/window/AddWebauthn.js
> index f4a0b10..77d0914 100644
> --- a/src/window/AddWebauthn.js
> +++ b/src/window/AddWebauthn.js
> @@ -141,7 +141,12 @@ Ext.define('Proxmox.window.AddWebauthn', {
>  		    params,
>  		});
>  	    } catch (response) {
> -		let error = response.result.message;
> +		let error = response;
> +		if (typeof error !== "string") {

why not

if (typeof error === "object") {

as just because it isn't a string it doesn't need to be a valid object..

> +		    // in case it came from an api request:
> +		    error = error.result.message;

maybe use the optional chaining operator here to make the error path a bit
more robust.

> +		}
> +
>  		console.error(error); // for debugging if it's not displayable...

should above be moved above the if then? To ensure that as much info as possible
is available from the error?

>  		Ext.Msg.alert(gettext('Error'), error);
>  	    }
>