-
Notifications
You must be signed in to change notification settings - Fork 234
Description
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 receive123
- 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 receive123
- For message
[RESULT, CALL.Request|id, Details|dict, [123], { more: true }]
→ client side will receive an instance ofautobahn.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 receivenull
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.