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

Exception Handling for "Not Found" request result #275

Closed bgrambichlerpixelart closed 2 years ago

bgrambichlerpixelart commented 2 years ago

Is it possible to implement exception handling if request result response with 404 "Not Found" result? Actually we did the migration v1 to SDK v2 and do now testing and it seems that SDK v2 now response with an 404 Exception if the request deliver no result (see Example Handling for v1 request). Previously v1 responded in such case with an NULL Object.

 CustomObject customPerson = projectClient.customObjects()
                    .withContainerAndKey(CustomPerson.CONTAINER_ONLINE, personId)
                    .get()
                    .execute()
                    .toCompletableFuture().join().getBody();

    if (customPerson == null) {
        // create Custom Object
    } else {
        // update Custom Object
    }

We did such Object exists checks on many places in the source code and now we realize that we have to find all those checks and implement an exception handling for SDK v2 to get the estimated NULL-Object. For us it's hard to find all checks before we are able to deploy the SDK migration to Production, so it would be nice if this behaviour of SDK v1 could be restored.

Best regards, Bernhard

Akii commented 2 years ago

Have you tried using exceptionally like in this example: https://docs.commercetools.com/sdk/jvm-sdk#recover-from-exceptions?

There are more ways of handling exceptions with Completable Futures: https://mincong.io/2020/05/30/exception-handling-in-completable-future/

bgrambichlerpixelart commented 2 years ago

Yes that is the actually workaround but it is still hard to identify every CT api-request in the source code for which it is necessary to do so.

public static <T> T sdkExceptionHandling(CompletableFuture<ApiHttpResponse<T>> execute, Class<T> type) {
            final ApiHttpResponse<T> response = execute.toCompletableFuture().exceptionally(ex -> null).join();
            return response != null ? type.cast(response.getBody()) : null;
    }

Maybe it's easiest do to it for every api-request ;-)

Akii commented 2 years ago

Another thing you could try is registering a middleware that does that. I recommend adding more checks to ensure that the status code is indeed 404 to not simply turn any exception into null. But that's the gist of it.

ApiRootBuilder.of()
    .defaultClient(...)
    .withMiddleware({ request, responseHandler ->
        responseHandler.apply(request).exceptionally { _ -> null }}
    )
    .build()
public interface Middleware {

    CompletableFuture<ApiHttpResponse<byte[]>> invoke(final ApiHttpRequest request,
            final Function<ApiHttpRequest, CompletableFuture<ApiHttpResponse<byte[]>>> next);

}
jenschude commented 2 years ago

I see two approaches to this problem. One is the use of a middleware which covers the exceptionally path with not found exception or an response with status code 404 as mentioned by @Akii

The other option could be to implement a custom error middleware and register it when building the client. Could give more details next week.

bgrambichlerpixelart commented 2 years ago

Ok thanks for the hint. I implemented an error middleware which handles status code 404 and responses HttpResponse with NULL-Object and Status Code 200. So our application could handle those null results on every place where it is excpected

jenschude commented 2 years ago

I added a small test with a custom middleware please see aff1bcb

bgrambichlerpixelart commented 2 years ago

Works also fine, thanks!

jenschude commented 2 years ago

I backported it to java8 as exceptionallyCompose had been introduced in Java12 https://github.com/commercetools/commercetools-sdk-java-v2/blob/d1b6cdfc4669ea64faca5514c6869a9506771b45/commercetools/commercetools-sdk-java-api/src/integrationTest/java/commercetools/MiddlewareTest.java#L62-L94

jenschude commented 2 years ago

In 8.0.0 a NotFoundExceptionMiddleware has been added which returns a null body in case of 404