qz-tray.js: isActive() is also true while connecting

[antoineveldhoven]

Hi @tresf,

I'm wondering what the motivation behind the concept isActive() === true while the websocket is connecting is.
In my opinion a connection is only active when it is truly connected, connecting state might not result in a connection.

In async operations qz reports back that it is active, while in fact it is still trying to connect.

            isActive: function() {
                return !_qz.websocket.shutdown && _qz.websocket.connection != null
                    && (_qz.websocket.connection.readyState === _qz.tools.ws.OPEN
                        || qz.websocket.connection.readyState === _qz.tools.ws.CONNECTING);
            },

[tresf]

@antoineveldhoven yeah we've had quite a few people complain about this behavior. I'll ping @akberenz for some input here. In short, I think we can fix this, but I'm not 100% certain if it'll cause regressions in certain edge-cases where people rely on this.

I guess if we do fix it, 2.3.0 -- which is right around the corner -- would be the time to make the change. I'll see what @akberenz has to say.

[antoineveldhoven]

We can introduce a isConnected() to the websocket, if that might avoid breaking existing implementations

[tresf]

@akberenz replied in PM stating the following:

it might have been changed that way for the disconnect() call,
and then probably missed the fact the isActive call just hooks into that same internal call...

tray/js/qz-tray.js

Lines 1380 to 1395 in 9da43e0

	disconnect: function() {
	return _qz.tools.promise(function(resolve, reject) {
	if (_qz.websocket.connection != null) {
	if (_qz.tools.isActive()) {
	// handles closing both 'connecting' and 'connected' states
	_qz.websocket.shutdown = true;
	_qz.websocket.connection.promise = { resolve: resolve, reject: reject };
	_qz.websocket.connection.close();
	} else {
	reject(new Error("Current connection is still closing"));
	}
	} else {
	reject(new Error("No open connection with " + _qz.TITLE));
	}
	});
	},

I think this means a proper implementation will split isActive() into two functions:isPending() | isActive()... then it will look something more like this:

- if (_qz.tools.isActive()) { 
-                  // handles closing both 'connecting' and 'connected' states 
+ if (_qz.tools.isActive() || _qz.tools.isPending()) { 
             ... 
  }

[antoineveldhoven]

I agree. Would you like me to write up a PR for this?

[tresf]

I agree. Would you like me to write up a PR for this?

That's greatly apprecaited however I think @akberenz has an even easier patch in mind.... Waiting on her. 🤞