Remove brackets from argsScheme string if they exist #929
Conversation
We have an argsScheme issue due its strign contains brackets. Remeving it does not affect existing tests ans solves the problem
test/sequence-test.js
Outdated
| _ = require('lodash'), | ||
| sinon = require('sinon'), | ||
| wsdl = require('../lib/wsdl'); | ||
| soap = require('..'), |
There was a problem hiding this comment.
please remove any changes that aren't related to your fix.
Reverting unrelated changes made by beautify
3 similar comments
3 similar comments
3 similar comments
| assert.ok(JSON.stringify(sequencedMethodRequest) === JSON.stringify(sequencedRequest)); | ||
| done(); | ||
| }); | ||
| it('check sort args on sequence required method', function (done) { |
| }); | ||
| describe('Method args sequence', function () { | ||
|
|
||
| it('check if method required sequence args', function (done) { |
There was a problem hiding this comment.
These test was already there
My test start with should, by the way
| @@ -212,6 +212,7 @@ Client.prototype._setSequenceArgs = function(argsScheme, args) { | |||
| return args; | |||
| } | |||
| for (var partIndex in argsScheme) { | |||
| partIndex = partIndex.replace('[]','') | |||
There was a problem hiding this comment.
This seems like a pretty big breaking change to me. Just because there aren't tests, doesn't mean it's good for use. What are we trying to solve here?
There was a problem hiding this comment.
Well, I agree it's a very specific issue, then i forked and create my own npm package to solve urgent my issue, but I decided to pull request it anyway.
I think it's a error with the SOAP server, but I have an argScheme with brackets while it should be just a string. If I try to pass the arg with the bracket the application crashes
There was a problem hiding this comment.
If it's such a specific scenario I guess we should guard it behind some option? I know it won't be an elegant solution, but it would fit both needs if the option will trigger the newly introduced behaviour and proceed "as is" if not provided...?
There was a problem hiding this comment.
we could end up breaking things for devs that depend on the brackets being there, so I think an option would be good.
There was a problem hiding this comment.
I tried to use with the brackets, but I had an error with parse response:
Error: Error: Cannot parse response
at kplClient.CadastrarCliente (src/connectors/applications/kpl/services/kplService.js:35:25)
at node_modules/soap/lib/client.js:180:7
at finish (node_modules/soap/lib/client.js:451:14)
at parseSync (node_modules/soap/lib/client.js:436:12)
at node_modules/soap/lib/client.js:412:14
at Request._callback (node_modules/soap/lib/http.js:118:5)
at Request.self.callback (node_modules/request/request.js:188:22)
at Request.<anonymous> (node_modules/request/request.js:1171:10)
at IncomingMessage.<anonymous> (node_modules/request/request.js:1091:12)
at endReadableNT (_stream_readable.js:975:12)
at _combinedTickCallback (internal/process/next_tick.js:80:11)
at process._tickCallback (internal/process/next_tick.js:104:9)
There was a problem hiding this comment.
sounds like you have a bug in your code
There was a problem hiding this comment.
It happens when I try to use the string with the brackets. I don't know if the field can have brackets at all, but in my case it cannot.
If i use the field without brackets, the field does not come to my args, due its difference to the field from the Scheme, with the brackets.
There was a problem hiding this comment.
@alanmarcell is it possible for you to share the source code of your kplService.js? The client.parseSync() is "just" a proxy to wsdl.xmlToObject() with a try/catch block around it. So, if anything, the xmlToObject() method would fail.
There was a problem hiding this comment.
Yes, of course
'use strict';
let
soap = require('soap');
class KplService {
createConnection(url) {
return new Promise((resolve, reject) => {
soap.createClient(url, (err, client) => {
if (err) {
return reject(new Error(err));
}
return resolve(client);
});
});
}
createClient(kplClient, args) {
return new Promise((resolve, reject) => {
kplClient.CadastrarCliente(args, (err, response) => {
if (err) {
return reject(new Error(err));
}
if (response.CadastrarClienteResult.ResultadoOperacao.Codigo !== 200002) {
return reject(new Error(response.CadastrarClienteResult.ResultadoOperacao.ExceptionMessage));
}
return resolve(response.CadastrarClienteResult.Rows);
});
});
}
createOrder(kplClient, args) {
return new Promise((resolve, reject) => {
kplClient.InserirPedido(args, (err, response) => {
if (err) {
return reject(new Error(err));
}
if (response.InserirPedidoResult.ResultadoOperacao.Codigo !== 200001) {
return reject(new Error(response.InserirPedidoResult.ResultadoOperacao.ExceptionMessage));
}
return resolve(response.InserirPedidoResult.Rows);
});
});
}
}
There was a problem hiding this comment.
I call the service this way:
[...]
const kplClient = await KplService.createConnection(url)
args.ListaDeClientes = {
'DadosClientes': DadosClientes
}
// I tried this way:
// args.ListaDeClientes = {
// 'DadosClientes[]': DadosClientes
// }
// And this way:
// args.ListaDeClientes = [{
// 'DadosClientes': DadosClientes
// }]
// With no success
let res = await KplService.createClient(kplClient, args)
[...]|
This is probably (or better to say, I'm sure it is) describing what the failing test added with #933 describes. |
|
@alanmarcell would you mind checkin' out the |
|
@alanmarcell would you mind trying the new |
|
Hi everyone I want to know what object should I use to send?
I use : Thank you |
We have an argsScheme issue due its string contains brackets. Removing it does not affect existing tests ans solves the problem