Skip to content

Fix erroneous error message on browser ws error #462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 14, 2019
Merged

Fix erroneous error message on browser ws error #462

merged 1 commit into from
Nov 14, 2019
+24 −2

Conversation

scottinet
Copy link
Contributor

@scottinet scottinet commented Nov 13, 2019 •
edited
Loading

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
@codecov
Copy link

codecov bot commented Nov 13, 2019 •
edited
Loading

Codecov Report

Merging #462 into 6-dev will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##            6-dev     #462   +/-   ##
=======================================
  Coverage   96.03%   96.03%           
=======================================
  Files          33       33           
  Lines        1362     1362           
=======================================
  Hits         1308     1308           
  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 0bc5a45...c762faa. Read the comment docs.

@jenow jenow merged commit 17d85fc into 6-dev Nov 14, 2019
@jenow jenow deleted the fix-ws-error-message branch November 14, 2019 10:12
@scottinet scottinet mentioned this pull request Nov 18, 2019
Merged
scottinet added a commit that referenced this pull request Nov 18, 2019
@scottinet
Merge pull request #466 from kuzzleio/6.2.7-proposal
e089091 
# [6.2.7](https://github.com/kuzzleio/sdk-javascript/releases/tag/6.2.7) (2019-11-18)


#### Bug fixes

- [ [#462](#462) ] Fix erroneous error message on browser ws error   ([scottinet](https://github.com/scottinet))
---
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexandrebouthinon alexandrebouthinon alexandrebouthinon approved these changes

+2 more reviewers

@jenow jenow jenow approved these changes

@Aschen Aschen Aschen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@scottinet scottinet

Labels

6-dev changelog:bug-fixes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@scottinet @jenow @Aschen @alexandrebouthinon