Ensure HTTP Device Factory logs errors correctly

Signed-off-by: Thomas Calmant <thomas.calmant@kentyou.com>
DataInMotion · Feb 8, 2024 · 902e90f · 902e90f
1 parent 8c1912e
commit 902e90f
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 5 deletions.
diff --git a/...rc/main/java/org/eclipse/sensinact/gateway/southbound/http/factory/HttpDeviceFactory.java b/...rc/main/java/org/eclipse/sensinact/gateway/southbound/http/factory/HttpDeviceFactory.java
@@ -183,15 +183,21 @@ public void onHeaders(Response response) {
 
                 @Override
                 public void onFailure(final Response response, final Throwable failure) {
-                    logger.error("Error {} requesting {}: {}", response.getStatus(), task.url, failure);
+                    logger.error("Error accessing {}: {} ({})", task.url, failure.getMessage(),
+                            failure.getClass().getName(), failure);
                 }
 
                 @Override
                 public void onSuccess(final Response response) {
-                    try {
-                        mappingHandler.handle(task.mapping, headers.get(), getContent());
-                    } catch (DeviceFactoryException e) {
-                        logger.error("Error parsing input from {}", task.url, e);
+                    final int status = response.getStatus();
+                    if (status >= 200 && status < 300) {
+                        try {
+                            mappingHandler.handle(task.mapping, headers.get(), getContent());
+                        } catch (DeviceFactoryException e) {
+                            logger.error("Error parsing input from {}: {}", task.url, e.getMessage(), e);
+                        }
+                    } else {
+                        logger.error("HTTP error {} accessing {}", status, task.url);
                     }
                 }
 

diff --git a/.../eclipse/sensinact/gateway/southbound/http/factory/integration/HttpDeviceFactoryTest.java b/.../eclipse/sensinact/gateway/southbound/http/factory/integration/HttpDeviceFactoryTest.java
@@ -44,6 +44,8 @@
 import org.eclipse.sensinact.core.session.SensiNactSession;
 import org.eclipse.sensinact.core.session.SensiNactSessionManager;
 import org.eclipse.sensinact.gateway.geojson.Point;
+import org.eclipse.sensinact.gateway.southbound.device.factory.dto.DeviceMappingConfigurationDTO;
+import org.eclipse.sensinact.gateway.southbound.http.factory.HttpDeviceFactory;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
@@ -52,9 +54,14 @@
 import org.osgi.service.cm.Configuration;
 import org.osgi.service.cm.ConfigurationAdmin;
 import org.osgi.test.common.annotation.InjectService;
+import org.slf4j.LoggerFactory;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.AppenderBase;
+
 /**
  * Tests the HTTP device factory
  */
@@ -401,4 +408,51 @@ void testCombined() throws Exception {
             config.delete();
         }
     }
+
+    @Test
+    void testError() throws Exception {
+
+        // Listen to logs
+        final Logger logger = (Logger) LoggerFactory.getLogger(HttpDeviceFactory.class);
+
+        final BlockingQueue<String> queue = new ArrayBlockingQueue<String>(128);
+        final AppenderBase<ILoggingEvent> logSpy = new AppenderBase<>() {
+            @Override
+            protected void append(ILoggingEvent eventObject) {
+                try {
+                    queue.put(eventObject.getFormattedMessage());
+                } catch (Exception e) {
+                    fail(e);
+                }
+            }
+        };
+        logSpy.start();
+        logger.addAppender(logSpy);
+
+        Configuration config = configAdmin.createFactoryConfiguration("sensinact.http.device.factory", "?");
+        try {
+            // Prepare some mapping configuration
+            final DeviceMappingConfigurationDTO mappingConf = new DeviceMappingConfigurationDTO();
+            mappingConf.mapping = Map.of("@provider", "provider");
+
+            // First try: handling of a 404 error
+            Map<String, Object> httpMapping = Map.of("url", "http://localhost:" + httpPort + "/non-existent", "mapping",
+                    mappingConf);
+            config.update(new Hashtable<>(Map.of("tasks.oneshot", mapper.writeValueAsString(List.of(httpMapping)))));
+            String errorMessage = queue.poll(1, TimeUnit.SECONDS);
+            assertNotNull(errorMessage);
+            assertTrue(errorMessage.contains("404"));
+
+            // Second try: handling of connectivity error (failure)
+            httpMapping = Map.of("url", "https://localhost:" + httpPort + "/static", "mapping", mappingConf);
+            config.update(new Hashtable<>(Map.of("tasks.oneshot", mapper.writeValueAsString(List.of(httpMapping)))));
+            errorMessage = queue.poll(1, TimeUnit.SECONDS);
+            assertNotNull(errorMessage);
+            assertTrue(errorMessage.contains("SSLHandshakeException"));
+        } finally {
+            logger.detachAppender(logSpy);
+            logSpy.stop();
+            config.delete();
+        }
+    }
 }