From 68c1ee06bd0fb5bc8e9134bfc771ba088023db20 Mon Sep 17 00:00:00 2001 From: Maxime Perrault Date: Wed, 26 Jun 2024 10:34:32 +0200 Subject: [PATCH 1/2] fix: add timeout and log to rapportnav api call --- .../APIRapportNavMissionActionsRepository.kt | 40 +++++++++++-------- .../APIRapportNavActionsRepositoryITests.kt | 36 +++++++++++++++++ 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/backend/src/main/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavMissionActionsRepository.kt b/backend/src/main/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavMissionActionsRepository.kt index 6eb850969..b7f59f62b 100644 --- a/backend/src/main/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavMissionActionsRepository.kt +++ b/backend/src/main/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavMissionActionsRepository.kt @@ -6,10 +6,13 @@ import fr.gouv.cacem.monitorenv.domain.entities.mission.rapportnav.RapportNavMis import fr.gouv.cacem.monitorenv.domain.repositories.IRapportNavMissionActionsRepository import io.ktor.client.call.* import io.ktor.client.request.* +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout import org.slf4j.Logger import org.slf4j.LoggerFactory import org.springframework.stereotype.Repository +import java.net.http.HttpTimeoutException @Repository class APIRapportNavMissionActionsRepository( @@ -23,24 +26,29 @@ class APIRapportNavMissionActionsRepository( "${rapportnavProperties.url}/api/v1/missions/$missionId" return runBlocking { - try { - val rapportNavMissionActions = - apiClient - .httpClient - .get(missionActionsUrl) - .body() - logger.info( - "Fetched is mission has actions and the result is : $rapportNavMissionActions", - ) + withTimeout(3000L) { + try { + val rapportNavMissionActions = + apiClient + .httpClient + .get(missionActionsUrl) + .body() + logger.info( + "Fetched is mission has actions and the result is : $rapportNavMissionActions", + ) - return@runBlocking rapportNavMissionActions - } catch (e: Exception) { - logger.error( - "Could not fetch mission actions from rapportNav at $missionActionsUrl", - e, - ) + return@withTimeout rapportNavMissionActions + } catch (e: CancellationException) { + logger.error("Timeout while fetching rapportNav $missionActionsUrl", e) + throw HttpTimeoutException(e.message) + } catch (e: Exception) { + logger.error( + "Could not fetch mission actions from rapportNav at $missionActionsUrl", + e, + ) - throw NoSuchElementException() + throw NoSuchElementException("Could not fetch mission actions from rapportNav at $missionActionsUrl") + } } } } diff --git a/backend/src/test/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavActionsRepositoryITests.kt b/backend/src/test/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavActionsRepositoryITests.kt index f45e7c893..8fbbb56c6 100644 --- a/backend/src/test/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavActionsRepositoryITests.kt +++ b/backend/src/test/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavActionsRepositoryITests.kt @@ -7,7 +7,10 @@ import io.ktor.http.* import io.ktor.utils.io.* import kotlinx.coroutines.runBlocking import org.assertj.core.api.Assertions +import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import java.net.http.HttpTimeoutException class APIRapportNavActionsRepositoryITests { @@ -66,4 +69,37 @@ class APIRapportNavActionsRepositoryITests { Assertions.assertThat(missionActions.containsActionsAddedByUnit).isFalse() } } + + @Test + fun `findRapportNavMissionActionsById should throw CancellationException after X ms when rapportNav doesnt answer`() { + runBlocking { + val mockEngine = MockEngine { _ -> + Thread.sleep(5000) + respond( + content = ByteReadChannel( + """ + { + "id": 1, + "containsActionsAddedByUnit": false + } + """, + ), + status = HttpStatusCode.OK, + headers = headersOf(HttpHeaders.ContentType, "application/json"), + ) + } + val apiClient = ApiClient(mockEngine) + val rapportnavProperties = RapportnavProperties() + rapportnavProperties.url = "http://test" + + // When + val httpTimeoutException = assertThrows { + APIRapportNavMissionActionsRepository(apiClient, rapportnavProperties).findRapportNavMissionActionsById( + 1, + ) + } + assertThat(httpTimeoutException.message).isEqualTo("Timed out waiting for 3000 ms") + + } + } } From 4fdba7dbff034939ad6a38a641180d39c3ad5c51 Mon Sep 17 00:00:00 2001 From: Maxime Perrault Date: Wed, 26 Jun 2024 13:28:09 +0200 Subject: [PATCH 2/2] review: put timeout into properties with default value of 3000 --- .../cacem/monitorenv/config/RapportnavProperties.kt | 2 ++ .../APIRapportNavMissionActionsRepository.kt | 6 ++++-- backend/src/main/resources/application.properties | 11 ++--------- .../APIRapportNavActionsRepositoryITests.kt | 4 ++-- .../backend/application-integration.properties | 2 -- .../backend/application-prod.properties | 2 -- 6 files changed, 10 insertions(+), 17 deletions(-) diff --git a/backend/src/main/kotlin/fr/gouv/cacem/monitorenv/config/RapportnavProperties.kt b/backend/src/main/kotlin/fr/gouv/cacem/monitorenv/config/RapportnavProperties.kt index 0063ca843..55c39bd49 100644 --- a/backend/src/main/kotlin/fr/gouv/cacem/monitorenv/config/RapportnavProperties.kt +++ b/backend/src/main/kotlin/fr/gouv/cacem/monitorenv/config/RapportnavProperties.kt @@ -1,4 +1,5 @@ package fr.gouv.cacem.monitorenv.config + import org.springframework.boot.context.properties.ConfigurationProperties import org.springframework.stereotype.Component @@ -6,4 +7,5 @@ import org.springframework.stereotype.Component @ConfigurationProperties(prefix = "rapportnav") class RapportnavProperties { var url: String = "" + var timeout: Long = 3000 } diff --git a/backend/src/main/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavMissionActionsRepository.kt b/backend/src/main/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavMissionActionsRepository.kt index b7f59f62b..cd38693d7 100644 --- a/backend/src/main/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavMissionActionsRepository.kt +++ b/backend/src/main/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavMissionActionsRepository.kt @@ -26,7 +26,7 @@ class APIRapportNavMissionActionsRepository( "${rapportnavProperties.url}/api/v1/missions/$missionId" return runBlocking { - withTimeout(3000L) { + withTimeout(rapportnavProperties.timeout) { try { val rapportNavMissionActions = apiClient @@ -47,7 +47,9 @@ class APIRapportNavMissionActionsRepository( e, ) - throw NoSuchElementException("Could not fetch mission actions from rapportNav at $missionActionsUrl") + throw NoSuchElementException( + "Could not fetch mission actions from rapportNav at $missionActionsUrl", + ) } } } diff --git a/backend/src/main/resources/application.properties b/backend/src/main/resources/application.properties index b6d954951..85fa4e401 100644 --- a/backend/src/main/resources/application.properties +++ b/backend/src/main/resources/application.properties @@ -1,24 +1,18 @@ - server.port=${monitorenv.server.port} server.use-forward-headers=true host.ip=${host.ip} monitorenv.ajp.port=${monitorenv.ajp.port} spring.jmx.enabled=true - spring.mvc.static-path-pattern=/** spring.web.resources.static-locations=file:${STATIC_FILES_PATH} - spring.flyway.locations=${monitorenv.flyway.locations} - spring.jpa.hibernate.ddl-auto=validate spring.jpa.properties.hibernate.temp.use_jdbc_metadata_defaults=false spring.jpa.properties.hibernate.dialect=org.hibernate.spatial.dialect.postgis.PostgisPG95Dialect -spring.jpa.properties.hibernate.jdbc.time_zone = UTC - +spring.jpa.properties.hibernate.jdbc.time_zone=UTC spring.datasource.url=${env.db.url} spring.datasource.driver-class-name=org.postgresql.Driver spring.datasource.hikari.maxLifetime=60000 - # Whether response compression is enabled. server.compression.enabled=true # List of user-agents to exclude from compression. @@ -27,11 +21,9 @@ server.compression.excluded-user-agents= server.compression.mime-types=text/html,text/xml,text/plain,text/css,text/javascript,application/javascript # Minimum "Content-Length" value that is required for compression to be performed. server.compression.min-response-size=2048 - management.endpoint.health.show-details=always management.endpoint.metrics.enable=false management.endpoints.web.exposure.include=* - monitorenv.sentry.enabled=${monitorenv.sentry.enabled} monitorenv.sentry.environment=${monitorenv.sentry.environment} monitorenv.sentry.dsn=${SENTRY_DSN} @@ -39,3 +31,4 @@ monitorenv.version=${VERSION} monitorfish.url=${monitorfish.url} monitorfish.xApiKey=${monitorfish.api.key} rapportnav.url=${rapportnav.url} +rapportnav.timeout=3000 diff --git a/backend/src/test/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavActionsRepositoryITests.kt b/backend/src/test/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavActionsRepositoryITests.kt index 8fbbb56c6..772695f27 100644 --- a/backend/src/test/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavActionsRepositoryITests.kt +++ b/backend/src/test/kotlin/fr/gouv/cacem/monitorenv/infrastructure/rapportnav/APIRapportNavActionsRepositoryITests.kt @@ -71,7 +71,7 @@ class APIRapportNavActionsRepositoryITests { } @Test - fun `findRapportNavMissionActionsById should throw CancellationException after X ms when rapportNav doesnt answer`() { + fun `findRapportNavMissionActionsById should throw HttpTimeoutException after X ms when rapportNav doesnt answer`() { runBlocking { val mockEngine = MockEngine { _ -> Thread.sleep(5000) @@ -91,6 +91,7 @@ class APIRapportNavActionsRepositoryITests { val apiClient = ApiClient(mockEngine) val rapportnavProperties = RapportnavProperties() rapportnavProperties.url = "http://test" + rapportnavProperties.timeout = 3000 // When val httpTimeoutException = assertThrows { @@ -99,7 +100,6 @@ class APIRapportNavActionsRepositoryITests { ) } assertThat(httpTimeoutException.message).isEqualTo("Timed out waiting for 3000 ms") - } } } diff --git a/infra/configurations/backend/application-integration.properties b/infra/configurations/backend/application-integration.properties index 819b4bd48..cc7f6e37d 100644 --- a/infra/configurations/backend/application-integration.properties +++ b/infra/configurations/backend/application-integration.properties @@ -5,9 +5,7 @@ monitorenv.server.port=8880 monitorenv.flyway.locations=classpath:/db/migration monitorenv.ajp.port=8000 host.ip=${HOST_IP} - monitorenv.sentry.environment=integration monitorenv.sentry.enabled=true - monitorfish.url=http://monitorfish-test.csam.e2.rie.gouv.fr rapportnav.url=http://int-rapportnav-appli01.dsi.damgm.i2 diff --git a/infra/configurations/backend/application-prod.properties b/infra/configurations/backend/application-prod.properties index 9ba248789..68f730498 100644 --- a/infra/configurations/backend/application-prod.properties +++ b/infra/configurations/backend/application-prod.properties @@ -5,10 +5,8 @@ monitorenv.server.port=8880 monitorenv.flyway.locations=classpath:/db/migration monitorenv.ajp.port=8000 host.ip=${HOST_IP} - monitorenv.sentry.environment=production monitorenv.sentry.enabled=true sentry.dsn=https://a5f3272efa794bb9ada2ffea90f2fec5@sentry.incubateur.net/8 - monitorfish.url=https://monitorfish.din.developpement-durable.gouv.fr rapportnav.url=https://rapport-nav.din.developpement-durable.gouv.fr