Wrapping/unwrapping invocation results

[KSDaemon]

The context: I'm playing with different clients' interoperability.

Let's say we have a kind of API when all clients are registering an RPC and must return the invocation result in args wamp message attribute. Something like this [123].

The problem: autobahn.js is wrapping invocation result into 1-item array and put it into args. For example:

• If invocation handler returns [123], the underlying wamp message that will be sent over the wire will look something like: [YIELD, INVOCATION.Request|id, Options|dict, [[123]]]

This can be avoided if invocation handler returns an instance of autobahn.Result object. In that case, args attribute of Result is not wrapped into an array but is put as-is. It is a bit strange why in one case there is wrapping and missing in the other. I think at least this should be explicitly added to the docs.

But let's move on to processing the RESULT wamp message on the caller side. If autobahn.js client receives wamp message like [RESULT, CALL.Request|id, Details|dict, [123]], client code will get plain 123 number as result, but not an array. And only if wamp arguments array length is 1.

For example:

• For message [RESULT, CALL.Request|id, Details|dict, [123]] → client side will receive 123
• For message [RESULT, CALL.Request|id, Details|dict, [123, 456]] → client side will receive [123, 456] in args attribute of autobahn.Result instance

This means that the result data scheme is dependent on the argument's array length! Let's say we have a users list RPC held by non-autobahn-based peer. It always returns an array of users: empty, 1 user object, many users objects. But client caller running on autobahn will receive a call result as 1 user object in one case and array of user objects in other. But at the same time, if there are some kwargs keys in the message — even 1-item array will be returned as array and not unwrapped object.

• For message [RESULT, CALL.Request|id, Details|dict, [123]] → client side will receive 123
• For message [RESULT, CALL.Request|id, Details|dict, [123], { more: true }] → client side will receive an instance of autobahn.Result where args will be [123] (not extracted from the array)!

So client code can not rely on the same kind of results for all cases and instead must always check if the result is an instance of autobahn.Result or no.

My investigation led me to this code part in lib/session.js:

   self._process_RESULT = function (msg) {
      //
      // process RESULT reply to CALL
      //
      var request = msg[1];
      if (request in self._call_reqs) {

         var details = msg[2];

         var args = msg[3] || [];
         var kwargs = msg[4] || {};

         // maybe wrap complex result:
         var result = null;
         if (args.length > 1 || Object.keys(kwargs).length > 0) {  // <<<<< Result is returned only if args length is > 1 or there is at least 1 kwargs key.
            // wrap complex result is more than 1 positional result OR
            // non-empty keyword result
            result = new Result(args, kwargs);
         } else if (args.length > 0) {                             // <<<<<< If there is something in the args — it is returned as extracted from args array
            // single positional result
            result = args[0];
         }

Also the code above shows, that if received wamp message arguments list is an empty array, client code will get null and never an empty array!

• For message [RESULT, CALL.Request|id, Details|dict, []] → client side will receive null

Unfortunately, existing tests do not catch this because both sides (caller and callee) are autobahn-based, so wrapping/unwrapping is done on both sides. I'm not familiar with the nodeunit framework that is used for testing (and btw it is deprecated). And in any case to face this issue we need to dig deeper that public API... So your thoughts/help will be required.

I think the current behavior is at least strange but rather incorrect. If I assume to get an array — I should get it no matter how many elements are in it. Right?

The fix for this can be pretty simple — just do not wrap args, something like this. And btw, this is not done in pub/sub. If the client is passing not an array to args — autobahn throws an error! But here in the rpc module, no errors can be thrown, autobahn just packs anything into a 1-item array.

diff --git a/packages/autobahn/lib/session.js b/packages/autobahn/lib/session.js
index de352f4..bdefecd 100644
--- a/packages/autobahn/lib/session.js
+++ b/packages/autobahn/lib/session.js
@@ -682,13 +682,13 @@ var Session = function (socket, defer, onchallenge, on_user_error, on_internal_e
 
          // maybe wrap complex result:
          var result = null;
-         if (args.length > 1 || Object.keys(kwargs).length > 0) {
+         if (Object.keys(kwargs).length > 0) {
             // wrap complex result is more than 1 positional result OR
             // non-empty keyword result
             result = new Result(args, kwargs);
          } else if (args.length > 0) {
             // single positional result
-            result = args[0];
+            result = args;
          }
 
          var r = self._call_reqs[request];
@@ -807,7 +807,7 @@ var Session = function (socket, defer, onchallenge, on_user_error, on_internal_e
                      }
                   }
                } else {
-                  reply.push([res]);
+                  reply.push(res);
                }
 
                // send WAMP message

The important question here is whether it changes the current behavior and is not backward compatible. So the clients that rely on 1-item results instead of array — will fail and need to be adopted. I think this is not a problem if the other side is not an autobahn, but if it is — then it is a problem.

I understand the root cause of this: to be able to seamlessly pass scalar values: the peer sends a number — the receiver gets exactly that number. But WAMP Spec defines that arguments MUST BE an array and even single number must be packed into an array. That is the easy part. The hard is to understand whether [123] is just a 123 number packed into array or it is originally an array and should be delivered to the receiver as is...

I don't think we have the right to allow sending single values in wamp messages as WAMP is pretty stable for a long time and there are plenty of implementations that rely on arguments as a list. But also we do not have anywhere in the SPEC description that 1-item wamp message arguments should be unwrapped before returning to client code. So this leads to kind of interop issues and different clients (i mean end-user code that uses wamp implementations) will get different results for the same WAMP messages. Of course, different languages and implementations have their own approaches and paradigms, but in any case, I think it is important to be able to receive results in a way they were sent by the initiator peer.

[oberstet]

I don't think we have the right to allow sending single values in wamp messages as WAMP is pretty stable for a long time and there are plenty of implementations that rely on arguments as a list.

yep. the spec if pretty clear https://wamp-proto.org/wamp_bp_latest_ietf.html#name-result-2

But also we do not have anywhere in the SPEC description that 1-item wamp message arguments should be unwrapped before returning to client code.

yep. because that's up to an implementation to decide, and the spec only defines the wire protocol.

also note, that ABPy does exactly the same:

https://github.com/crossbario/autobahn-python/blob/f77396401d52a5284c3506171cf45eb64505804f/autobahn/wamp/protocol.py#L961

---

implementations can expose WAMP args/kwargs in different ways, as they see fit the host language and run-time environment.

you can say this is annoying (what is the canonical interface in host language X for WAMP?) or you can say this is welcome because there is no one best agreed canonical interface .. it depends. fwiw, I support the latter.

the reason why the question comes up anyways is:

• WAMP consequently supports both positional and keyword based parameters and return values.
• some host languages only support positional parameters and 1-positional return types
• some host languages only support positional and keyword parameters and 1-positional return types
• only very few host languages natively support all of it: positional and keyword for both parameters and return types

eg PL/SQL supports the full set, but sadly Python doesn't .. invalid syntax roughly like this:

def foo(x):
    if x > 0:
        return 2 * x, 3 * x, c = "x={}".format(x)
    elif x == 0:
        return 0, 23
    else:
        return 42, 5 * x, d = "hello world"

for i in [-1, 0, 1]:
    a, b, c: None, d: None = foo(i)
    print(a, b, c, d)

[oberstet]

just checked, I think the way ABPy works and exposes this stuff is "good" (the best we can do == the least surprising to the user):

from autobahn.wamp.types import CallResult

# returns both positional and keyword results filled from call arguments
def echo(*args, **kwargs):
    return CallResult(*args, **kwargs)

print(echo(2, 3, b = 4))

# returns a single positional result which is one list with all positional call arguments
def echo(*args):
    return args

print(echo(2, 3))

# returns zero or more positional results filled with positional call arguments
def echo(*args):
    return CallResult(*args)

print(echo(2, 3))

# same as 2nd!
def echo(*args):
    return CallResult(args)

print(echo(2, 3))

# returns zero or more keyword results filled with keyword call arguments
def echo(**kwargs):
    return CallResult(**kwargs)

print(echo(a=2, b=3))

# returns one positional result which is map with keyword call arguments
def echo(**kwargs):
    return kwargs

print(echo(a=2, b=3))

[aramallo]

My two cents, I agree with @oberstet that this might not be something the specification should prescribe, but I agree with @KSDaemon that it would be best for all clients to agree on a best practice.

The above ABPy code above exposes another issue, why isn't the RESULT.Details returned to the user in AB?

Same in AB|JS:

var Result = function (args, kwargs) {

   var self = this;

   self.args = args || [];
   self.kwargs = kwargs || {};
};

Another example in AB|JS:

var Event = function (publication,
                      topic,
                      publisher,
                      publisher_authid,
                      publisher_authrole,
                      retained,
                      forward_for) {

   var self = this;

   self.publication = publication;
   self.topic = topic;
   self.publisher = publisher;
   self.publisher_authid = publisher_authid;
   self.publisher_authrole = publisher_authrole;
   self.retained = retained;
   self.forward_for = forward_for;
};

In this case we are hardcoding the Publisher Identification fields, which means any other key in EVENT.Details will be ignored right? This limits extensibility (which brings me back to the _{custom_attr} discussion) but also means the we need to re-implement client code when the WAMP Spec itself blesses a new attribute.

If it was for me I would always make the client return a RESULT which includes args, kwargs and details respecting the best practices for the host language. .e.g object | tuple | struct containing those keys when only single return value is allowed.

[KSDaemon]

Yeah! Lack of details in call results is an important issue. In Wampy.js for example, in all cases (event, result) I return all of them:

{
         details
         argsList
         argsDict
}

[oberstet]

The above ABPy code above exposes another issue, why isn't the RESULT.Details returned to the user in AB?

you can, just enable it using RegisterOptions and SubscribeOptions

https://github.com/crossbario/autobahn-python/blob/f77396401d52a5284c3506171cf45eb64505804f/autobahn/wamp/types.py#L809 - there are 2 flavors (just enable will use "details" as keyword arg, and correctly pop off details before forwarding kwargs - or you can specify the name for details, so that it doesn't collide with your own details in kwargs)

[oberstet]

@aramallo

The above ABPy code above exposes another issue, why isn't the RESULT.Details returned to the user in AB?

in ABPy, this is already implemented:

https://github.com/crossbario/autobahn-python/blob/f77396401d52a5284c3506171cf45eb64505804f/autobahn/wamp/protocol.py#L943

in ABJS: because nobody asked until now;) seriously, you are right. we could add that (as it is for registering procedures or event handlers):

1. add a client-side option to enable

https://github.com/crossbario/autobahn-js/blob/562974e8088689687f7f73f10ad4696d4c4f838a/packages/autobahn/lib/session.js#L1205

2. and if enabled, always return a complex result with details filled in

https://github.com/crossbario/autobahn-js/blob/562974e8088689687f7f73f10ad4696d4c4f838a/packages/autobahn/lib/session.js#L683

when enabling, it must always return a complex result, even if the RESULT has only 1 positional result (because JS only allow 1 result in functions)

[aramallo]

@oberstet 😄 . I do like that!