From f833aa172fc0e15b119ba332bac01a1267ddb4b5 Mon Sep 17 00:00:00 2001
From: antonbabak <ababak@magnite.com>
Date: Fri, 13 Oct 2023 15:56:16 +0200
Subject: [PATCH] Refactor Error Handling

---
 .../prebid/cache/handlers/CacheHandler.java   | 100 +++++++-----------
 .../aerospike/AerospikeRepositoryImpl.java    |   3 +-
 2 files changed, 42 insertions(+), 61 deletions(-)

diff --git a/src/main/java/org/prebid/cache/handlers/CacheHandler.java b/src/main/java/org/prebid/cache/handlers/CacheHandler.java
index b83ba5f..1ebdf63 100644
--- a/src/main/java/org/prebid/cache/handlers/CacheHandler.java
+++ b/src/main/java/org/prebid/cache/handlers/CacheHandler.java
@@ -4,7 +4,6 @@
 import org.apache.commons.lang3.exception.ExceptionUtils;
 import org.prebid.cache.exceptions.BadRequestException;
 import org.prebid.cache.exceptions.DuplicateKeyException;
-import org.prebid.cache.exceptions.PrebidException;
 import org.prebid.cache.exceptions.RepositoryException;
 import org.prebid.cache.exceptions.RequestParsingException;
 import org.prebid.cache.exceptions.ResourceNotFoundException;
@@ -19,6 +18,7 @@
 import org.springframework.web.reactive.function.server.ServerRequest;
 import org.springframework.web.reactive.function.server.ServerResponse;
 import reactor.core.publisher.Mono;
+import reactor.core.publisher.Signal;
 
 import java.util.concurrent.TimeoutException;
 
@@ -59,30 +59,32 @@ public String toString() {
 
     <T> Mono<T> validateErrorResult(final Mono<T> mono) {
         return mono.doOnSuccess(v -> log.debug("{}: {}", type, v))
-            .onErrorResume(t -> {
-                // skip overwrite, report first prebid error
-                if (t instanceof PrebidException) {
-                    // TODO: 19.09.18 move to handleErrorMetrics
-                    applyMetrics(t);
-
-                    if (t instanceof DuplicateKeyException) {
-                        return Mono.error(new BadRequestException(UUID_DUPLICATION));
-                    }
-                    return Mono.error(t);
-                } else if (t instanceof org.springframework.core.codec.DecodingException) {
-                    return Mono.error(new RequestParsingException(t.toString()));
-                } else if (t instanceof org.springframework.web.server.UnsupportedMediaTypeStatusException) {
-                    return Mono.error(new UnsupportedMediaTypeException(t.toString()));
-                } else {
-                    return Mono.error(t);
-                }
-            });
+                .onErrorMap(DuplicateKeyException.class, error -> {
+                    metricsRecorder.getExistingKeyError().increment();
+                    return new BadRequestException(UUID_DUPLICATION);
+                })
+                .onErrorMap(org.springframework.core.codec.DecodingException.class, error ->
+                        new RequestParsingException(error.toString()))
+                .onErrorMap(org.springframework.web.server.UnsupportedMediaTypeStatusException.class, error ->
+                        new UnsupportedMediaTypeException(error.toString()));
     }
 
-    // TODO: 19.09.18 refactor this to normal interaction with mono instead of "casting" exceptions to status codes
-    private Mono<ServerResponse> handleErrorMetrics(final Throwable error,
-                                                    final ServerRequest request) {
-        if (error instanceof ResourceNotFoundException) {
+    Mono<ServerResponse> finalizeResult(final Mono<ServerResponse> mono,
+                                        final ServerRequest request,
+                                        final MetricsRecorderTimer timerContext) {
+        // transform to error, if needed and send metrics
+        return mono
+                .onErrorResume(throwable -> handleErrorMetrics(throwable, request))
+                .doOnEach(signal -> {
+                    if (timerContext != null)
+                        timerContext.stop();
+                });
+    }
+
+    private Mono<ServerResponse> handleErrorMetrics(final Throwable error, final ServerRequest request) {
+        if (error instanceof RepositoryException) {
+            recordMetric(MetricsRecorder.MeasurementTag.ERROR_DB);
+        } else if (error instanceof ResourceNotFoundException) {
             conditionalLogger.info(
                     error.getMessage()
                             + ". Refererring URLs: " + request.headers().header(HttpHeaders.REFERER)
@@ -103,45 +105,25 @@ private Mono<ServerResponse> handleErrorMetrics(final Throwable error,
         }
 
         return builder.error(Mono.just(error), request)
-            .doOnEach(signal -> {
-                final var response = signal.get();
-                HttpMethod method = request.method();
-                if (method == null || signal.isOnError() || response == null) {
-                    metricsRecorder.markMeterForTag(this.metricTagPrefix,
-                            MetricsRecorder.MeasurementTag.ERROR_UNKNOWN);
-                } else {
-                    if (response.statusCode() == HttpStatus.INTERNAL_SERVER_ERROR) {
-                        metricsRecorder.markMeterForTag(this.metricTagPrefix,
-                                MetricsRecorder.MeasurementTag.ERROR_UNKNOWN);
-                    } else if (response.statusCode() == HttpStatus.BAD_REQUEST) {
-                        metricsRecorder.markMeterForTag(this.metricTagPrefix,
-                                MetricsRecorder.MeasurementTag.ERROR_BAD_REQUEST);
-                    } else if (response.statusCode() == HttpStatus.NOT_FOUND) {
-                        metricsRecorder.markMeterForTag(this.metricTagPrefix,
-                                MetricsRecorder.MeasurementTag.ERROR_MISSINGID);
-                    }
-                }
-            });
+                .doOnEach(signal -> handleErrorStatusCodes(request, signal));
     }
 
-    Mono<ServerResponse> finalizeResult(final Mono<ServerResponse> mono,
-                                        final ServerRequest request,
-                                        final MetricsRecorderTimer timerContext) {
-        // transform to error, if needed and send metrics
-        return mono.onErrorResume(throwable -> handleErrorMetrics(throwable, request))
-            .doOnEach(signal -> {
-                if (timerContext != null)
-                    timerContext.stop();
-            });
-    }
-
-    void applyMetrics(Throwable t) {
-        if (t instanceof RepositoryException) {
-            metricsRecorder.markMeterForTag(this.metricTagPrefix, MetricsRecorder.MeasurementTag.ERROR_DB);
+    private void handleErrorStatusCodes(ServerRequest request, Signal<ServerResponse> signal) {
+        final var response = signal.get();
+        HttpMethod method = request.method();
+        if (method == null || signal.isOnError() || response == null) {
+            recordMetric(MetricsRecorder.MeasurementTag.ERROR_UNKNOWN);
+        } else if (response.statusCode() == HttpStatus.INTERNAL_SERVER_ERROR) {
+            recordMetric(MetricsRecorder.MeasurementTag.ERROR_UNKNOWN);
+        } else if (response.statusCode() == HttpStatus.BAD_REQUEST) {
+            recordMetric(MetricsRecorder.MeasurementTag.ERROR_BAD_REQUEST);
+        } else if (response.statusCode() == HttpStatus.NOT_FOUND) {
+            recordMetric(MetricsRecorder.MeasurementTag.ERROR_MISSINGID);
         }
+    }
 
-        if (t instanceof DuplicateKeyException) {
-            metricsRecorder.getExistingKeyError().increment();
-        }
+    private void recordMetric(MetricsRecorder.MeasurementTag tag) {
+        metricsRecorder.markMeterForTag(this.metricTagPrefix, tag);
     }
+
 }
diff --git a/src/main/java/org/prebid/cache/repository/aerospike/AerospikeRepositoryImpl.java b/src/main/java/org/prebid/cache/repository/aerospike/AerospikeRepositoryImpl.java
index b25179a..b42b2cf 100644
--- a/src/main/java/org/prebid/cache/repository/aerospike/AerospikeRepositoryImpl.java
+++ b/src/main/java/org/prebid/cache/repository/aerospike/AerospikeRepositoryImpl.java
@@ -105,8 +105,7 @@ && getRetryCodes().contains(((AerospikeException) e).getResultCode()))
     }
 
     private <T> Mono<T> handleAerospikeError(Throwable throwable) {
-        if (throwable instanceof AerospikeException) {
-            AerospikeException aerospikeException = (AerospikeException) throwable;
+        if (throwable instanceof AerospikeException aerospikeException) {
             if (aerospikeException.getResultCode() == ResultCode.KEY_EXISTS_ERROR) {
                 return Mono.error(new DuplicateKeyException(throwable.toString(), throwable));
             }
