commercetools / commercetools-sdk-java-v2

The e-commerce SDK from commercetools for Java.
https://commercetools.github.io/commercetools-sdk-java-v2/javadoc/index.html
Apache License 2.0
34 stars 15 forks source link

Question: RetryHandler only with UNWRAP_COMPLETION_EXCEPTION? #553

Closed pintomau closed 7 months ago

pintomau commented 7 months ago

Hi.

Maybe more of a clarification.

I've noticed RetryHandler (https://github.com/commercetools/commercetools-sdk-java-v2/pull/385/files), but it is looking only for a specialized instance of ConcurrentModificationException (com.commercetools.api.client.error.ConcurrentModificationException instead of io.vrap...).

Screenshot 2024-01-18 230924

Am I using this wrong, do we need to use UNWRAP_COMPLETION_EXCEPTION?

Do you think creating our own RetryHandler based on io.vrap...ConcurrentModificationException would solve this, or is there something I'm missing?

jenschude commented 7 months ago

For the api module a special HttpExceptionFactory is initialized here:

https://github.com/commercetools/commercetools-sdk-java-v2/blob/dcae522192de548b72037c200dda5d18feb021d1/commercetools/commercetools-sdk-java-api/src/main/java/com/commercetools/api/defaultconfig/ApiRootBuilder.java#L39

The reason is that for api..commercetools.com you get additional information like the actual version or an informational body. This ApiHttpExceptionFactory maps some of the errors to specific exceptions for this module. Namely the ConcurrentModificationException and the BadRequestException so these additional informatio becomes accessible.

pintomau commented 7 months ago

I see. We're indeed using ClientBuilder with ProjectApiRoot#fromClient instead of ApiBuilder.

Adding that factory, works.

jenschude commented 7 months ago

Btw the unwrap completion exception flag is useful if the future should directly throw the exception in the error case. Which saves the check for the completionexceptions cause

pintomau commented 7 months ago

Yeah, if I had noticed sooner, I'd use it. We took care of it in a different way. Will consider refactor. Thanks