camunda / camunda-bpm-platform

Flexible framework for workflow and decision automation with BPMN and DMN. Integration with Quarkus, Spring, Spring Boot, CDI.
https://camunda.com/
Apache License 2.0
4.03k stars 1.53k forks source link

Camunda Http Connector Enhancement for 4XX and 5XX errors #4241

Open Nandanrshenoy opened 3 months ago

Nandanrshenoy commented 3 months ago

User Story

Current HTTP connector does not throw an error for 4XX and 5XX errors The errors needs to be explicitly handled by the modeler through an inline script as shown below.

exception_handling Connector_Screenshot

Functional Requirements (Required before implementation)

An incident needs to be created when ever HTTP connector throws an error for 4XX and 5XX errors

Technical Requirements (Required before implementation)

Since this code of handling HTTP response is repeatable and reusable, it would be good to modify default behavior of http connector to throw a bpmnError in case of API call failures so that modeler can handle them as desired.

This can be made configurable through an engine setting by adding a new flag called throwErrorForHttpConnector. By default, this flag would be false, where modeler would have to explicitly handle such Errors. If marked true, then the feature that is being built to handle such errors should be enabled.

Limitations of Scope

The scope of this fix is limited to just http connectors

Hints

Links

The following camunda Http and Soap related connector code needs to be modified: https://github.com/camunda/camunda-connect

Breakdown

Dev2QA handover

Nandanrshenoy commented 3 months ago

@yanavasileva , Good day !! I wish to work on this http-connector enhancement. Could you kindly suggest If I can go ahead with my analysis?

Thanks and Regards, Nandan Shenoy

danielkelemen commented 3 months ago

Hi @Nandanrshenoy, Thank you for opening an issue!

An incident needs to be created when ever HTTP connector throws an error for 4XX and 5XX errors

This generally makes sense, but it's very dependent on the user's business case. In my opinion these two connectors were meant to be very generic on purpose so they cover the most basic use-case, sending and receiving request. You already linked the source code, these connectors can be modified and extended as you like.

I wouldn't add the instant incident creation to these connectors. Especially that now it would be a breaking change for existing users. I'd recommend you to customize or extend the connector and add your own requirements. Here you can find more infos about the HTTP Connector configuration.

Furthermore, there are some more custom connectors in our camunda-community-hub. You could find something interesting or share your custom connector.

With that I'm closing this ticket.

-Daniel

Nandanrshenoy commented 3 months ago

Hi @danielkelemen , Thanks for responding back. For existing users the code would continue to work as is, since we are ensuring backward compatibility is intact when introducing the code change. For future users, they would have an option of configuring the connector via a flag ,through which they can enable/disable this feature .When enabled , the extended piece of code will take care of the concerns regarding 4XX and 5XX errors originating for http connector calls. I will post my code changes in the coming weeek so that you will have more clarity on how our changes will not break the existing connector capabilities. Post which, you can again revalidate and share your suggestions.Thanks.

Regards, Nandan

danielkelemen commented 3 months ago

@Nandanrshenoy Sounds good to me!

Nandanrshenoy commented 2 months ago

Hi @danielkelemen , Good day. I wish to share some updates as part of this issue. I made code changes locally and tested the 4XX and 5XX related scenarios for http-connector. Kindly note that the changes which I have made are backward compatible and this is in line with the discussion we had earlier. Once you validate my changes, I will share my test scenarios which I executed locally in detail.

Actual Changes

After analyzing the camunda-connect-http-client project end to end, the logic that we are planning to propose would basically be part of the execute method of AbstractHttpConnector class.The execute method intakes the request object and then executes the request on the connector and then provides the connector response. We have made this feature configurable through a connector setting (request-config parameter) by adding a new flag called throw-http-error. By default, this flag would be false, where modeler would have to explicitly handle such Errors. If marked true, then the feature that is being built to handle such errors would be enabled.

Module Name : camunda-connect-http-client Package Name : org.camunda.connect.httpclient.impl Java Class : AbstractHttpConnector.java

image-2024-4-18_18-29-20

Two addition connector request exception types are being added to support handling 4XX and 5XX errors.

Module Name : camunda-connect-http-client Package Name : org.camunda.connect.httpclient.impl Java Class : ConnectorRequestException.java

image-2024-4-18_19-22-1

Thanks for all your support.

Regards, Nandan Shenoy

Nandanrshenoy commented 2 months ago

@danielkelemen, A gentle reminder to help me with your review.

Thanks and Regards, Nandan Shenoy

Nandanrshenoy commented 2 months ago

@danielkelemen , Could you kindly help me with reviewing the actual code changes that I have posted, so that I can start working on the test cases. Thanks for your support.

Regards, Nandan Shenoy

tasso94 commented 2 weeks ago

Hi @danielkelemen,

@Nandanrshenoy raised a PR. Can you have a look?

Best, Tassilo

danielkelemen commented 5 days ago

Hi @Nandanrshenoy, Thanks for raising a PR and moving it from connect. You can find my review comments in the PR. Let me know if you have any questions. -Daniel