From 721de783b707d48448be6579e8fa9adacc4ef26c Mon Sep 17 00:00:00 2001 From: Ewan Donovan Date: Fri, 31 Jan 2025 16:28:36 +0000 Subject: [PATCH] Added timeout and tested --- build.gradle.kts | 2 ++ .../config/HmppsBoldLrsExceptionHandler.kt | 17 ++++++++++ .../config/HttpClientConfiguration.kt | 7 ++++ src/main/resources/application-dev.yml | 5 ++- src/main/resources/application-local.yml | 5 ++- src/main/resources/application.yml | 5 ++- .../HmppsBoldLrsExceptionHandlerTest.kt | 33 +++++++++++++++++++ .../config/TestExceptionResource.kt | 24 +++++++++++++- src/test/resources/application-test.yml | 5 ++- 9 files changed, 98 insertions(+), 5 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 0c486e8..a22b60d 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -25,6 +25,8 @@ dependencies { exclude(group = "io.swagger.core.v3") } testImplementation("org.jetbrains.kotlinx:kotlinx-coroutines-test:1.10.1") + testImplementation("com.squareup.okhttp3:mockwebserver:4.10.0") + } kotlin { diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/HmppsBoldLrsExceptionHandler.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/HmppsBoldLrsExceptionHandler.kt index 33c3f5e..1166436 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/HmppsBoldLrsExceptionHandler.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/HmppsBoldLrsExceptionHandler.kt @@ -12,6 +12,7 @@ import org.springframework.web.context.request.WebRequest import org.springframework.web.servlet.resource.NoResourceFoundException import uk.gov.justice.digital.hmpps.learnerrecordsapi.logging.LoggerUtil import uk.gov.justice.digital.hmpps.learnerrecordsapi.models.lrsapi.response.exceptions.LRSException +import java.net.SocketTimeoutException @RestControllerAdvice class HmppsBoldLrsExceptionHandler { @@ -125,4 +126,20 @@ class HmppsBoldLrsExceptionHandler { log.error(ex.message.orEmpty()) return ResponseEntity(errorResponse, HttpStatus.INTERNAL_SERVER_ERROR) } + + @ExceptionHandler(SocketTimeoutException::class) + fun handleSocketTimeoutException( + ex: SocketTimeoutException, + request: WebRequest, + ): ResponseEntity { + val errorResponse = ErrorResponse( + status = HttpStatus.REQUEST_TIMEOUT, + errorCode = "Request Timeout", + userMessage = "A request to an upstream service timed out.", + developerMessage = "${ex.message}", + moreInfo = "A request timed out while waiting for a response from an upstream service.", + ) + log.error("Socket Timeout Error: {}", ex) + return ResponseEntity(errorResponse, HttpStatus.REQUEST_TIMEOUT) + } } diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/HttpClientConfiguration.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/HttpClientConfiguration.kt index d810c3b..a54bf3a 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/HttpClientConfiguration.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/HttpClientConfiguration.kt @@ -8,11 +8,15 @@ import org.springframework.beans.factory.annotation.Value import org.springframework.context.annotation.Configuration import retrofit2.Retrofit import retrofit2.converter.jaxb.JaxbConverterFactory +import java.util.concurrent.TimeUnit @Configuration class HttpClientConfiguration( @Value("\${lrs.pfx-path}") val pfxFilePath: String, @Value("\${lrs.base-url}") val baseUrl: String, + @Value("\${lrs.connectTimeoutSeconds}") val connectTimeoutSeconds: Int, + @Value("\${lrs.writeTimeoutSeconds}") val writeTimeoutSeconds: Int, + @Value("\${lrs.readTimeoutSeconds}") val readTimeoutSeconds: Int, ) { fun buildSSLHttpClient(): OkHttpClient { log.info("Building HTTP client with SSL") @@ -25,6 +29,9 @@ class HttpClientConfiguration( val trustManager = sslContextConfiguration.getTrustManager() val httpClientBuilder = OkHttpClient.Builder() + .connectTimeout(connectTimeoutSeconds.toLong(), TimeUnit.SECONDS) + .writeTimeout(writeTimeoutSeconds.toLong(), TimeUnit.SECONDS) + .readTimeout(readTimeoutSeconds.toLong(), TimeUnit.SECONDS) .sslSocketFactory(sslContext.socketFactory, trustManager) .addInterceptor(loggingInterceptor) diff --git a/src/main/resources/application-dev.yml b/src/main/resources/application-dev.yml index 554115d..3a0f06c 100644 --- a/src/main/resources/application-dev.yml +++ b/src/main/resources/application-dev.yml @@ -3,4 +3,7 @@ hmpps-auth: lrs: base-url: "https://cmp-ws.dev.lrs.education.gov.uk" - pfx-path: "WebServiceClientCert.pfx" \ No newline at end of file + pfx-path: "WebServiceClientCert.pfx" + connectTimeoutSeconds: 20 + writeTimeoutSeconds: 20 + readTimeoutSeconds: 20 \ No newline at end of file diff --git a/src/main/resources/application-local.yml b/src/main/resources/application-local.yml index 06cef12..98e0e1b 100644 --- a/src/main/resources/application-local.yml +++ b/src/main/resources/application-local.yml @@ -3,4 +3,7 @@ hmpps-auth: lrs: base-url: "http://lrs-api:8080" - pfx-path: "WebServiceClientCert.pfx" \ No newline at end of file + pfx-path: "WebServiceClientCert.pfx" + connectTimeoutSeconds: 20 + writeTimeoutSeconds: 20 + readTimeoutSeconds: 20 \ No newline at end of file diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index bc48298..424855b 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -53,4 +53,7 @@ management: lrs: base-url: ${lrs.base-url} - pfx-path: ${lrs.pfx-path} \ No newline at end of file + pfx-path: ${lrs.pfx-path} + connectTimeoutSeconds: 20 + writeTimeoutSeconds: 20 + readTimeoutSeconds: 20 \ No newline at end of file diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/HmppsBoldLrsExceptionHandlerTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/HmppsBoldLrsExceptionHandlerTest.kt index f29a8ac..9881086 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/HmppsBoldLrsExceptionHandlerTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/HmppsBoldLrsExceptionHandlerTest.kt @@ -2,12 +2,14 @@ package uk.gov.justice.digital.hmpps.learnerrecordsapi.config import com.google.gson.GsonBuilder import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.springframework.http.HttpStatus import uk.gov.justice.digital.hmpps.learnerrecordsapi.integration.IntegrationTestBase import uk.gov.justice.digital.hmpps.learnerrecordsapi.models.gsonadapters.LocalDateAdapter import uk.gov.justice.digital.hmpps.learnerrecordsapi.models.gsonadapters.ResponseTypeAdapter import uk.gov.justice.digital.hmpps.learnerrecordsapi.models.response.LRSResponseType +import java.time.Duration import java.time.LocalDate // Tests that when exceptions are thrown, the exception handler will pick them up and behave correctly. @@ -15,6 +17,13 @@ import java.time.LocalDate class HmppsBoldLrsExceptionHandlerTest : IntegrationTestBase() { + @BeforeEach + fun setUp() { + webTestClient = webTestClient.mutate() + .responseTimeout(Duration.ofMillis(180000)) + .build() + } + val gson = GsonBuilder() .registerTypeAdapter(LocalDate::class.java, LocalDateAdapter().nullSafe()) .registerTypeAdapter(LRSResponseType::class.java, ResponseTypeAdapter().nullSafe()) @@ -104,6 +113,30 @@ class HmppsBoldLrsExceptionHandlerTest : IntegrationTestBase() { testExceptionHandling("/test/forbidden", expectedResponse, expectedStatus = HttpStatus.FORBIDDEN) } + @Test + fun `should catch timeout exceptions (SocketTimeoutException) and return Gateway Timeout`() { + val expectedResponse = HmppsBoldLrsExceptionHandler.ErrorResponse( + status = HttpStatus.REQUEST_TIMEOUT, + errorCode = "Request Timeout", + userMessage = "A request to an upstream service timed out.", + developerMessage = "Read timed out", + moreInfo = "A request timed out while waiting for a response from an upstream service.", + ) + + val actualResponse = webTestClient.post() + .uri("/test/okhttp-timeout") + .headers(setAuthorisation(roles = listOf("ROLE_TEMPLATE_KOTLIN__UI"))) + .exchange() + .expectStatus() + .isEqualTo(HttpStatus.REQUEST_TIMEOUT) + .expectBody() + .returnResult() + .responseBody + + val actualResponseString = actualResponse?.toString(Charsets.UTF_8) + assertThat(actualResponseString).isEqualTo(gson.toJson(expectedResponse)) + } + @Test fun `should catch generic exceptions and return Internal Server Error`() { val expectedResponse = HmppsBoldLrsExceptionHandler.ErrorResponse( diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/TestExceptionResource.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/TestExceptionResource.kt index 3379f4e..fe55b05 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/TestExceptionResource.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/learnerrecordsapi/config/TestExceptionResource.kt @@ -1,7 +1,11 @@ package uk.gov.justice.digital.hmpps.learnerrecordsapi.config import com.google.gson.JsonParseException +import okhttp3.Request +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer import org.mockito.Mockito +import org.springframework.beans.factory.annotation.Autowired import org.springframework.core.MethodParameter import org.springframework.http.HttpInputMessage import org.springframework.http.converter.HttpMessageNotReadableException @@ -12,11 +16,15 @@ import org.springframework.web.bind.annotation.PostMapping import org.springframework.web.bind.annotation.RestController import uk.gov.justice.digital.hmpps.learnerrecordsapi.models.lrsapi.response.exceptions.LRSException import java.io.File +import java.util.concurrent.TimeUnit // Simply to assist in testing HmppsBoldLrsExceptionHandler, the endpoints are only visible to tests. @RestController -class TestExceptionResource { +class TestExceptionResource( + @Autowired + private val httpClientConfiguration: HttpClientConfiguration, +) { @PostMapping("/test/validation") fun triggerValidationException(): Nothing = throw MethodArgumentNotValidException( @@ -41,4 +49,18 @@ class TestExceptionResource { @PostMapping("/test/generic-exception") fun triggerGenericException(): Nothing = throw Exception() + + @PostMapping("/test/okhttp-timeout") + fun triggerOkhttpTimeout(): String? { + val mockTimeoutServer = MockWebServer() + mockTimeoutServer.enqueue(MockResponse().setBody("Delayed response").setBodyDelay(15, TimeUnit.SECONDS)) + mockTimeoutServer.start() + + val request = Request.Builder() + .url(mockTimeoutServer.url("/timeout")) // Point to the MockWebServer URL + .build() + + val response = httpClientConfiguration.buildSSLHttpClient().newCall(request).execute() + return response.body?.string() + } } diff --git a/src/test/resources/application-test.yml b/src/test/resources/application-test.yml index 5daff14..6095b68 100644 --- a/src/test/resources/application-test.yml +++ b/src/test/resources/application-test.yml @@ -10,4 +10,7 @@ hmpps-auth: lrs: base-url: "http://localhost:8082" - pfx-path: "WebServiceClientCert.pfx" \ No newline at end of file + pfx-path: "WebServiceClientCert.pfx" + connectTimeoutSeconds: 1 + writeTimeoutSeconds: 1 + readTimeoutSeconds: 1 \ No newline at end of file