Problem with ari-client : sendMessage about node-ari-client HOT 10 OPEN

After much struggling with this I found the following solution:
If "variables" are passed, or at least the variables argument exists, then the "body" argument is ignored, (perhaps destroyed). So to get a message "body" do one of two things:

1. Do not provide any 'variables' argument.
2. If you need the variables to provide your own headers etc. Include the body content inside the variables argument. eg:

var variables = { "body":"My Message Content",
"variables": {"Event":"myowneventname"}
};

ari.endpoints.sendMessage( {from:"from",
to:"techprefix:to",
body: "this gets ignored",
variables: variables,
},function (err){});

Hope it helps. Just as an aside, we use sendMessage within ARI to generate out of dialogue NOTIFYs. We create our own tech c module as an extension of message.c and adapt some of the header handling, but it seems a good solution where no simple alternative seems to exist.

from node-ari-client.

Thanks for researching this. This looks like a bug. I believe it's due to another API parameter called variables for channel variables requiring the body type and special handling we have for that parameter. I should be able to get a PR that handles both situations correctly.

from node-ari-client.

probably related to this function? https://github.com/asterisk/node-ari-client/blob/master/lib/utils.js#L26

from node-ari-client.

I dug into the Asterisk source code a bit to understand this more

for reference, the API endpoint of this method is

PUT /endpoints/{tech}/{resource}/sendMessage

in ast_ari_endpoints_send_message_to_endpoint_cb() of res/res_ari_endpoints.c, we are setting from and body from the "params" (query string i think?) and tech and resource from the path, as expected. Then below that, it attempts to parse any JSON passed in the request body, and puts that in the variables property of the args.

then in ast_ari_endpoints_send_message() of res/ari/resource_endpoints.c, it checks to see if the args->variables is set. Remember, at this point, this is the entire request body. If it is set, it parses the request body (with ast_ari_endpoints_send_message_parse_body()) and overwrites any to, from, and body passed on the query string with the values set in the request body, if they exist. After that, it grabs the variables property out of the request body, and calls json_to_ast_variables() in an if statement, returning if the function returns a non-zero result. Assuming everything is okay up to this point, it dispatches the params it now holds in args to send_message() which handles actually sending the SIP message.

Anyway, I still need to dig into the code here in node-ari-client, but at least we know now what the Asterisk side is doing on this specific call...

from node-ari-client.

Thanks chadxz, that seems to fall in with our findings. Hopefully any fix won't break the workaround.

from node-ari-client.

Inside the version of the swagger-client that we are using (v2.0.26), there is some code that prevents a request from having a query parameter named body, which is what this request and a few other ARI requests expect. I'm pretty sure this was considered a bug in that library, because in later versions of the library it looks like it was fixed. However, those later versions heavily refactored the internals of the library, which is the reason that we're currently pinned to the version that we are on. You can see more details about that in #47.

While looking at this, I also noticed that when the variables param is not set and the body param is set when you call sendMessageToEndpoint(), the request has its body set to the value of the body param, even though its paramType is query. This is due to the same bug as above - body is never treated as a queryParam by swagger-client, only as a literal json body to be passed on the request. For the example given by @bof22 above, this would actually result in invalid JSON, since only a string was passed.

Things got even worse when I looked at the example of how @robgamlin got this to actually work. Our code in parseBodyParams is setting the body option to the entire value of the param that is passed that has a paramType of 'body'. For the request in question, the only param that has this type is variables. So whatever you set as the value for variables is what ends up in the body.

This behavior is just wrong. The swagger paramType of 'body' is meant to indicate that the property should be a part of the body, not that the property should have its value set as the entire body. We have coded in a few exceptions, one specifically for the variables, which causes the variables to be set to the body's variables param, but instead of augmenting any existing request body, it overwrites it with an entirely new object. So regardless, this is not the behavior that we really want.

So to actually fix this bug, I think we have these options:

Option 1:

• fork swagger-client v2.0.26 and change the SwaggerOperation.do() method's parameter parsing logic such that it parses the request body based on the paramType and not by just blindly looking for a 'body' key in the passed options.
• remove parseBodyParams, as the above work would negate the need for it

or

Option 2:

• pull in the latest swagger-client, and update this library to work with it. The latest swagger client already has the updated logic mentioned above to only set body params based on the paramType of the parameters, instead of looking for a 'body' key in the passed options.

In my opinion, neither of these options is that great. I've already tried Option 2 and after a few hours of work, I felt like I was digging myself into a hole. Option 1 is also not compelling, because it would be digging the hole of being even more stuck on an outdated dependency.

For me, this is yet another push in the direction of just dropping the swagger-client dependency completely, and instead performing the network requests ourselves.

from node-ari-client.

Seeing as there's a workaround, it doesn't feel like we should go with option 1 at this time until we encounter a scenario that cannot be resolved otherwise. Option 2 feels like a waste of time at this point.

What I would like to see is us carving out some time to rip swagger code gen out of this project. I don't think it's a trivial change, but nor should it take months to accomplish.

from node-ari-client.

Reading this thread makes my head hurt!!

I hope I could get someone to verify that the behavior I see is "to be expected" following the logic above: You cannot send a body parameter along with the request, but must pass in a variables parameter which is an object that must contain both the body parameter and some value for variables?

That's what it looks like to me, but my brain is pudding.

from node-ari-client.

Put in your terms capcouch:
If you need 'variables', then specify a 'body' inside the variables object.
'body' is ok outside variables, but will be ignored if a 'variables' object is included.

from node-ari-client.

Was not able to get it to go using body with or without variables. Works only if I omit body but put body content and some value for variables inside variables parm. This seems to be somewhat verified by the first paragraph in the third post by @chadxz above.

It's all good and working that way for me, so the details at this point are academic :-)

from node-ari-client.