wsdl-tsclient icon indicating copy to clipboard operation
wsdl-tsclient copied to clipboard

Stack limit too low for giving unique names to elements with same name.

Open zengatsu opened this issue 3 years ago • 19 comments

~~This is the same issue with wsdl-to-ts. They both can't handle recursive definitions.~~

edit: MAX_STACK constant that defines the max iterations to find a non colliding name before giving up, is set to only 30 while in my example there were hundreds of elements with the name "result". I chose a random high number that worked for me.

I will send a PR if I think about a way to find an optimal value for it.

zengatsu avatar May 12 '21 18:05 zengatsu

Hi, could you please provide example wsdl file ? I tried to do my best to avoid to handle recursive definitions - some of them works....

Edit: And error log pls

dderevjanik avatar May 12 '21 19:05 dderevjanik

I have a good one here:

sm_Synccom_senior_g5_rh_sm_historicos.wsdl.zip

renanwilliam avatar May 12 '21 19:05 renanwilliam

@renanwilliam thank you!

I tried it and I'm getting an error from node-soap:

Error: getaddrinfo ENOTFOUND someserver.in.the.web

Do you have any wsdl with public endpoint ? And what exactly recursive problem means ? Which kind of error are you getting ?

dderevjanik avatar May 12 '21 20:05 dderevjanik

No, I don't.. but I don't know if we are talking about the same issue... For me is more related to ComplexTypes. Probably I changing the issue propose (my fail)

renanwilliam avatar May 12 '21 20:05 renanwilliam

@renanwilliam Yes, that's different topic, it should be in another issue. ComplexType are right now.... complex.

dderevjanik avatar May 12 '21 21:05 dderevjanik

I tried to find some public wsdl with the same problem, couldn't find any!

I'm sorry I know that it would be more helpful if I posted the wsdl, But I can't as I might get in trouble,

The Error I'm getting is: Error occured while generating client "wsdl" Error: Error while parsing Subdefinition for tns:getXXXTypesResponse.tns:getXXXTypesResponse 1 Errors occured!

As you can see tns:getXXXTypesResponse is exactly the same and it's nested, I guess that's what a recursive definition is right?

I also think it's a recursive definition because wsdl-to-ts gives a call stack error, but I'm not 100% sure.

zengatsu avatar May 14 '21 00:05 zengatsu

After looking at the wsdl more, I have another theory. Is it possible that the parser gets confused when an element has the same name as the type? could that happen because I noticed that in this wsdl all ements are like this:

<element name="getXXXTypesResponse" type="tns:getXXXTypesResponse"/>

Could this be the problem and not my first assumption?

edit: I think I can confirm that the issue is caused by using the same name. After renaming the elements where it crashes it passes to the next one, But there are many to be renamed, and I'm still not sure what will the consequences of this be :( I don't get this error when using Apache CXF, so hopefully this can be fixed!

edit2: I cloned the code and logged the error raised by parser.ts:51 nonCollisionDefName = parsedWsdl.findNonCollisionDefinitionName(defName); output : Error: Out of stack (30) for "Result", there's probably cyclic definition So, I'm not sure anymore! could the issue be because there is an element "result" with different types inside all the response types? :thinking:

edit3: By increasing the MAX_STACK I was able to parse the wsdl without any errors and the result seems ok. I still have to test it but it looks good so far.

zengatsu avatar May 14 '21 01:05 zengatsu

@zengatsu yes, you're with problem when there are several types with same name. To referencing structures, e.g. ProductItem between different types, I ended up with checking if similar type with name/structure exists.
But to avoid circular references, I used MAX_STACK.

Maybe it would be good idea to increase MAX_STACK and also to have possibility to pass it as argument to CLI. Maybe also some kind of warning if there are too many references.

PS: I was inspired by wsimport, which is always increasing suffix number of type with same name

dderevjanik avatar Jun 21 '21 16:06 dderevjanik

Hey @dderevjanik I wish I had more time and knowledge of wsdl to help with this. I am working on something else at the moment and will get back to this later, for now I will provide what I've learned.

So, my problem was with elements that have the same name for example "result" so I get many suffixed "result" types.

If you look at the definitions bellow, the expected behavior (which can be seen when parsing in PHP and JAVA) is to get classes/interfaces :

interface getProductsResponse {
  result?: GetProductsRS;
}

interface GetProductsRS {
  result?: ProductsResult;
  totalProducts: int;
}

but instead what we get is :

interface result1 {
  result?: result1;
}

interface result1 {
  result?: result1; 
  totalProducts: int;
}
<complexType name="getProductsResponse">
  <sequence>
    <element name="result" type="tns:GetProductsRS" nillable="true" />
  </sequence>
</complexType>
<complexType name="GetProductsRS">
  <sequence>
    <element name="result" type="tns:ProductsResult" nillable="true" minOccurs="0"/>
    <element name="totalProducts" type="int" />
  </sequence>
</complexType> 

So the issue is that the attribute name is used for the type instead of the type (it only get the right type for methods but not for definitions) maybe that's a limitation of soap package because I looked into this and couldn't find a way to get the type information with the definitions. My knowledge of how this is parsed is limited so I'm not sure why.

If we don't care about the type names and decide to continue using it like this there is the issue of it giving the same suffix to results under the same complex type, or something like this because as you can see in the example above I get 2 types with the same name and suffix.

zengatsu avatar Jun 21 '21 18:06 zengatsu

Thank you very much for detailed report !

The biggest issue I have with generating wsdl client is requirement to use runtime package node-soap. So, wsdl-tsclient is using node-soap to parse wsdl files in order to make it works same in runtime. Because of that, I'm very limited with parsed node-soap types - node-soap is loosing a lot of information, names too :/ . Same issue with enums, type restrictions and additional information (min, max size of string/array).

For now, I can resolve this issue by increasing MAX_STACK, adding additional option to CLI and warning user if there are so many types with same name. Then creating another issue with parsing/generating proper name. The ultimate idea is to fork node-soap and provide more data from wsdl files.

What do you think ?

dderevjanik avatar Jun 21 '21 18:06 dderevjanik

Yeah I have noticed that the limitation is caused by node-soap, I don't see any other way than patching node-soap to get the type information with the definitions. Let's investigate the feasibility of doing this, and see what we can do.

zengatsu avatar Jun 21 '21 22:06 zengatsu

Sure, I already started with patching node-soap . There's forked project node-soap https://github.com/dderevjanik/node-soap with support of Documentation element (generated wsdl-tsclient includes jsdoc from wsdl documentation elements).

dderevjanik avatar Jun 22 '21 09:06 dderevjanik

@renanwilliam please, could you try again parse your wsdl ? There's new version 1.2.0 https://github.com/dderevjanik/wsdl-tsclient/releases/tag/1.2.0 . You can increase MAX_STACK by passing --maxRecursiveDefinitionName to wsdl-tsclient, e.g. wsdl-tsclient ./Human_Resources.wsdl --maxRecursiveDefinitionName=90 -o ./generated

cc @zengatsu

dderevjanik avatar Jun 26 '21 08:06 dderevjanik

Hello @dderevjanik

I have tested with --caseInsensitiveNames=true and --maxRecursiveDefinitionName=100 and it works.

when the max_stack is lower than needed I don't get the expected error, but instead I get Error occured while generating client "test.wsdl" Error: Error while parsing Subdefinition for ... though I only tested directly by running the cli.js, but I don't think there is a difference, I will test again later.

There is only one issue left. When child and parent have the same type the same file is imported in itself which causes a conflict.

import { User } from "./User";

export interface User {
    /** User[] */
    user?: Array<User>;
}

zengatsu avatar Jul 03 '21 17:07 zengatsu

@zengatsu fixed in #39

dderevjanik avatar Apr 18 '22 14:04 dderevjanik

@dderevjanik Great work on this package! I was using ts-morph on it's own which became a nightmare really quickly. This package is about to save me few days headache, but I now have a similar problem as stated in this issue. (P.S I am on V1.4.0)

The WSDL file were converted just fine before I discovered a name/import clash. I have screenshots here of the part of the WSDL in question as well as the generated TS files here.

Though I am not very good with WSDL, it seems the binding and the service have the same name, hence, the clash. Please, how can I fix this? Thanks

WSDL: image

TS file in /ports folder: image

TS file in /services folder: image

dave-ok avatar May 19 '22 15:05 dave-ok

Hi ! I have the exact same problem as @dave-ok. I have a service name WebPaymentApi that imports a port with the same WebPaymentApi name.

In the generated index.ts file, I get the an obvious type clash as both interfaces are exported :

image

How can I fix this issue ? Can it be bypasses somehow ?

Clovel avatar Oct 04 '22 19:10 Clovel

As a workaround, you can rename the last import as:

export { WebPaymentApi as WebPaymentApiPort } from './ports/WebPaymentApi';

You will need to find and rename de occurrences also.

renanwilliam avatar Oct 04 '22 19:10 renanwilliam

As a workaround, you can rename the last import as:


export { WebPaymentApi as WebPaymentApiPort } from './ports/WebPaymentApi';

You will need to find and rename de occurrences also.

That would work for a temporary fix in one's development environment but as the generated code isn't version controlled, the fix would have to be scripted and called by CI/CD jobs and applied by new developers when they clone and generate the API definitions. Each new generation would require running the script afterwards.

This could be done using a node script or something like that, but should be implemented is this package.

Can it be easily done ? And if so, would it be merged quickly ?

Clovel avatar Oct 04 '22 19:10 Clovel