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

[REST connector] Access response in error expression with status code 400 #2520

Open
saig0 opened this issue May 14, 2024 · 7 comments
Open
Assignees
Labels
kind:enhancement New feature or request scope:rest

Comments

@saig0
Copy link
Member

saig0 commented May 14, 2024

Is your feature request related to a problem? Please describe.

I use the SpaceTraders connector in my process and try to access the response in the error expression. The connector is based on Camunda's REST connector.

I followed the documentation for error expressions (ref) but I can't access the response if the API returned a status code 400.

In my case, the API returns a status code 400 and sends additional data about the failure:

// status code
400 Bad Request

// URL
POST https://api.spacetraders.io/v2/my/ships/C_4979-1/navigate

// response data
{
   "error":{
      "message":"Ship action failed. Ship is not currently in orbit at X1-ZQ60-A1.",
      "code":4236,
      "data":{
         "waypointSymbol":"X1-ZQ60-A1"
      }
   }
}

NOTE: This is specific for a failure response (i.e. status code 4xx). If the response returns successfully (i.e. status code 2xx), I can access the response data in the error expression.

Describe the solution you'd like

I can access the response data in the error expression, even if the response has a status code 400.

I can use the following error expression to throw a BPMN error if the error code in the response matches:

if matches(error.code, "4\d\d") and response.body.error.code = 4236 
then bpmnError("shipNotInOrbitError", response.body.error.message, response.body.error.data)
else null

Describe alternatives you've considered

None. Since the response has a status code 400, I can't handle the response in the connector or the process.

Additional context

How to reproduce:

  1. Deploy the process Handle BPMN error.bpmn (with .bpmn file extension)
  2. Create a new instance of the process
  3. Wait and verify that the process instance has an incident at the last task

Screenshot from 2024-05-14 10-10-26

Screenshot from 2024-05-14 10-11-15

@johnBgood
Copy link
Contributor

@saig0 Thank you for reporting this.
Something we can do short term is provide the response (if any) in the error.errorVariables object.

You would then be able to use the following error expression:

if matches(error.code, "4\d\d") and error.errorVariables.response.error.code = 4236 
then bpmnError("shipNotInOrbitError", error.errorVariables.response.error.message, error.errorVariables.response.error.data)
else null

@sbuettner
Copy link
Contributor

@johnBgood Can we change the property name to variables to not repeat the error prefix here?

@johnBgood
Copy link
Contributor

@sbuettner It's not been release if I'm correct, so yes let's change the name 👍

@saig0
Copy link
Member Author

saig0 commented May 22, 2024

@johnBgood sounds interesting. 👍

Why do we plan to store the response in error.variables.response.x instead of response.body.x?

In other cases, I can access the response directly via response.body.x. Using a different prefix error.variables.response.x could be more complicated and confusing for users.

@johnBgood
Copy link
Contributor

@saig0 good question :D

The idea is that the solution explained above is pretty much free to implement + it solves the user need.

Then, we can collect users' feedback, if they're happy great. If not we can iterate and go for the more complex solution.
Using response.body.x is more complex and it would require some specific code both in the Webhook Connector and the ConnectorJobHandler which is the core piece for Outbound Connectors.

It's possible but longer and trickier :) (but we might implement it some day)

@saig0
Copy link
Member Author

saig0 commented May 27, 2024

@johnBgood thank you for the reasoning. 👍

I care about the UX and vote for an intuitive solution that is aligned with the other expressions. If it is a general behavior that connectors store the error details in error.variables, it's fine.

I don't need a solution very soon. 😉

@b3rnh8rd
Copy link

b3rnh8rd commented Jun 12, 2024

Is there already a plan for when this feature will be available? Our project uses the REST connectors quite frequently. In many cases, the response would also be essential for correct processing in the event of an error.

@johnBgood johnBgood self-assigned this Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:enhancement New feature or request scope:rest
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

4 participants