elsa-workflows / elsa-core

A .NET workflows library
https://v3.elsaworkflows.io/
MIT License
6.23k stars 1.13k forks source link

[BUG] Issues with POST and GET Execute APIs #5553

Closed cvijayak closed 3 weeks ago

cvijayak commented 3 months ago

POST : Execute API The POST /workflow-definitions/{definitionId}/execute endpoint in Swagger is not showing a text box for accepting the request body.

image

It looks like the highlighted line below in the Elsa core is causing the problem of not showing the request body for the POST API. It seems the same handler is used for both GET and POST requests. Therefore, if we remove GET from the configuration, we will be able to see the request body for the POST API. A different handler needs to be written for the GET API to support both verbs.

image

To confirm, I changed the configuration and checked whether the POST API shows the request body. Yes, it shows properly. Please refer to the screenshot below for more details.

image

GET : Execute API Additionally, I am looking for some clarification on the GET /workflow-definitions/{definitionId}/execute endpoint. When I try to call the GET API as shown below, the API throws type-casting error.

https://<HOST>/api/workflow-definitions/bddbe8462d444632/execute?correlationId=34343222&versionOptions=Latest&input={"accountId": "23242267889"}

Please find the type-casting error in the screenshot below image

Please note that I am passing the 'input' as a query string parameter. As per my understanding, in the FastEndpoints framework, GET request DTO properties are automatically serialized from query string parameters by default.

So, I tried to send the 'input' in the query string like the other parameters, but it throws a type-casting error, stating that it is unable to cast the JSON object into a dictionary. This seems like a bug to me. Please confirm the same.

Highlighted line in the below screenshot throws the exception image

It looks like the JsonConverter defined in the model(shown in the screenshot below) is not getting invoked if we pass the 'input' through the query string. image

If the JsonConverter invoked properly, we will not see the casting issue.

glime-ai[bot] commented 3 months ago

Based on the context provided, it seems that there is a configuration issue in the Elsa Workflow API that is causing the Swagger UI not to display the request body text box for the POST /workflow-definitions/{definitionId}/execute endpoint. The issue appears to be related to the same handler being used for both GET and POST requests, which can be resolved by separating the handlers for each HTTP method or by adjusting the configuration to differentiate between them.

Regarding the GET /workflow-definitions/{definitionId}/execute endpoint, the error you are encountering when passing the 'input' as a query string parameter seems to be due to the inability of the FastEndpoints framework to deserialize the JSON object from the query string into a dictionary. This could be a limitation or a bug in the framework's handling of complex types in query strings. Typically, GET requests should not contain a body, and complex types are not well-suited for query string parameters due to encoding and length limitations.

To avoid these issues, it is recommended to use the POST method for executing workflows when you need to pass complex input data. This aligns with RESTful practices and ensures that the input data is properly handled by the API.

If you need to continue using the GET method for some reason, you may need to look into custom serialization and deserialization mechanisms or consult the FastEndpoints documentation or support channels for guidance on handling complex query string parameters.

For the POST method, you should ensure that the Swagger configuration correctly reflects the ability to accept a request body for the POST request. If necessary, you may need to update the Elsa Workflow API or its Swagger configuration to address this issue.

For the GET method, you may need to consult the FastEndpoints documentation or reach out to their support to understand how to properly pass complex types in query strings or to confirm if there is a bug related to the JsonConverter not being invoked.

This comment was generated by Glime.

cvijayak commented 2 months ago

@sfmskywalker Any updates on this?

The issue still there in 3.2 rc1

sfmskywalker commented 2 months ago

Hi @cvijayak , unfortunately I don’t have the bandwidth to work on this for inclusion with 3.2. However, if you want to propose a PR then maybe we could include it.

cvijayak commented 1 month ago

@sfmskywalker Sure, I will raise PR for the same

cvijayak commented 1 month ago

@sfmskywalker I have created a PR that fix the bug. Please review and approve the same https://github.com/elsa-workflows/elsa-core/pull/5858