From 2a9f975a5da50f8982918160dc03dd56d2c33885 Mon Sep 17 00:00:00 2001 From: Lyn Long Date: Fri, 24 Jul 2026 16:10:16 +1000 Subject: [PATCH] ensure wfs download failure error --- .../core/exception/wfs/WfsErrorHandler.java | 24 +++- .../geoserver/wfs/DownloadWfsDataService.java | 52 ++++++- .../core/service/geoserver/wfs/WfsServer.java | 8 +- .../ogcapi/server/processes/RestServices.java | 4 +- .../exception/wfs/WfsErrorHandlerTest.java | 131 ++++++++++++++++++ .../service/geoserver/wfs/WfsServerTest.java | 53 +++++++ .../server/core/util/TestLogAppender.java | 49 +++++++ .../wfs/DownloadWfsDataServiceTest.java | 103 +++++++++++++- 8 files changed, 414 insertions(+), 10 deletions(-) create mode 100644 server/src/test/java/au/org/aodn/ogcapi/server/core/exception/wfs/WfsErrorHandlerTest.java create mode 100644 server/src/test/java/au/org/aodn/ogcapi/server/core/util/TestLogAppender.java diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/exception/wfs/WfsErrorHandler.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/exception/wfs/WfsErrorHandler.java index fac2ec7f..93b1dc53 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/exception/wfs/WfsErrorHandler.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/exception/wfs/WfsErrorHandler.java @@ -4,6 +4,7 @@ import au.org.aodn.ogcapi.server.core.exception.UnauthorizedServerException; import au.org.aodn.ogcapi.server.core.model.enumeration.SseEventName; import lombok.extern.slf4j.Slf4j; +import org.springframework.web.client.HttpStatusCodeException; import org.springframework.web.servlet.mvc.method.annotation.SseEmitter; import java.io.IOException; @@ -17,6 +18,7 @@ public class WfsErrorHandler { private static final Set handledEmitters = Collections.newSetFromMap(new ConcurrentHashMap<>()); private enum ErrorType { + WFS_SERVER_ERROR, NETWORK_ERROR, VALIDATION_ERROR, UNAUTHORIZED_SERVER_ERROR, @@ -43,6 +45,19 @@ public static void handleError(Exception e, String uuid, SseEmitter emitter, Run } switch (errorType) { + case WFS_SERVER_ERROR -> { + // ERROR level so New Relic can alert on failed downloads and identify + // the problematic upstream server (the URL is in the exception message). + log.error("WFS server error during download for UUID {}", uuid, e); + emitter.send(SseEmitter.event() + .name(SseEventName.ERROR.getValue()) + .data(Map.of( + "message", "WFS server error", + "timestamp", System.currentTimeMillis() + ))); + emitter.completeWithError(e); + } + case NETWORK_ERROR -> { log.warn("Client disconnected for UUID: {}", uuid); emitter.completeWithError(e); @@ -82,7 +97,7 @@ public static void handleError(Exception e, String uuid, SseEmitter emitter, Run } case UNKNOWN_ERROR -> { - log.warn("Unknown error for UUID {}", uuid, e); + log.error("Unknown error during WFS download for UUID {}", uuid, e); emitter.send(SseEmitter.event() .name(SseEventName.ERROR.getValue()) .data(Map.of( @@ -103,6 +118,13 @@ public static void handleError(Exception e, String uuid, SseEmitter emitter, Run } private static ErrorType categorizeError(Exception e) { + // Upstream WFS server rejected the request (4xx/5xx). Must be checked before + // the network heuristics below so a failing server is never mistaken for a + // client disconnect. + if (e instanceof HttpStatusCodeException) { + return ErrorType.WFS_SERVER_ERROR; + } + if (e instanceof IOException || e instanceof IllegalStateException || e.getMessage() != null && ( e.getMessage().contains("Broken pipe") || e.getMessage().contains("Connection reset") || diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geoserver/wfs/DownloadWfsDataService.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geoserver/wfs/DownloadWfsDataService.java index a7779349..18bcd1be 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geoserver/wfs/DownloadWfsDataService.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geoserver/wfs/DownloadWfsDataService.java @@ -11,6 +11,9 @@ import org.springframework.cache.annotation.Cacheable; import org.springframework.context.annotation.Lazy; import org.springframework.http.*; +import org.springframework.web.client.HttpStatusCodeException; +import org.springframework.web.client.ResourceAccessException; +import org.springframework.web.client.RestClientException; import org.springframework.web.client.RestTemplate; import org.springframework.web.servlet.mvc.method.annotation.SseEmitter; @@ -104,7 +107,13 @@ public BigInteger getUnfilteredRecordCount(String uuid, String layerName) { uuid, null, null, null, null, layerName, "application/json", 1L, false ); - ResponseEntity response = restTemplate.exchange(countUrl, HttpMethod.GET, pretendUserEntity, String.class); + ResponseEntity response; + try { + response = restTemplate.exchange(countUrl, HttpMethod.GET, pretendUserEntity, String.class); + } catch (RestClientException e) { + log.error("WFS record count request failed for {}/{} against server url {}", uuid, layerName, countUrl, e); + throw e; + } if (response.getStatusCode().is2xxSuccessful() && response.getBody() != null) { try { JsonNode root = objectMapper.readTree(response.getBody()); @@ -137,7 +146,13 @@ public BigInteger getBytesPerRecord(String uuid, String layerName, String output uuid, null, null, null, null, layerName, outputFormat, sampleSize, false ); - ResponseEntity bytes = restTemplate.exchange(sampleUrl, HttpMethod.GET, pretendUserEntity, byte[].class); + ResponseEntity bytes; + try { + bytes = restTemplate.exchange(sampleUrl, HttpMethod.GET, pretendUserEntity, byte[].class); + } catch (RestClientException e) { + log.error("WFS sample download failed for {}/{} against server url {}", uuid, layerName, sampleUrl, e); + throw e; + } if (bytes.getStatusCode().is2xxSuccessful() && bytes.getBody() != null) { return BigInteger.valueOf(bytes.getBody().length).divide(BigInteger.valueOf(sampleSize)); } @@ -164,7 +179,13 @@ public BigInteger estimateDownloadSize( uuid, startDate, endDate, multiPolygon, fields, layerName, "application/json", 1L, false ); - ResponseEntity countResponse = restTemplate.exchange(countUrl, HttpMethod.GET, pretendUserEntity, String.class); + ResponseEntity countResponse; + try { + countResponse = restTemplate.exchange(countUrl, HttpMethod.GET, pretendUserEntity, String.class); + } catch (RestClientException e) { + log.error("WFS estimate count request failed for {}/{} against server url {}", uuid, layerName, countUrl, e); + throw e; + } if (countResponse.getStatusCode().is2xxSuccessful() && countResponse.getBody() != null) { try { @@ -201,6 +222,31 @@ public void streamWfsDataWithSse( String outputFormat, SseEmitter emitter, AtomicBoolean wfsServerResponded) { + try { + doStreamWfsDataWithSse(wfsRequestUrl, uuid, layerName, outputFormat, emitter, wfsServerResponded); + } catch (HttpStatusCodeException e) { + log.error("WFS download failed for UUID {} layer {}: server url {} returned status {}", + uuid, layerName, wfsRequestUrl, e.getStatusCode(), e); + throw e; + } catch (ResourceAccessException e) { + // A ResourceAccessException raised before the WFS server responded means the + // server itself is unreachable; after it responded, the wrapped IOException + // almost always comes from emitter.send() to a disconnected client. + if (!wfsServerResponded.get()) { + log.error("WFS download failed for UUID {} layer {}: cannot reach server url {}", + uuid, layerName, wfsRequestUrl, e); + } + throw e; + } + } + + private void doStreamWfsDataWithSse( + String wfsRequestUrl, + String uuid, + String layerName, + String outputFormat, + SseEmitter emitter, + AtomicBoolean wfsServerResponded) { restTemplate.execute( wfsRequestUrl, HttpMethod.GET, diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geoserver/wfs/WfsServer.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geoserver/wfs/WfsServer.java index b735ed3e..e5ffa835 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geoserver/wfs/WfsServer.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geoserver/wfs/WfsServer.java @@ -377,7 +377,9 @@ public T getFieldValues(String collectionId, WfsFeatureRequest request, Para } } } catch (Exception e) { - log.debug("Ignore error for {}, will try another url", uri); + // WARN so New Relic can identify a problematic WFS server even when a + // later url succeeds and the download recovers. + log.warn("WFS field value query failed for {}, will try another url. Cause: {}", uri, e.toString()); } } } @@ -419,7 +421,9 @@ public WfsFields getDownloadableFields(String collectionId, WfsFeatureRequest re } } } catch (URISyntaxException | JsonProcessingException | RestClientException e) { - log.debug("Ignore error for {}, will try another url", uri); + // WARN so New Relic can identify a problematic WFS server even when a + // later url succeeds and the download recovers. + log.warn("WFS DescribeFeatureType failed for {}, will try another url. Cause: {}", uri, e.toString()); } } } else { diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestServices.java b/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestServices.java index 844016ae..158571e2 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestServices.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestServices.java @@ -283,7 +283,7 @@ public SseEmitter estimateWfsDownloadWithSse(String uuid, "timestamp", System.currentTimeMillis() )); } catch (Exception e) { - log.warn("Unexpected error during size estimation for UUID {}: {}", uuid, e.getMessage()); + log.error("WFS size estimation failed for UUID {} layer {}", uuid, layerName, e); session.send(SseEventName.ESTIMATE_FAILED, Map.of( "message", "Size estimation failed: " + e.getMessage(), "timestamp", System.currentTimeMillis() @@ -344,7 +344,7 @@ public SseEmitter estimateCloudOptimisedDownloadWithSse(String uuid, } /** - * Shared input validation for the two WFS SSE flows. + * Shared input validation for the two SSE flows. */ private void validateWfsSseInputs(String uuid, String layerName, String outputFormat) { if (uuid == null || layerName == null || layerName.trim().isEmpty()) { diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/core/exception/wfs/WfsErrorHandlerTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/core/exception/wfs/WfsErrorHandlerTest.java new file mode 100644 index 00000000..27d64dbc --- /dev/null +++ b/server/src/test/java/au/org/aodn/ogcapi/server/core/exception/wfs/WfsErrorHandlerTest.java @@ -0,0 +1,131 @@ +package au.org.aodn.ogcapi.server.core.exception.wfs; + +import au.org.aodn.ogcapi.server.core.exception.GeoserverFieldsNotFoundException; +import au.org.aodn.ogcapi.server.core.exception.UnauthorizedServerException; +import au.org.aodn.ogcapi.server.core.util.TestLogAppender; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.core.LogEvent; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpStatus; +import org.springframework.web.client.HttpServerErrorException; +import org.springframework.web.servlet.mvc.method.annotation.SseEmitter; + +import java.io.IOException; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +public class WfsErrorHandlerTest { + + protected TestLogAppender logAppender; + + @BeforeEach + void attachLogAppender() { + logAppender = TestLogAppender.attachTo(WfsErrorHandler.class); + } + + @AfterEach + void detachLogAppender() { + logAppender.detachFrom(WfsErrorHandler.class); + } + + /** + * An upstream WFS server error status must be logged at ERROR level (so New + * Relic can pick it up), sent to the client as an SSE error event, and must + * not be mistaken for a client disconnect. + */ + @Test + void verifyUpstreamServerErrorLoggedAtErrorLevel() throws IOException { + SseEmitter emitter = mock(SseEmitter.class); + + WfsErrorHandler.handleError( + new HttpServerErrorException(HttpStatus.INTERNAL_SERVER_ERROR), + "uuid-123", emitter, null); + + List errors = logAppender.eventsAtLevel(Level.ERROR); + assertEquals(1, errors.size()); + assertTrue(errors.get(0).getMessage().getFormattedMessage().contains("uuid-123")); + assertNotNull(errors.get(0).getThrown(), "stack trace expected in error log"); + + verify(emitter, times(1)).send(any(SseEmitter.SseEventBuilder.class)); + verify(emitter, times(1)).completeWithError(any()); + } + + /** + * An unexpected exception must be logged at ERROR level too. + */ + @Test + void verifyUnknownErrorLoggedAtErrorLevel() throws IOException { + SseEmitter emitter = mock(SseEmitter.class); + + WfsErrorHandler.handleError(new RuntimeException("boom"), "uuid-123", emitter, null); + + assertEquals(1, logAppender.eventsAtLevel(Level.ERROR).size()); + verify(emitter, times(1)).send(any(SseEmitter.SseEventBuilder.class)); + verify(emitter, times(1)).completeWithError(any()); + } + + /** + * A WFS server that is not authorized must be logged at WARN with the + * exception attached (its message names the server) and reported to the + * client, without raising an ERROR alert. + */ + @Test + void verifyUnauthorizedServerLoggedAtWarnLevel() throws IOException { + SseEmitter emitter = mock(SseEmitter.class); + + WfsErrorHandler.handleError( + new UnauthorizedServerException("Server http://not-allowed/wfs is not authorized"), + "uuid-123", emitter, null); + + assertTrue(logAppender.eventsAtLevel(Level.ERROR).isEmpty()); + List warns = logAppender.eventsAtLevel(Level.WARN); + assertEquals(1, warns.size()); + assertTrue(warns.get(0).getMessage().getFormattedMessage().contains("uuid-123")); + assertNotNull(warns.get(0).getThrown(), "exception expected in log for server identification"); + + verify(emitter, times(1)).send(any(SseEmitter.SseEventBuilder.class)); + verify(emitter, times(1)).completeWithError(any()); + } + + /** + * No downloadable fields found must be logged at WARN with the exception + * attached and reported to the client, without raising an ERROR alert; the + * per-server failures behind it are logged separately by WfsServer. + */ + @Test + void verifyDownloadableFieldsNotFoundLoggedAtWarnLevel() throws IOException { + SseEmitter emitter = mock(SseEmitter.class); + + WfsErrorHandler.handleError( + new GeoserverFieldsNotFoundException("No downloadable fields found for all url"), + "uuid-123", emitter, null); + + assertTrue(logAppender.eventsAtLevel(Level.ERROR).isEmpty()); + List warns = logAppender.eventsAtLevel(Level.WARN); + assertEquals(1, warns.size()); + assertTrue(warns.get(0).getMessage().getFormattedMessage().contains("uuid-123")); + assertNotNull(warns.get(0).getThrown(), "exception expected in log"); + + verify(emitter, times(1)).send(any(SseEmitter.SseEventBuilder.class)); + verify(emitter, times(1)).completeWithError(any()); + } + + /** + * A client disconnect stays at WARN level so New Relic error alerting is not + * flooded with failures that are not server problems. + */ + @Test + void verifyClientDisconnectStaysAtWarnLevel() { + SseEmitter emitter = mock(SseEmitter.class); + + WfsErrorHandler.handleError(new IOException("Broken pipe"), "uuid-123", emitter, null); + + assertTrue(logAppender.eventsAtLevel(Level.ERROR).isEmpty()); + assertEquals(1, logAppender.eventsAtLevel(Level.WARN).size()); + verify(emitter, times(1)).completeWithError(any()); + } +} diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/core/service/geoserver/wfs/WfsServerTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/core/service/geoserver/wfs/WfsServerTest.java index 88f67cd3..4c0dfd2e 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/core/service/geoserver/wfs/WfsServerTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/core/service/geoserver/wfs/WfsServerTest.java @@ -1,6 +1,10 @@ package au.org.aodn.ogcapi.server.core.service.geoserver.wfs; import au.org.aodn.ogcapi.server.core.exception.GeoserverFieldsNotFoundException; +import au.org.aodn.ogcapi.server.core.util.TestLogAppender; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.core.LogEvent; +import org.springframework.web.client.ResourceAccessException; import au.org.aodn.stac.model.StacCollectionModel; import au.org.aodn.stac.model.LinkModel; import au.org.aodn.ogcapi.server.core.model.ogc.FeatureRequest; @@ -311,6 +315,55 @@ public void testGetDownloadableFieldsNetworkError() { assertTrue(exception.getMessage().contains("Connection timeout")); } + /** + * An unreachable WFS server during DescribeFeatureType must leave a WARN log + * entry naming the failing url, so New Relic can identify the problematic + * server even though the failure surfaces as "no downloadable fields". + */ + @Test + public void testGetDownloadableFieldsUnreachableServerIsLoggedWithUrl() { + WfsServer.WfsFeatureRequest request = WfsServer.WfsFeatureRequest.builder().layerName("test:layer").build(); + + ElasticSearchBase.SearchResult stac = new ElasticSearchBase.SearchResult<>(); + stac.setCollections(List.of( + StacCollectionModel.builder() + .links(List.of( + LinkModel.builder() + .href("http://external-geoserver.example.org/geoserver/ows") + .title(request.getLayerName()) + .aiGroup(WFS_LINK_MARKER) + .build()) + ) + .build() + )); + + String id = "id6"; + + when(mockSearch.searchCollections(eq(id))) + .thenReturn(stac); + + when(restTemplate.exchange(any(String.class), eq(HttpMethod.GET), eq(entity), eq(String.class))) + .thenThrow(new ResourceAccessException("I/O error on GET request: Connection refused")); + + WfsServer server = new WfsServer(mockSearch, restTemplate, new RestTemplateUtils(restTemplate), entity, wfsDefaultParam); + + TestLogAppender logAppender = TestLogAppender.attachTo(WfsServer.class); + try { + assertThrows( + GeoserverFieldsNotFoundException.class, + () -> server.getDownloadableFields(id, request) + ); + + List warns = logAppender.eventsAtLevel(Level.WARN); + assertEquals(1, warns.size(), "Failing server should be logged at WARN"); + String message = warns.get(0).getMessage().getFormattedMessage(); + assertTrue(message.contains("external-geoserver.example.org"), "Log should name the failing server"); + assertTrue(message.contains("Connection refused"), "Log should include the failure cause"); + } finally { + logAppender.detachFrom(WfsServer.class); + } + } + @Test public void testGetDownloadableFieldsNoCollection() { WfsServer.WfsFeatureRequest request = WfsServer.WfsFeatureRequest.builder().layerName("test:layer").build(); diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/core/util/TestLogAppender.java b/server/src/test/java/au/org/aodn/ogcapi/server/core/util/TestLogAppender.java new file mode 100644 index 00000000..23fbdc84 --- /dev/null +++ b/server/src/test/java/au/org/aodn/ogcapi/server/core/util/TestLogAppender.java @@ -0,0 +1,49 @@ +package au.org.aodn.ogcapi.server.core.util; + +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.Logger; +import org.apache.logging.log4j.core.appender.AbstractAppender; +import org.apache.logging.log4j.core.config.Configurator; +import org.apache.logging.log4j.core.config.Property; + +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; + +/** + * Captures log4j2 events emitted by a single class so tests can assert on log + * level and content (e.g. that download failures are logged at ERROR for New + * Relic). Unit tests do not load log4j2-spring.xml, so attaching also lowers + * the logger level to DEBUG to make sure events reach the appender. + */ +public class TestLogAppender extends AbstractAppender { + + private final List events = new CopyOnWriteArrayList<>(); + + private TestLogAppender(String name) { + super(name, null, null, true, Property.EMPTY_ARRAY); + } + + public static TestLogAppender attachTo(Class clazz) { + TestLogAppender appender = new TestLogAppender("test-" + clazz.getSimpleName()); + appender.start(); + Configurator.setLevel(clazz.getName(), Level.DEBUG); + ((Logger) LogManager.getLogger(clazz)).addAppender(appender); + return appender; + } + + public void detachFrom(Class clazz) { + ((Logger) LogManager.getLogger(clazz)).removeAppender(this); + stop(); + } + + public List eventsAtLevel(Level level) { + return events.stream().filter(event -> event.getLevel() == level).toList(); + } + + @Override + public void append(LogEvent event) { + events.add(event.toImmutable()); + } +} diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/service/wfs/DownloadWfsDataServiceTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/service/wfs/DownloadWfsDataServiceTest.java index 1dc96d71..b5697594 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/service/wfs/DownloadWfsDataServiceTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/service/wfs/DownloadWfsDataServiceTest.java @@ -1,14 +1,22 @@ package au.org.aodn.ogcapi.server.service.wfs; import au.org.aodn.ogcapi.server.core.model.ogc.FeatureRequest; +import au.org.aodn.ogcapi.server.core.util.TestLogAppender; import com.fasterxml.jackson.databind.ObjectMapper; import au.org.aodn.ogcapi.server.core.service.geoserver.wfs.DownloadWfsDataService; import au.org.aodn.ogcapi.server.core.service.geoserver.wfs.WfsServer; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.core.LogEvent; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; import org.springframework.http.HttpEntity; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; import org.springframework.http.client.ClientHttpResponse; +import org.springframework.web.client.HttpServerErrorException; +import org.springframework.web.client.ResourceAccessException; import org.springframework.web.client.ResponseExtractor; import org.springframework.web.client.RestTemplate; import org.springframework.web.servlet.mvc.method.annotation.SseEmitter; @@ -21,8 +29,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; -import static org.junit.jupiter.api.Assertions.assertArrayEquals; -import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; public class DownloadWfsDataServiceTest { @@ -33,6 +40,22 @@ public class DownloadWfsDataServiceTest { @Mock protected HttpEntity entity; + protected TestLogAppender logAppender; + + @BeforeEach + void attachLogAppender() { + logAppender = TestLogAppender.attachTo(DownloadWfsDataService.class); + } + + @AfterEach + void detachLogAppender() { + logAppender.detachFrom(DownloadWfsDataService.class); + } + + private List errorLogs() { + return logAppender.eventsAtLevel(Level.ERROR); + } + /** * Test a text file from source is break down into chunk correct and reconstruct correctly * @@ -212,4 +235,80 @@ void verifyDecodeBinaryCorrectlyForSSE() throws Exception { assertArrayEquals(originalBytes, reconstructed.toByteArray()); } + + /** + * A 4xx/5xx from the WFS server must produce an ERROR level log entry that + * contains the server URL, so New Relic can identify the problematic server. + */ + @Test + void verifyUpstreamHttpErrorIsLoggedAtErrorLevelWithServerUrl() { + RestTemplate restTemplateMock = mock(RestTemplate.class); + DownloadWfsDataService service = new DownloadWfsDataService( + wfsServer, restTemplateMock, entity, 10, new ObjectMapper()); + + String wfsRequestUrl = "http://external-geoserver/wfs?request=GetFeature"; + doThrow(new HttpServerErrorException(HttpStatus.INTERNAL_SERVER_ERROR)) + .when(restTemplateMock).execute(anyString(), eq(HttpMethod.GET), any(), any()); + + SseEmitter emitter = new SseEmitter(Long.MAX_VALUE); + + assertThrows(HttpServerErrorException.class, () -> service.streamWfsDataWithSse( + wfsRequestUrl, "uuid-123", "layer:test", + FeatureRequest.GeoServerOutputFormat.CSV.getValue(), + emitter, new AtomicBoolean())); + + List errors = errorLogs(); + assertEquals(1, errors.size()); + assertTrue(errors.get(0).getMessage().getFormattedMessage().contains(wfsRequestUrl)); + assertTrue(errors.get(0).getMessage().getFormattedMessage().contains("uuid-123")); + } + + /** + * An unreachable WFS server (connection refused / DNS / timeout before any + * response) must also produce an ERROR level log entry with the server URL. + */ + @Test + void verifyUnreachableServerIsLoggedAtErrorLevelWithServerUrl() { + RestTemplate restTemplateMock = mock(RestTemplate.class); + DownloadWfsDataService service = new DownloadWfsDataService( + wfsServer, restTemplateMock, entity, 10, new ObjectMapper()); + + String wfsRequestUrl = "http://external-geoserver/wfs?request=GetFeature"; + doThrow(new ResourceAccessException("I/O error on GET request: Connection refused")) + .when(restTemplateMock).execute(anyString(), eq(HttpMethod.GET), any(), any()); + + SseEmitter emitter = new SseEmitter(Long.MAX_VALUE); + + assertThrows(ResourceAccessException.class, () -> service.streamWfsDataWithSse( + wfsRequestUrl, "uuid-123", "layer:test", + FeatureRequest.GeoServerOutputFormat.CSV.getValue(), + emitter, new AtomicBoolean(false))); + + List errors = errorLogs(); + assertEquals(1, errors.size()); + assertTrue(errors.get(0).getMessage().getFormattedMessage().contains(wfsRequestUrl)); + } + + /** + * A client dropping the SSE connection mid-stream (after the WFS server has + * responded) is not a server failure and must not create an ERROR level entry. + */ + @Test + void verifyClientDisconnectAfterServerRespondedIsNotLoggedAsError() { + RestTemplate restTemplateMock = mock(RestTemplate.class); + DownloadWfsDataService service = new DownloadWfsDataService( + wfsServer, restTemplateMock, entity, 10, new ObjectMapper()); + + doThrow(new ResourceAccessException("I/O error on GET request: Broken pipe")) + .when(restTemplateMock).execute(anyString(), eq(HttpMethod.GET), any(), any()); + + SseEmitter emitter = new SseEmitter(Long.MAX_VALUE); + + assertThrows(ResourceAccessException.class, () -> service.streamWfsDataWithSse( + "http://mock/wfs?...", "uuid-123", "layer:test", + FeatureRequest.GeoServerOutputFormat.CSV.getValue(), + emitter, new AtomicBoolean(true))); + + assertTrue(errorLogs().isEmpty()); + } }