fix(mapper): custom mapper should not always return a 503 when the ma…

…pping fails - eclipse-hono#3609
JeffreyThijs · Jan 22, 2024 · c458082 · c458082
1 parent e9a92f3
commit c458082
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 5 deletions.
diff --git a/...s/mqtt-base/src/main/java/org/eclipse/hono/adapter/mqtt/impl/HttpBasedMessageMapping.java b/...s/mqtt-base/src/main/java/org/eclipse/hono/adapter/mqtt/impl/HttpBasedMessageMapping.java
@@ -24,7 +24,9 @@
 import org.eclipse.hono.adapter.mqtt.MessageMapping;
 import org.eclipse.hono.adapter.mqtt.MqttContext;
 import org.eclipse.hono.adapter.mqtt.MqttProtocolAdapterProperties;
+import org.eclipse.hono.client.ClientErrorException;
 import org.eclipse.hono.client.ServerErrorException;
+import org.eclipse.hono.client.ServiceInvocationException;
 import org.eclipse.hono.client.command.Command;
 import org.eclipse.hono.util.MessageHelper;
 import org.eclipse.hono.util.RegistrationAssertion;
@@ -199,7 +201,8 @@ private void mapUpstreamMessageRequest(
                         command.getDeviceId(),
                         mapperEndpoint.getHost(), mapperEndpoint.getPort(), mapperEndpoint.getUri(),
                         httpResponseAsyncResult.cause());
-                    result.fail(new ServerErrorException(HttpURLConnection.HTTP_UNAVAILABLE, httpResponseAsyncResult.cause()));
+                    final Throwable exception = mapException(httpResponseAsyncResult);
+                    result.fail(exception);
                 } else {
                     final HttpResponse<Buffer> httpResponse = httpResponseAsyncResult.result();
                     if (httpResponse.statusCode() == HttpURLConnection.HTTP_OK) {
@@ -208,8 +211,9 @@ private void mapUpstreamMessageRequest(
                         LOG.debug("mapping service [host: {}, port: {}, URI: {}] returned unexpected status code: {}",
                             mapperEndpoint.getHost(), mapperEndpoint.getPort(), mapperEndpoint.getUri(),
                             httpResponse.statusCode());
-                        result.fail(new ServerErrorException(HttpURLConnection.HTTP_UNAVAILABLE,
-                            "could not invoke configured mapping service"));
+                        final Throwable exception = mapException(httpResponseAsyncResult,
+                            "could not invoke configured mapping service");
+                        result.fail(exception);
                     }
                 }
                 resultHandler.handle(result.future());
@@ -279,4 +283,22 @@ private void mapDownstreamMessageRequest(
                 resultHandler.handle(result.future());
             });
     }
+
+    private Throwable mapException(AsyncResult<HttpResponse<Buffer>> httpResponseAsyncResult) {
+        final Optional<HttpResponse<Buffer>> httpResponse = Optional.ofNullable(httpResponseAsyncResult.result());
+        final int statusCode = httpResponse.map(HttpResponse::statusCode).orElse(HttpURLConnection.HTTP_INTERNAL_ERROR);
+        if (statusCode >= 400 && statusCode < 500) {
+            return new ClientErrorException(statusCode, httpResponseAsyncResult.cause());
+        }
+        return new ServerErrorException(statusCode, httpResponseAsyncResult.cause());
+    }
+
+    private Throwable mapException(AsyncResult<HttpResponse<Buffer>> httpResponseAsyncResult, String message) {
+        final Optional<HttpResponse<Buffer>> httpResponse = Optional.ofNullable(httpResponseAsyncResult.result());
+        final int statusCode = httpResponse.map(HttpResponse::statusCode).orElse(HttpURLConnection.HTTP_INTERNAL_ERROR);
+        if (statusCode >= 400 && statusCode < 500) {
+            return new ClientErrorException(statusCode, message);
+        }
+        return new ServerErrorException(statusCode, message);
+    }
 }
diff --git a/...tt-base/src/test/java/org/eclipse/hono/adapter/mqtt/impl/HttpBasedMessageMappingTest.java b/...tt-base/src/test/java/org/eclipse/hono/adapter/mqtt/impl/HttpBasedMessageMappingTest.java
@@ -34,6 +34,7 @@
 import org.eclipse.hono.adapter.MapperEndpoint;
 import org.eclipse.hono.adapter.mqtt.MqttContext;
 import org.eclipse.hono.adapter.mqtt.MqttProtocolAdapterProperties;
+import org.eclipse.hono.client.ClientErrorException;
 import org.eclipse.hono.client.ServerErrorException;
 import org.eclipse.hono.client.command.Command;
 import org.eclipse.hono.service.auth.DeviceUser;
@@ -379,8 +380,8 @@ public void testMappingCommandFailsForWhenPayloadCannotMapped(final VertxTestCon
         messageMapping.mapUpstreamMessage(assertion, command)
             .onComplete(ctx.failing(t -> {
                 ctx.verify(() -> {
-                    assertThat(t).isInstanceOf(ServerErrorException.class);
-                    assertThat((((ServerErrorException) t).getErrorCode())).isEqualTo(HttpURLConnection.HTTP_UNAVAILABLE);
+                    assertThat(t).isInstanceOf(ClientErrorException.class);
+                    assertThat((((ClientErrorException) t).getErrorCode())).isEqualTo(HttpURLConnection.HTTP_FORBIDDEN);
                 });
                 ctx.completeNow();
             }));