[Spice-devel] [PATCH] SPICE-HTML5: Improve error message

[Frantisek Kobzik]

Yeah, I know about this.

The thing is that error is logged anyway using this.parent.report_log on the next line. I didn't want to duplicate the (useless) information in the debug div.

But I can change the patch so that it prints the error the old way and concats it with my message which is a little bit more detailed.

If you think I should choose this way, let me know.

Cheers,
F.


----- Original Message -----
From: "Christophe Fergeau" <cfergeau at redhat.com>
To: "Frantisek Kobzik" <fkobzik at redhat.com>
Cc: spice-devel at freedesktop.org
Sent: Thursday, October 30, 2014 11:28:15 AM
Subject: Re: [Spice-devel] [PATCH] SPICE-HTML5: Improve error message

On Thu, Oct 30, 2014 at 10:35:15AM +0100, Frantisek Kobzik wrote:
> Current error message in SPICE-HTML5 error is poor (error-logging function
> prints error.toString() which doesn't contain anything detailed).
> 
> This patch enhances this message in the following way:
>  - error event contains target attribute ([1]),
>  - the code in this patch checks for presence of 'url' attribute (which is
>    contained in WebSocket object[2]). If the target contains url then
>    meaningful message is logged.
> 
> [1]: https://developer.mozilla.org/en-US/docs/Web/API/Event
> [2]: https://developer.mozilla.org/en-US/docs/Web/API/WebSocket
> ---
>  spiceconn.js | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/spiceconn.js b/spiceconn.js
> index e33227e..ceebd5d 100644
> --- a/spiceconn.js
> +++ b/spiceconn.js
> @@ -78,7 +78,9 @@ function SpiceConn(o)
>          this.parent.state = "start";
>      });
>      this.ws.addEventListener('error', function(e) {
> -        this.parent.log_err(">> WebSockets.onerror" + e.toString());
> +        if ('url' in e.target) {
> +            this.parent.log_err("WebSocket error: Can't connect to websocket on URL: " + e.target.url);
> +        }

We lost the "WebSockets.onerror" log if there is no 'url' in e.target, I
don't know if this is likely to happen?

ACK for this version if you think this is fine.

Christophe