Skip to content
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.

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

error occurs in IE11 and edge when connection is closed #36

Closed
houyaowei opened this issue Sep 27, 2018 · 8 comments
Closed

error occurs in IE11 and edge when connection is closed #36

houyaowei opened this issue Sep 27, 2018 · 8 comments

Comments

@houyaowei
Copy link

houyaowei commented Sep 27, 2018

Environment: React 16.2, webpack 4.10, sockette 2.0.0

in the react lifecycle method componentWillUnmount method, I will close the socket instance , the error occurs ,

the code blow
_20180927181119

error blow
_20180927181225
_20180927181247

when I add annotation to the ws close method , it get ok,
_20180927181606

How can I fix this problem , thx.

@lukeed
Copy link
Owner

lukeed commented Sep 27, 2018

How are you constructing the Sockette instance? Are you using new Sockette(...)? Are you experiencing the same issue on other browsers, or is it only IE11?

Thanks!

@houyaowei
Copy link
Author

houyaowei commented Sep 28, 2018

I use Sockette with npm package
the code like blow:

`import Sockette from "sockette";

this.ws = new Sockette(url, {
timeout: 100,
maxAttempts: 10,
},
onmessage: e => { },
onmaximum: e => {
that.ws.reconnect;
},
onerror: e => { },
onclose: e => {},
onreconnect: e => { }
});
`
the errors orrcured on the IE11 and Edge, other browsers(Chrome, Firefox, Opera) work fine.

Thanks!

@lukeed
Copy link
Owner

lukeed commented Nov 13, 2018

@houyaowei Try this:

this.ws ? this.ws.close(1000) : null

Unrelated, but when the opts.maximum is reached, Sockette quits. If you want to restart it, you need to call open and not reconnect because open starts the series whereas reconnect is a single attempt.

onmaximum: e => {
-  that.ws.reconnect;
+ this.ws.open();
},

@lukeed lukeed closed this as completed in 5102793 Nov 14, 2018
@elliotaplant
Copy link

elliotaplant commented Nov 29, 2018

@lukeed I know this issue is closed, but it seems a bit concerning that the default close .close() call throws an error in IE. Also, MDN states that specifying any code except 1000 and 3000 is illegal (in FireFox): https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent#Status_codes.

Could we make it so that the .close() method simply passes along its args to the native .close() method?

...
$.close = ws.close;
...

@lukeed
Copy link
Owner

lukeed commented Dec 4, 2018

@elliotaplant Thanks, this actually matches what I had thought initially, but I changed it to 1005 at the last second. Can you re-verify for me that sending code 1000 on close works on IE and your copy of Firefox? (I don't have access to any IE)

Thanks!

@elliotaplant
Copy link

I can verify that tomorrow!

@elliotaplant
Copy link

Sending ws.close(1000) Works on IE and Firefox for me!

I'm pretty busy right now but I could make a PR for this in January.

If you want to make the change, I think it would just be:

// src/index.js
...
-  $.close = function (x, y) {
-    ws.close(x || 1005, y);
-  };
+  $.close = ws.close
...

@lukeed
Copy link
Owner

lukeed commented Dec 12, 2018

Awesome, thank you 🙇

It cannot be your suggestion because the internal ws changes over any given Sockette instance's lifespan. After all, the purpose of Sockette is to swap out & setup new WebSocket instances without your concern.

Plus, ws isn't defined until $.open() is called, whereas $.close = ws.close would run (and $ returned) instantly

lukeed added a commit that referenced this issue Dec 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants