camunda-community-hub / camunda-platform-7-rest-client-spring-boot

Camunda REST client for Java Spring Boot Projects, implemented using Feign
https://camunda-community-hub.github.io/camunda-platform-7-rest-client-spring-boot/
Apache License 2.0
73 stars 22 forks source link

Default throw ProcessEngineException rather than RemoteProcessEngineException #241

Closed callaertanthony closed 2 years ago

callaertanthony commented 2 years ago

Scenario

Current Behaviour

A RemoteProcessEngineException is thrown.

Wanted Behaviour

Throw the source ProcessEngineException instead.

Possible Workarounds

A current workaround is :

camunda.rest.client.error-encoding.enabled=false (note that there is an error on the property mapping, it's encoding rather than decoding in your doc)

@Override
@SneakyThrows
public Exception decode(String methodKey, Response response) {
  try {
    CamundaFeignExceptionDecoder camundaFeignExceptionDecoder = new CamundaFeignExceptionDecoder(response);
    throw Objects.requireNonNull(camundaFeignExceptionDecoder.decodeException());
  } catch (RemoteProcessEngineException remoteProcessEngineException) {
    throw Objects.requireNonNull(remoteProcessEngineException.getCause());
  }
}
rohwerj commented 2 years ago

Thank you for your interest in this library. Actually the RemoteProcessEngineException is extending the ProcessEngineException, so a calling application can still just catch the ProcessEngineException without needing to know, that indeed it's called via REST. But you have the possibility to differentiate on the specific subclass. Regarding the documentation you are correct. We have to decide, whether to fix the documentation or the property naming.

rohwerj commented 2 years ago

@callaertanthony Since the contract of the class states a ProcessEngineException can be thrown and RemoteProcessEngineException is a subclass of that, I don't see that the contract is broken. What exactly is your problem using this? If no further arguments exist, I'll close this issue. Opened #254 for the error in the documentation.

callaertanthony commented 2 years ago

Hi @rohwerj , The behavior I would explain is when you expect an extended camunda's ProcessEngineException to be catch, for example a MismatchingMessageCorrelationException, it seems to be better to directly throw this exception subtype rather than a RemoteProcessEngineException.

rohwerj commented 2 years ago

@callaertanthony okay I understand the problem now. Will think about a possible solution.

rohwerj commented 2 years ago

Added a new property camunda.rest.client.error-decoding.wrap-exceptions, that controls the behavior. Setting this to false will bring the desired result.