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

access to ABORT messages #71

Open
brianbirke opened this issue Dec 22, 2015 · 6 comments
Open

access to ABORT messages #71

brianbirke opened this issue Dec 22, 2015 · 6 comments

Comments

@brianbirke
Copy link
Contributor

It should be possible for user-code to know when an ABORT messages has arrived.
Now - it ends in a FIXME statement.

For example:

  • create a new wamp_abort class
  • introduce an virtual function on_abort
  • add a process_abort function

an instance of the wamp_abort is filled in the new process_abort function,
and the virtual on_abort function is called with that instance as argument.

or how to make it better????

@jpetso
Copy link
Contributor

jpetso commented Dec 22, 2015

If I'm not misreading the spec, ABORT would be sent to a client before (instead of) WELCOME messages, meaning the session won't be established.

The wamp_session::join() method already returns a boost::future<uint64_t> for the case when establishing the session succeeds. I'm thinking the most natural way to signal an ABORT here would be to utilize future's ability to relay exceptions, and instead of the FIXME, call the promise's m_session_join.set_exception(...) with a wamp_abort_exception argument that contains all of the necessary data.

The caller can then catch that exception in particular if they're interested in the ABORT reason. They should already be guarding against exceptions in general because of bad connectivity, session already joined errors, etc.

@jpetso
Copy link
Contributor

jpetso commented Dec 22, 2015

On that note, I think the throw protocol_error("session already joined") in join() might end up in the wrong spot - it'll throw inside the generic run loop without much of an opportunity to catch it, rather than signaling the error to whoever called join().

@brianbirke
Copy link
Contributor Author

I agree it is more correct to use the m_session_join promise. I will make a change request implementing your suggestion...... soon...

@brianbirke
Copy link
Contributor Author

Eventually I found the time.

I had problems though, transfer the "Details|dict" to the exception. Missing knowledge about what exactly it can contain! If anyone have a strategy of how to transfer the "Details|dict" to the exception in a readable way, please speak out, and I will fix it.... ( or just fix it if easier ... :-) )

@brianbirke
Copy link
Contributor Author

sorry for the mess, It looks like I closed - no I actually did close it, but it was actually not what I wanted!!!

@brianbirke brianbirke reopened this Feb 6, 2016
@jpetso
Copy link
Contributor

jpetso commented Feb 7, 2016

re: adding "Details|dict" to the exception
Since you're coming from got_message(), which gets both the msgpack message obj as parameter as well as a movable zone, you could pass down the zone and move it into the exception, together with the msgpack object for Details. It seems like the exception has to be copyable, so you might have to move the zone from its current unique_ptr into a shared_ptr stored by the exception(s).

Then you can offer access to the exception handler via e.g. const msgpack::object& details() const; and let them deal with the rest.

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

2 participants