Limit the cloud host lookup to the timeout and log why processing failed

device-detection/device-detection.cloud/src/main/java/fiftyone/devicedetection/cloud/flowelements/DeviceDetectionCloudEngine.java

@@ -41,6 +41,8 @@
 import org.json.JSONObject;
 import org.slf4j.Logger;
 
+import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.NoCloudResponse;
+
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -138,6 +140,14 @@ protected void processEngine(FlowData data, DeviceDataCloud aspectData) {
             String json = "";
             json = requestData.getJsonResponse();
 
+            // A failed cloud request leaves no response. The Cloud Request
+            // Engine reports why, so this engine adds no error of its own.
+            if (json == null || json.isEmpty()) {
+                logger.warn(String.format(
+                    NoCloudResponse, getClass().getSimpleName()));
+                return;
+            }
+
             // Extract data from json to the aspectData instance.
             JSONObject jsonObj = new JSONObject(json);
             JSONObject deviceObj = getElementJson(jsonObj);

device-detection/device-detection.cloud/src/test/java/fiftyone/devicedetection/cloud/flowelements/ProductMissingTests.java

@@ -97,6 +97,26 @@ public void ProductMissing_ProcessLogsAndThrowsNothing() throws Exception {
         }
     }
 
+    /**
+     * A failed cloud request leaves no response. The engine processes
+     * nothing, throws nothing and logs one warning, as the Cloud Request
+     * Engine's error says why.
+     */
+    @Test
+    public void NoResponse_ProcessesNothing() throws Exception {
+        try (DeviceDetectionCloudEngine engine = buildEngine("device", "hardwarevendor")) {
+            for (String response : new String[] { null, "" }) {
+                DeviceDataCloud data = process(engine, response);
+                assertFalse(data.asKeyMap().containsKey("hardwarevendor"));
+            }
+            List<String> logged = warningsAndErrors();
+            assertEquals(logged.toString(), 2, logged.size());
+            for (String line : logged) {
+                assertTrue(line, line.startsWith("warn "));
+            }
+        }
+    }
+
     /**
      * With the Device Detection product included, a response that holds no
      * part for it leaves the element data without values and throws

ip-intelligence/ip-intelligence.cloud/src/main/java/fiftyone/ipintelligence/cloud/flowelements/IPIntelligenceCloudEngine.java

@@ -41,6 +41,8 @@
 import org.json.JSONObject;
 import org.slf4j.Logger;
 
+import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.NoCloudResponse;
+
 import fiftyone.pipeline.core.data.IWeightedValue;
 import fiftyone.pipeline.core.data.WeightedValue;
 import fiftyone.pipeline.core.data.WktString;
@@ -150,6 +152,14 @@ protected void processEngine(FlowData data, IPIntelligenceDataCloud aspectData)
             String json = "";
             json = requestData.getJsonResponse();
 
+            // A failed cloud request leaves no response. The Cloud Request
+            // Engine reports why, so this engine adds no error of its own.
+            if (json == null || json.isEmpty()) {
+                logger.warn(String.format(
+                    NoCloudResponse, getClass().getSimpleName()));
+                return;
+            }
+
             // Extract data from json to the aspectData instance.
             JSONObject jsonObj = new JSONObject(json);
             JSONObject ipiObj = getElementJson(jsonObj);

ip-intelligence/ip-intelligence.cloud/src/test/java/fiftyone/ipintelligence/cloud/flowelements/ProductMissingTests.java

@@ -97,6 +97,26 @@ public void ProductMissing_ProcessLogsAndThrowsNothing() throws Exception {
         }
     }
 
+    /**
+     * A failed cloud request leaves no response. The engine processes
+     * nothing, throws nothing and logs one warning, as the Cloud Request
+     * Engine's error says why.
+     */
+    @Test
+    public void NoResponse_ProcessesNothing() throws Exception {
+        try (IPIntelligenceCloudEngine engine = buildEngine("ip", "registeredname")) {
+            for (String response : new String[] { null, "" }) {
+                IPIntelligenceDataCloud data = process(engine, response);
+                assertFalse(data.asKeyMap().containsKey("registeredname"));
+            }
+            List<String> logged = warningsAndErrors();
+            assertEquals(logged.toString(), 2, logged.size());
+            for (String line : logged) {
+                assertTrue(line, line.startsWith("warn "));
+            }
+        }
+    }
+
     /**
      * With the IP Intelligence product included, a response that holds no
      * part for it leaves the element data without values and throws

location/geo-location.cloud/src/main/java/fiftyone/geolocation/flowelements/GeoLocationCloudEngine.java

@@ -43,6 +43,8 @@
 import org.json.JSONObject;
 import org.slf4j.Logger;
 
+import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.NoCloudResponse;
+
 import java.util.*;
 import org.json.JSONArray;
 
@@ -137,6 +139,14 @@ protected void processEngine(FlowData data, CloudGeoData aspectData) {
             CloudRequestData requestData = data.getFromElement(cloudRequestEngine);
             String json = requestData.getJsonResponse();
 
+            // A failed cloud request leaves no response. The Cloud Request
+            // Engine reports why, so this engine adds no error of its own.
+            if (json == null || json.isEmpty()) {
+                logger.warn(String.format(
+                    NoCloudResponse, getClass().getSimpleName()));
+                return;
+            }
+
             JSONObject jsonObj = new JSONObject(json);
             JSONObject geoObj = getElementJson(jsonObj);
 

location/geo-location.cloud/src/test/java/fiftyone/geolocation/flowelements/GeoLocationCloudEngineNullValueTests.java

@@ -36,6 +36,7 @@
 import fiftyone.pipeline.engines.services.MissingPropertyServiceDefault;
 import org.junit.Test;
 import org.slf4j.ILoggerFactory;
+import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.Arrays;
@@ -46,8 +47,12 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.NoCloudResponse;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 /**
@@ -142,6 +147,27 @@ public void SetNoValueReasons_ChangesNothing() throws Exception {
         }
     }
 
+    /**
+     * A failed cloud request leaves no response. The engine processes
+     * nothing, throws nothing and logs one warning each time, as the Cloud
+     * Request Engine's error says why.
+     */
+    @Test
+    public void NoResponse_ProcessesNothing() throws Exception {
+        Logger logger = mock(Logger.class);
+        ILoggerFactory warnings = mock(ILoggerFactory.class);
+        when(warnings.getLogger(anyString())).thenReturn(logger);
+        try (GeoLocationCloudEngine engine = buildEngine(warnings)) {
+            for (String response : new String[] { null, "" }) {
+                CloudGeoData data = process(engine, response);
+
+                assertFalse(data.asKeyMap().containsKey("Town"));
+            }
+        }
+        verify(logger, times(2)).warn(String.format(
+            NoCloudResponse, GeoLocationCloudEngine.class.getSimpleName()));
+    }
+
     private static void assertMissing(Runnable read) {
         try {
             read.run();
@@ -183,8 +209,16 @@ private static void assertValue(Object expected, Object value)
      * the Suburb and Town properties.
      */
     private GeoLocationCloudEngine buildEngine() throws Exception {
+        return buildEngine(loggerFactory);
+    }
+
+    /**
+     * The same engine, logging through the given factory.
+     */
+    private GeoLocationCloudEngine buildEngine(ILoggerFactory engineLoggers)
+        throws Exception {
         GeoLocationCloudEngine engine =
-            new GeoLocationCloudEngineBuilder(loggerFactory)
+            new GeoLocationCloudEngineBuilder(engineLoggers)
                 .build(Enums.GeoLocationProvider.FiftyOneDegrees);
 
         ProductMetaData product = new ProductMetaData();

location/pom.xml

@@ -63,7 +63,7 @@
         <devicedetection.version>4.5.130</devicedetection.version>
 
         <junit.version>4.13.2</junit.version>
-        <slf4j-api.version>1.7.36</slf4j-api.version>
+        <slf4j-api.version>2.0.20</slf4j-api.version>
         <logback-classic.version>1.3.16</logback-classic.version>
         <mockito-core.version>4.3.1</mockito-core.version>
         <org.json.version>20211205</org.json.version>

pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java

@@ -58,6 +58,16 @@ public class Messages {
 
         public static final String EvidenceConflict =
             "'%s:%s' evidence conflicts with %s";
+
+        /**
+         * Warning a cloud engine logs when the Cloud Request Engine has no
+         * response for it to read. The argument is the engine's class name.
+         */
+        public static final String NoCloudResponse =
+            "The %s did not process, as the Cloud Request Engine has no "
+            + "response. The Cloud Request Engine's error in the log says "
+            + "why.";
+
         public static final String MessageNoDataInResponse =
             "No data in response from cloud service at '%s'";
         public static final String MessageErrorCodeReturned =

pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudAspectEngineBase.java

@@ -23,6 +23,7 @@
 package fiftyone.pipeline.cloudrequestengine.flowelements;
 
 import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.MessageProductNotInResourceKey;
+import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.NoCloudResponse;
 import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.ProcessCloudEngineNotImplemented;
 
 import fiftyone.pipeline.cloudrequestengine.NotImplementedException;
@@ -432,11 +433,8 @@ protected void processEngine(FlowData data, TData aspectData) {
         if (json != null && !json.isEmpty()) {
         	 processCloudEngine(data, aspectData, json);
         } else {
-            logger.warn("The  " + this.getClass().getSimpleName() + "  did not process " +
-                    "as the JSON response from the CloudRequestEngine was null " +
-                    "or empty. Please refer to errors generated by the " +
-                    "CloudRequestEngine in the logs as this indicates an error " +
-                    "occurred there.");
+            logger.warn(String.format(
+                NoCloudResponse, getClass().getSimpleName()));
        }
 	}
 

pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/FlowDataDefault.java

@@ -121,7 +121,7 @@ public void addError(FlowError error) {
                     error.getFlowElement().getClass().getSimpleName() +
                     "-" + error.getFlowElement().hashCode();
             }
-            logger.error(message);
+            logger.error(message, error.getThrowable());
         }
     }
 

pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/FlowDataTests.java

@@ -317,6 +317,25 @@ public void FlowData_GetNotPresent_Element() {
         assertNull(result);
     }
 
+    /**
+     * The error logged for a processing failure carries the exception, so
+     * the log says why processing failed.
+     */
+    @Test
+    public void FlowData_AddError_LogsTheException() throws Exception {
+        Logger logger = mock(Logger.class);
+        when(logger.isErrorEnabled()).thenReturn(true);
+        Exception cause = new Exception("The cause of the failure.");
+        try (FlowData data = new FlowDataDefault(
+            logger,
+            pipeline,
+            new EvidenceDefault(mock(Logger.class)))) {
+
+            data.addError(cause, mock(FlowElement.class));
+        }
+        verify(logger).error(anyString(), same(cause));
+    }
+
     @Test(expected = IllegalStateException.class)
     public void FlowData_PipelineDisposed() throws Exception {
         when(pipeline.isClosed()).thenReturn(true);

pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/flowelements/SimpleCloudEngine.java

@@ -34,6 +34,8 @@
 import org.json.JSONObject;
 import org.slf4j.Logger;
 
+import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.NoCloudResponse;
+
 import java.util.ArrayList;
 import java.util.List;
 
@@ -90,6 +92,14 @@ protected void processEngine(FlowData data, StarSignData aspectData) {
         CloudRequestData requestData = data.getFromElement(cloudRequestEngine);
         String json = requestData.getJsonResponse();
 
+        // A failed cloud request leaves no response. The Cloud Request
+        // Engine reports why, so this engine adds no error of its own.
+        if (json == null || json.isEmpty()) {
+            logger.warn(String.format(
+                NoCloudResponse, getClass().getSimpleName()));
+            return;
+        }
+
         // Extract data from json to the aspectData instance. A cloud
         // service answers only for the products the resource key covers,
         // so a response without the star sign product leaves the value