Skip to content

Conversation

scottinet
Copy link
Contributor

Description

On a websocket connection error, the error thrown by the SDK has the following message: [object Object]

This is because on a connection error, ws sends a complex object, but our error handler processes it as if it's a string.

How to test it

Run this snippet:

const {Kuzzle, WebSocket} = require('kuzzle-sdk');
const
  ws = new WebSocket('localhost'),
  kuzzle = new Kuzzle(ws);

kuzzle.connect().catch(err => console.error(err));
kuzzle.disconnect();

Before this PR, this prints:

Error: [object Object]
    ... stack trace ...

With this PR:

Error: WebSocket was closed before the connection was established
    ... stack trace ...

@codecov-io
Copy link

codecov-io commented Feb 14, 2019

Codecov Report

Merging #373 into 6-dev will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           6-dev     #373      +/-   ##
=========================================
+ Coverage   96.7%   96.71%   +<.01%     
=========================================
  Files         29       29              
  Lines       1427     1429       +2     
=========================================
+ Hits        1380     1382       +2     
  Misses        47       47
Impacted Files Coverage Δ
src/protocols/websocket.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa88cf7...d1b5213. Read the comment docs.

this.client.onerror = error => {
const err = (error instanceof Error) && error || new Error(error);
const err = error ?
new Error(error.message || error) : new Error('Unexpected error');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we loose the stacktrace returned by Kuzzle?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be fixed with https://jira.kaliop.net/browse/KZL-1001, nevermind

Copy link
Contributor Author

@scottinet scottinet Feb 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, even if it will be fixed, I think your point is valid. I prefer to fix this now, even if it is redundant with a future task.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Aschen > fixed

@alexandrebouthinon alexandrebouthinon merged commit 5898461 into 6-dev Feb 19, 2019
@alexandrebouthinon alexandrebouthinon deleted the fix-invalid-error-object branch February 19, 2019 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants