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

feat(connection): have a way to know what is the state of the connection #43

Open
davinkevin opened this issue Jun 29, 2016 · 9 comments
Assignees

Comments

@davinkevin
Copy link
Owner

Linked to the issue #42, we should have a method to know if the system is connected or not.

@davinkevin davinkevin self-assigned this Jun 29, 2016
@davinkevin
Copy link
Owner Author

@baczus I think, maybe, changing the reference of the promise during the connection error fallback could help you, you'll always a promise initialized, but the resolve / reject happen only after the next ngStomp.connect() method.

In this way, I just have to move the init of the promise, and if I forget nothing, you can check the state of the connection by accessing the promise itself. I'll expose the promise around a getter to have something more simple (in term of api), but I think this could be a good idea.

Do you see something wrong with this idea ? Don't hesitate 😄

@baczus
Copy link
Contributor

baczus commented Jun 30, 2016

How can I determine state of the connection based on promise?

@davinkevin
Copy link
Owner Author

By using the then function ? Like this :

ngStomp.getConnectionState()
    .then(callbackCalledIfConnected)
    .catch(callbackCalledIfNotConnected)

This way, instead of using a boolean, which reflect the $$state of the angular $q promise, we can use the promise itself.

@baczus
Copy link
Contributor

baczus commented Jun 30, 2016

That sounds reasonable. I can't wait to test this change 😄

davinkevin added a commit that referenced this issue Jul 2, 2016
… the promise on the ngStomp service

If you want to know if the system is connected or not, you can subscribe to the promise `ngStomp.connectionState.then()` and execute some code. `then` first function will be called if connection is up, `then` or `catch` will be called if connection is down.

Don't forget, if you set a long timeOut in setting, the promise won't be fulfilled until we reach this timeOut.

Linked to #43
davinkevin added a commit that referenced this issue Jul 2, 2016
… the promise on the ngStomp service

If you want to know if the system is connected or not, you can subscribe to the promise `ngStomp.connectionState.then()` and execute some code. `then` first function will be called if connection is up, `then` or `catch` will be called if connection is down.

Don't forget, if you set a long timeOut in setting, the promise won't be fulfilled until we reach this timeOut.

Linked to #43
@davinkevin
Copy link
Owner Author

@baczus : Could you try to see if this modification feet your need.

I've updated the master but not yet released it, I'm waiting to see if it's all ok for you. To try this version, you just have to get the version available in the /dist/ folder.

@baczus
Copy link
Contributor

baczus commented Jul 2, 2016

All looks good,but I have two questions.

  1. What about function which returns connection state (promise)?
  2. Does function initConnectionState should not be set as private (with $ at the beginning)?

@davinkevin
Copy link
Owner Author

davinkevin commented Jul 3, 2016

  1. What about function which returns connection state (promise)?

The promise is accessible with ngStomp.connectionState. It's not a function, just a reference to the promise itself.

  1. Does function initConnectionState should not be set as private (with $ at the beginning)?

The problem with this, and what I don't like in this solution, is if someone want to init a connection by itseft (for example, after an intentional disconnection), he should call initiConnectionState and connect after it... And, I don't think this is a good solution after all, but I don't find a better way to do that right now. Because of that, the two method should have the same visibility, both private or public. Right now, I've choose public, but I'm thinking about a better way to do that...

If you have idea about it, let me know 😄

@baczus
Copy link
Contributor

baczus commented Jul 3, 2016

I don't think it's a good idea to force user to invoke two methods in situation when he wants to set new connection. Maybe a better solution is creating one public method connect, which internally will invoke private methods $initConnectionState and $connect?

davinkevin added a commit that referenced this issue Jul 4, 2016
… the promise on the ngStomp service

If you want to know if the system is connected or not, you can subscribe to the promise `ngStomp.connectionState.then()` and execute some code. `then` first function will be called if connection is up, second `then` function  or `catch` will be called if connection is down.

Don't forget, if you set a long timeOut in setting, the promise won't be fulfilled until we reach this timeOut. If the connection have already been open, the catch method won't be called

Linked to #43
davinkevin added a commit that referenced this issue Jul 4, 2016
… the promise on the ngStomp service

If you want to know if the system is connected or not, you can subscribe to the promise `ngStomp.connectionState.then()` and execute some code. `then` first function will be called if connection is up, second `then` function  or `catch` will be called if connection is down.

Don't forget, if you set a long timeOut in setting, the promise won't be fulfilled until we reach this timeOut. If the connection have already been open, the catch method won't be called

Linked to #43
@davinkevin
Copy link
Owner Author

I've published the 0.9.3, with this improvement, but I still think there is a better solution for this...

One promise is used for the connection, resolved in the success callback and reject in the error callback. But, both can be called by the lib, so I begin to think it should be handle by two different promise.

And, to get the connectionState, something like the Promise.race or $q.all on both promise to have the general state of the connection...

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

2 participants