Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
3 tasks done
Nandanrshenoy opened this issue Apr 2, 2024 · 12 comments
Closed
3 tasks done

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

Nandanrshenoy opened this issue Apr 2, 2024 · 12 comments
Assignees
Labels
scope:connect Changes to the Connect library. type:feature Issues that add a new user feature to the project. version:7.22.0-alpha4 version:7.22.0

Comments

@Nandanrshenoy
Copy link
Contributor

Nandanrshenoy commented Apr 2, 2024

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 Nandanrshenoy added the type:feature Issues that add a new user feature to the project. label Apr 2, 2024
@Nandanrshenoy
Copy link
Contributor Author

@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
Copy link
Member

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

@danielkelemen danielkelemen closed this as not planned Won't fix, can't repro, duplicate, stale Apr 3, 2024
@Nandanrshenoy
Copy link
Contributor Author

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 danielkelemen reopened this Apr 5, 2024
@danielkelemen danielkelemen self-assigned this Apr 5, 2024
@danielkelemen
Copy link
Member

@Nandanrshenoy Sounds good to me!

@Nandanrshenoy
Copy link
Contributor Author

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
Copy link
Contributor Author

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

Thanks and Regards,
Nandan Shenoy

@Nandanrshenoy
Copy link
Contributor Author

@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

@Nandanrshenoy Nandanrshenoy changed the title Camunda Http Connector Enhancemnet for 4XX and 5XX errors Camunda Http Connector Enhancement for 4XX and 5XX errors Jun 5, 2024
@tasso94
Copy link
Member

tasso94 commented Jun 21, 2024

Hi @danielkelemen,

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

Best,
Tassilo

@danielkelemen
Copy link
Member

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

@danielkelemen
Copy link
Member

@Nandanrshenoy, thank you for the contribution!

@Nandanrshenoy
Copy link
Contributor Author

Thanks for all the support you have extended @danielkelemen . It was definitely a nice experience for me.

@danielkelemen
Copy link
Member

danielkelemen commented Sep 23, 2024

Dev2QA

  • Create a process with async Service Task & HTTP Connector
  • Test it with the extra throw-http-error configuration:
    image
  • If true, it should create an incident, otherwise the process just completes.
  • Attached sample process: connector-test.zip

hauptmedia added a commit to hauptmedia/operaton that referenced this issue Nov 3, 2024
related to camunda/camunda-bpm-platform#4241

Backported commit 42c77fc8d9 from the camunda-bpm-platform repository. Original author: Shenoy, Nandan <nandan.shenoy@fmr.com>
kthoms pushed a commit to kthoms/operaton that referenced this issue Nov 5, 2024
related to camunda/camunda-bpm-platform#4241

Backported commit 42c77fc8d9 from the camunda-bpm-platform repository. Original author: Shenoy, Nandan <nandan.shenoy@fmr.com>
kthoms pushed a commit to kthoms/operaton that referenced this issue Nov 5, 2024
related to camunda/camunda-bpm-platform#4241

Backported commit 42c77fc8d9 from the camunda-bpm-platform repository. Original author: Shenoy, Nandan <nandan.shenoy@fmr.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:connect Changes to the Connect library. type:feature Issues that add a new user feature to the project. version:7.22.0-alpha4 version:7.22.0
Projects
None yet
Development

No branches or pull requests

4 participants