Skip to content

Conversation

scottinet
Copy link
Contributor

@scottinet scottinet commented Nov 13, 2019

Description

The onerror event handler called in browsers' WebSocket API differ from the one exposed by the ws module used in Node.js: the handler in browsers receive a generic Event object without any error information, while the ws handler receives a proper error.

Result: kuzzle.connect().catch(e => console.log(e)) prints something like Error: [object Event] in a browser's console.

This PR fixes that issue by detecting the case and sending a generic Connection error to the connect promise handler.

How to reproduce

Run the following script in a browser:

<html>
<head>
<script src="node_modules/kuzzle-sdk/dist/kuzzle.js"></script>
</head>
<body>
<script>
const
  // doesn't matter, must not reach any kuzzle server is what is important
  ws = new KuzzleSDK.WebSocket('172.19.0.129', {port: 5555}),
  kuzzle = new KuzzleSDK.Kuzzle(ws);

console.log('Connecting...');

kuzzle
  .connect()
  .catch(err => {
    console.log(err);
    kuzzle.disconnect();
  });

</script>
</body>

Without this PR, the console should show the following message:

Error: [object Event]

With this PR:

Error: Connection error

@scottinet scottinet self-assigned this Nov 13, 2019
@scottinet scottinet changed the base branch from master to 7-dev November 13, 2019 13:37
@Aschen Aschen force-pushed the 7/fix-ws-error-message branch from 16cb507 to fb0388e Compare November 14, 2019 10:20
@codecov
Copy link

codecov bot commented Nov 14, 2019

Codecov Report

Merging #463 into 7-dev will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##            7-dev     #463   +/-   ##
=======================================
  Coverage   96.01%   96.01%           
=======================================
  Files          33       33           
  Lines        1355     1355           
=======================================
  Hits         1301     1301           
  Misses         54       54
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 4af6751...fb0388e. Read the comment docs.

@Aschen Aschen merged commit 961548b into 7-dev Nov 14, 2019
@Aschen Aschen deleted the 7/fix-ws-error-message branch November 14, 2019 12:14
@Aschen Aschen mentioned this pull request Nov 20, 2019
Aschen added a commit that referenced this pull request Nov 21, 2019
# [7.0.0](https://github.com/kuzzleio/sdk-javascript/releases/tag/7.0.0) (2019-11-20)


#### Bug fixes

- [ [#464](#464) ] Fix option refresh in queries   ([Yoann-Abbes](https://github.com/Yoann-Abbes))
- [ [#463](#463) ] Fix erroneous error message on browser ws error   ([scottinet](https://github.com/scottinet))

#### Enhancements

- [ [#455](#455) ] Do not try to reconnect if the browser is offline   ([scottinet](https://github.com/scottinet))
---
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