2 sets of fixes. Connection error handling and first frame accessibility

[revelfire]

Hi Tom. Some important details on this request:

* Fixes issue where connection failures due to 302 and other http (upgrade request) failures cause the client to NPE by allowing to define a connectionError function to pass in and handle.  Bug was system was trying to invoke a null onClose handler because websocket was not yet established.  This was uncovered in working with Spring4 websocket support with security in place.

* Fixes the need (and lack of ability) to get information about the connection frames.  Use case here was Spring STOMP/Websocket implementation passes custom header user-name on CONNECT and there was no way to get ahold of it.

Probably you won't want to adopt this pull request in whole. Unfortunately I ended up changing the basic interface - the Future from connect() now returns a Map instead of a StompClient.

connecting.complete({ "stompClient": this, "frame": frame });

There may be a better way to do this like "'stompClient.getFirstFrame()" or something. I can make appropriate changes if you want to make a suggestion, and offer another pull request. Or if you make the changes please let me know.

Another thing that might be nice is to be able to get knowledge of the status code from the connection attempt. Right now I just have to guess that if it didn't work, it was due to 302. It wasn't immediately obvious how to fix that in the websocket stuff.

[tomyeh]

Thanks for your pull requests. Basically, there are two issues:

1, Why you need to return the first frame in connecting?

2. Why introduce StompFrame?

[revelfire]

1. Use case here was Spring STOMP/Websocket implementation passes custom header user-name on CONNECT and there was no way to get ahold of it. I could recode the application to provide alternate endpoints for accessing that information I suppose, but this was the simplest way to get to it. Other implementations may also provide client-specific information in the connect frame response.
2. Probably not entirely necessary. I wanted to keep your implementation 'private' while giving an exposable class to access the frame. In my implementation I have something like this: _connectingClientFuture = connect(...).then((connectionResult) { _stompClient = connectionResult["stompClient"]; userId = connectionResult["frame"].headers["user-name"]; logger.info("Got user id:" + userId) ; _connected = true; ... }); At the time I had anticipated having StompFrame frame = connectionResult["frame"] and then doing more things with it but in practice it was unnecessary.

In short it feels a little messy right now and it might be better to provide StompClient.getLastFrame() or some other access to them (or maybe even just the headers map).

[tomyeh]

Sorry, I don't accept this pull request. Instead, I implemented Issue #9.

[revelfire]

Looks like a better plan and accommodates the use case. I wasn't satisfied entirely with my solution. Thanks so much for the fixes.

I don't see anything that entirely accommodates the websocket connect failure, such as http 302, as the pull request had in Websocket.dart 57-59

WebSocket ws = new WebSocket(url);

ws.onError.listen(onConnectionError);

return new _WSStompConnector(ws)._starting.future;

Chris

On Tue, Jan 28, 2014 at 10:50 PM, Tom Yeh notifications@github.com wrote:

Closed #8 https://github.com/rikulo/stomp/pull/8.

Reply to this email directly or view it on GitHubhttps://github.com/rikulo/stomp/pull/8 .