easternbloc / node-stomp-client

A STOMP client for Node.js
Other
93 stars 47 forks source link

Messages published multiple times #59

Closed felimartina closed 7 years ago

felimartina commented 8 years ago

Hi! we built a wrapper over the stomp-client. We basically want to provide a onMessage and putMessage capabilities. This is what our code looks like

var Stomp = require('stomp-client');
module.exports = function (config) {
  var client = new Stomp(config.address, config.port, config.user, config.pass);
  return {
    onMessage: function (onMessage) {
      client.connect(function (sessionId) {
        client.subscribe(config.queueName, onMessage);
      });
    },
    putMessage: function (message) {
      client.connect(function (sessionId) {
        client.publish(config.queueName, message);
      });
    }
  }
}

Problem we are having is that often times the line client.publish(config.queueName, message); is called a random amount of times...yes...random. This is:

  1. We receive a request in our API
  2. We do something
  3. Call putMessage function

When I debug the line client.publish(config.queueName, message); sometimes is called like up to 10 times. Do any of you have an idea of what may be happening? I probably will be trying a different stomp-client to see its behavior. Really weird and odd to me.

I know that from the other side of the queue there is something subscribed that will convert messages into files and do something with them...but problem seems to be in this side of the queue (publish) since when I debug I can see the code going through it multiple times for one single request.

any thoughts?

macklevine commented 8 years ago

The issue may have something to do with the fact that you're creating a new connection each time you want to publish a message to the queue. I'd suggest creating and using a single connection instead.

felimartina commented 8 years ago

It might, but because of the a-synchronicity in NodeJS we had to use it that way (that's what I think at least) We solved the problem by using a different NodeJS client: stompit. What I like about this other library is that it is implemented using standard err/callback functions so that you can easily promisify it.

macklevine commented 8 years ago

You don't have to use the client that way at all. I've used stompit as well but if you decide to switch back to this client for any reason, here's a modification to your wrapper that should fix the problem:

'use strict';

var Stomp = require('stomp-client');

module.exports = function (config) {
  var client = new Stomp(config.address, config.port, config.user, config.pass);
  return new Promise(function(resolve, reject){
    var failureCallback = function(err){
      if(err.reconnectionFailed){
        reject("could not connect");
      } else {
        reject(err);
      }
    }
    client.once('error', failureCallback);
    client.connect(function(sessionId){
      client.removeListener('error', failureCallback);
      //add other listeners for errors here if you want
      resolve({
        onMessage: function (onMessage) {
          client.subscribe(config.queueName, onMessage);
        },
        putMessage: function (message) {
          client.publish(config.queueName, message);
        }
      });
    });
  });
};

Require the module, invoke it with a config object, and when the promise it returns resolves, it's ready to use. You can get access to the wrapper API in the callback of the then() block.

macklevine commented 8 years ago

As for why your original wrapper was invoking client.publish() multiple times, take a look at StompClient.prototype.connect (lines 104-154 of client.js). Every time you invoke the putMessage method of your wrapper, you add another callback as a listener to the 'connect' event:

  if (connectedCallback) {
    self.on('connect', connectedCallback);
  }

This is why client.publish() ends up being invoked multiple times--it's in the body of each callback you add as a listener every time you use your wrapper to open another connection with the same instance of StompClient.

easternbloc commented 7 years ago

@macklevine thanks for helping the issuer of this out. Closing due to aforementioned.