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

Force websocket close without reconnection how? #17

Closed
outaTiME opened this issue Feb 28, 2018 · 20 comments
Closed

Force websocket close without reconnection how? #17

outaTiME opened this issue Feb 28, 2018 · 20 comments

Comments

@outaTiME
Copy link

Hi pal,

How do you intentional close the websocket without reconnection execution, its necessary to pass event code to close method?

when manually invoke close method I think that the reconnection should not be done

ws.close()

thanks

@lukeed
Copy link
Owner

lukeed commented Feb 28, 2018

Hey,

You may need to update? The latest version will gracefully shutdown on ws.close(). Previous versions attempted a reconnection if a 1000 code was explicitly passed.

@outaTiME
Copy link
Author

hi @lukeed, im using the following version im my project:

"sockette": "^2.0.0",

And reconnection happens when i force close execution, i don't see any special handling for this logic in source.

@outaTiME
Copy link
Author

@lukeed
Copy link
Owner

lukeed commented Feb 28, 2018

Yes. The WebSocket.close internally defaults to 1000, which we accept as graceful. Similarly, servers that terminate the connection without a explicit code come down as 1005, which is also considered graceful.

This probably depends on your usage and/or your browser? Although I'm not aware of any browsers that don't comply to this spec.

@lukeed
Copy link
Owner

lukeed commented Feb 28, 2018

Here's quick little test you can paste in an empty tab:

function noop() {}

function Sockette(url, opts) {
	opts = opts || {};

	let k, ws, num, $={}, self=this;
	let ms=opts.timeout || 1e3, max=opts.maxAttempts || Infinity;

	$.onmessage = opts.onmessage || noop;

	$.onclose = e => {
		(e.code !== 1e3 && e.code !== 1005) && self.reconnect(e);
		(opts.onclose || noop)(e);
	};

	$.onerror = e => {
		(e && e.code==='ECONNREFUSED') ? self.reconnect(e) : (opts.onerror || noop)(e);
	};

	$.onopen = e => {
		num=0; (opts.onopen || noop)(e);
	};

	self.open = () => {
		ws = new WebSocket(url, opts.protocols);
		for (k in $) ws[k] = $[k];
	};

	self.reconnect = e => {
		(num++ < max) ? setTimeout(_ => {
			(opts.onreconnect || noop)(e);
			self.open();
		}, ms) : (opts.onmaximum || noop)(e);
	};

	self.json = x => {
		ws.send(JSON.stringify(x));
	};

	self.send = x => {
		ws.send(x);
	};

	self.close = (x, y) => {
		ws.close(x, y);
	};

	self.open(); // init

	return self;
}

ws = new Sockette('wss://echo.websocket.org', {
    onopen: e => console.log('OPENED', e),
    onconnect: e => console.log('connected', e),
    onreconnect: e => console.log('reconnecting...', e),
    onmessage: e => console.log('HELLO', e.data),
    onclose: e => console.log('closed!', e),
    onerror: e => console.log('error', e),
});
//=> OPENED {...}

ws.close();
//=> closed! {isTrusted: true, wasClean: true, code: 1005, ... }

@outaTiME
Copy link
Author

Mmm,
im using react native, probably it needs special handling ... i will check if close code is the same, i tell you in few minutes

@outaTiME
Copy link
Author

@lukeed in react native (with iOS) im getting the following event:

{ code: 1001, reason: 'Stream end encountered' }

by the moment im do special handling for close operations

@lukeed
Copy link
Owner

lukeed commented Feb 28, 2018

Okay, I would just do ws.close(1000). Sockette is made for browsers and so follows the browser specifications.

@outaTiME
Copy link
Author

Thanks @lukeed im try with your code but no luck, i go with websockets for react native 👍 thanks again =)

@lukeed
Copy link
Owner

lukeed commented Feb 28, 2018

Okay, sorry I couldn't be of more help! But glad to know that about RN.

@IDevJoe
Copy link

IDevJoe commented Mar 14, 2018

Reproduced the problem perfectly. It's definitely something with the library and it's compatibility with React.

@lukeed
Copy link
Owner

lukeed commented Mar 14, 2018

React Native you mean?

@outaTiME
Copy link
Author

Maybe when websockets are part of the core nodejs/node#19308

@IDevJoe
Copy link

IDevJoe commented Mar 14, 2018

I'm thinking it's react as a whole right now. I reproduced it using React Web.

I also noticed that it did not respect the maximum retries, and continued to try until I killed it

@lukeed
Copy link
Owner

lukeed commented Mar 14, 2018

If you could post or send me a gist of your React code that'd be great and I'll try to reproduce tonight!

@nonotest
Copy link

nonotest commented Nov 7, 2018

hello, I encountered same problem with react native (note that react works fine for me - which makes sense since its the normal javascript websocket lib) .

ws.close(1000) -> { code: 1001, reason: 'Stream end encountered' }
then sockette attempts to reconnect at least once.

The only workaround I could think of is using the close code argument passed in the self.close handler instead of relying on just the ws event code.
I can then turn off reconnecting easily.

hacky but i no better idea at the moment. I could do a PR but I doubt that's something you would want in your lib!

@lukeed
Copy link
Owner

lukeed commented Nov 7, 2018

@nonotest Thanks~! You could wrap/overwrite the reconnect method after you initialize Sockette.

Since the onclose event is passed to reconnect() directly, you can assert against/abort the reconnection.

It sounds like RN is doing something funky – and tbh, I've never used or plan to use RN so I can't really help in that particular context.

let foo = new Sockette({ ... });

// save old function
let _reconnect = foo.reconnect.bind(foo);

// overwrite it
foo.reconnect = e => {
  if (e && e.code === 1001) return; // abort
  _reconnect(e); // invoke old fn
};

@nonotest
Copy link

nonotest commented Nov 7, 2018

that's great thanks! btw sorry didn't realise the issue was closed before!
maybe could add a note in the readme for react-native users :)

@lukeed
Copy link
Owner

lukeed commented Nov 7, 2018

Sure, will do. Do you happen to know why it's closing with 1001 code? Maybe can point me to some relevant documentation?

Because if that is the standard behavior for RN (and not specific to your app) then I think that is a code I can exclude for browser users too.

@nonotest
Copy link

nonotest commented Nov 7, 2018

Hello,

When using a browser and connecting to the same server, I get the correct close code (using sockette as well), so I would definitely say there is something happening on the RN side.

I have found a few people talking about it https://github.com/facebook/react-native/issues/12678 but the issue was closed without much discussion..
I am not on the latest RN version (55.3) so not 100% sure if it's the intended behaviour thus I would not automatically exclude it for browsers.

I will try with RN latest and do some more research later and update you!

thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants