stomp-js icon indicating copy to clipboard operation
stomp-js copied to clipboard

Stomp.on('error') Not Called for TLS Errors

Open joshuarubin opened this issue 12 years ago • 2 comments

The following code will cause node to terminate on a TLS authentication error:

var stomp = new Stomp(args);
stomp.on('error', function (err) {
  console.error("Stomp Error: " + err);
});
stomp.connect()

I had to add the following (immediately after connect()) to be able to catch it:

stomp.socket.on('error', function (err) {
  console.error("Stomp TLS Error: " + err);
});

The reason for this is due to this code in stomp.js

stomp.socket = tls.connect(stomp.port, stomp.host, stomp.ssl_options, function() {
    log.debug('SSL connection complete');
    if (!stomp.socket.authorized) {
        log.error('SSL is not authorized: '+stomp.socket.authorizationError);
        if (stomp.ssl_validate) {
            _disconnect(stomp);
            return;
        }
    }
    _setupListeners(stomp);
});

_setupListeners(stomp); is only called after tls.connect() succeeds, in the callback.

According to http://nodejs.org/api/tls.html#tls_tls_connect_port_host_options_callback

the callback parameter will be added as a listener for the 'secureConnect' event.

Since there is no socket.on('error') handler when tls.connect fails, stomp never catches the error and never, subsequently, emits its own error.

I did find the above workaround, but it seems that stomp.on('error') should have worked the first time.

joshuarubin avatar Jun 20 '13 07:06 joshuarubin

This code is bad, and I feel bad :(

I've been on vacation, but I'm starting to get back into the swing of things. I'll look into refactoring this code, and hopefully addressing this in a nicer way, as soon as possible.

Thanks for your report, and if you'd like to take a stab at cleaning this up, feel free!

benjaminws avatar Jun 24 '13 18:06 benjaminws

Thanks for the update!

joshuarubin avatar Jun 24 '13 20:06 joshuarubin