-
Notifications
You must be signed in to change notification settings - Fork 231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wrapping/unwrapping invocation results #570
Comments
yep. the spec if pretty clear https://wamp-proto.org/wamp_bp_latest_ietf.html#name-result-2
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: 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:
eg PL/SQL supports the full set, but sadly Python doesn't .. invalid syntax roughly like this:
|
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)) |
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 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 If it was for me I would always make the client return a RESULT which includes |
Yeah! Lack of details in call results is an important issue. {
details
argsList
argsDict
} |
you can, just enable it using 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) |
in ABPy, this is already implemented: in ABJS: because nobody asked until now;) seriously, you are right. we could add that (as it is for registering procedures or event handlers):
autobahn-js/packages/autobahn/lib/session.js Line 1205 in 562974e
autobahn-js/packages/autobahn/lib/session.js Line 683 in 562974e
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) |
@oberstet 😄 . I do like that! |
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:
[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 ofResult
is not wrapped into an array but is putas-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 plain123
number as result, but not an array. And only if wamp arguments array length is 1.For example:
[RESULT, CALL.Request|id, Details|dict, [123]]
→ client side will receive123
[RESULT, CALL.Request|id, Details|dict, [123, 456]]
→ client side will receive[123, 456]
in args attribute of autobahn.Result instanceThis 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.
[RESULT, CALL.Request|id, Details|dict, [123]]
→ client side will receive123
[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
: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![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.
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 a123
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.
The text was updated successfully, but these errors were encountered: