kaazing / javascript.client

Apache License 2.0
21 stars 16 forks source link

Cannot provide error callbacks - `_enqueueAction(...)` is never passed an error callback #9

Closed ZachBray closed 9 years ago

ZachBray commented 9 years ago

I was investigating how to provide an error callback for functions like channel.bindQueue(...) because it isn't specified clearly in the jsdoc and I stumbled across the following issue.

AmqpChannel.js makes use of _enqueueAction(...) in many places. For example:

  AmqpChannel.prototype.bindQueue = function(
   queue,exchange,routingKey,noWait,arguments, callback) {
      var args = [ 0  , queue  , exchange  , routingKey  , noWait  , arguments   ];
      var methodName = 'bindQueue';
      var amqpMethod = _methodsByName[methodName];

      // TODO
      // This is a temporary workaround to get the value of the noack flag
      //   without knowing up front if the method has a noack parameter.
      //
      // This logic can be removed if a noack field is added to the codegen model
      var hasnowait = false;
          for (var i=0; i<amqpMethod.allParameters.length; i++) {
              var argname = amqpMethod.allParameters[i].name;
              if (argname = "noWait") {
                  hasnowait = true;
                  break;
              }
          }

      if(this._connection._readyState == this._connection.OPEN)
          this._enqueueAction(methodName, _channelWrite, [this, amqpMethod, this._id, args], callback);

      // need for conditional insertion of code while using string template
      // Below if condition should be added for funtions like declareExchange(), declareQueue(), bindQueue() and consumeBasic()
      // As of now they are added in all functions due to string template

      if (methodName == "flowChannel") {
          AmqpChannel.prototype.flowFlag = active;
      }

      // if there is a nowait param, and it is true, we need to get out of the waiting state
      if (hasnowait) {
          if (typeof(noWait) !== "undefined" && noWait) {
              this._enqueueAction("nowait");
          }
      }

      return this;
  };

However, it never passes more than 4 arguments to _enqueueAction(...), which means the defaultErrorCallable callback will always be used.

_prototype._enqueueAction = function _enqueueAction(actionName, func, args, continuation, error) {
    var action = {};
    action.actionName = actionName || "";
    action.func = func || nullCallable;
    action.args = args || null;
    action.continuation = continuation || nullCallable;
    action.error = error || defaultErrorCallable;

    this._actions.push(action);

    var context = this;
    var func = function(){context._processActions();};
    setTimeout(func,0);
};

In the guide it looks like the writer was expecting to be able to pass an error callback into these methods. For example:

channel.bindQueue(queueName, exchangeName, routingkey, noWait, null, function(e) { log(output, exchangeName + " bound");}, errorHandler(output));

I would expect to be able to provide an error callback parameter or for my callback parameter to be invoked with an error.

sbadugu commented 9 years ago

@pkhanal @dpwspoon @cmebarrow Should we prioritize this ?

pkhanal commented 9 years ago

@sanjay-saxena can provide more input on this issue.

sanjay-saxena commented 9 years ago

@sbadugu -- Without spending any significant time investigating this issue, it seems like a bug. I will let the PMs decide whether it should be prioritized.

sbadugu commented 9 years ago

@sanjay-saxena Let's bring this in our sprint planning meeting and see if we can prioritize this for next sprint.

sanjay-saxena commented 9 years ago

Note that AMQP 0-9-1 Javascript API does not specify any parameter for passing an error-handler into bindQueue() function which is invoked like this:

var channel = ...;
var config = {queue: myQueueName, 
                     exchange: myExchangeName,
                    routingKey: key};
 channel.bindQueue(config, callback);

And, here is some context:

Long time ago, we had decided to not support passing in an error-handler explicitly into each of our publicly exposed Javascript API functions. Instead, we had decided to support a listener-based mechanism where an application can register an error-handler on the channel object like this:

channel.addEventListener("error", function(e) {
    log("CHANNEL ERROR: " + e.message);
 });

AMQP Javascript Client HowTo shows how to build an app using AMQP JS client library.