From 038aa79776d45af6ad23a2f4b2a724dba57911cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20Kobyli=C5=84ski?= Date: Thu, 14 May 2026 11:30:23 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20HTTP=20exception=20classification=20?= =?UTF-8?q?=E2=80=94=20separate=20permanent=20from=20retriable=20failures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, a single generic catch (e: Exception) mapped all errors to RetriableFailure. Corrupt delivery metadata or an unknown service therefore triggered an infinite retry loop instead of moving the entry to FAILED. Now classified explicitly: - JsonProcessingException (corrupt metadata) → PermanentFailure (caught before IOException — it's a subtype) - IOException (connection/timeout) → RetriableFailure - InterruptedException → RetriableFailure (interrupt flag restored, consistent with KafkaMessageDeliverer) - other Exception (malformed URI, unknown service, IllegalStateException from ServiceUrlResolver) → PermanentFailure Adds two unit tests covering the new classification paths: - corrupt delivery metadata → PermanentFailure - urlResolver throwing → PermanentFailure --- .../okapi/http/HttpMessageDeliverer.kt | 61 +++++++++++-------- .../okapi/http/HttpMessageDelivererTest.kt | 11 ++++ 2 files changed, 47 insertions(+), 25 deletions(-) diff --git a/okapi-http/src/main/kotlin/com/softwaremill/okapi/http/HttpMessageDeliverer.kt b/okapi-http/src/main/kotlin/com/softwaremill/okapi/http/HttpMessageDeliverer.kt index 0890ff5..8a93abb 100644 --- a/okapi-http/src/main/kotlin/com/softwaremill/okapi/http/HttpMessageDeliverer.kt +++ b/okapi-http/src/main/kotlin/com/softwaremill/okapi/http/HttpMessageDeliverer.kt @@ -1,8 +1,10 @@ package com.softwaremill.okapi.http +import com.fasterxml.jackson.core.JsonProcessingException import com.softwaremill.okapi.core.DeliveryResult import com.softwaremill.okapi.core.MessageDeliverer import com.softwaremill.okapi.core.OutboxEntry +import java.io.IOException import java.net.URI import java.net.http.HttpClient import java.net.http.HttpRequest @@ -17,7 +19,10 @@ import java.time.Duration * - 5xx, 429, 408 → [DeliveryResult.RetriableFailure] (configurable via [retriableStatusCodes]) * - other → [DeliveryResult.PermanentFailure] * - * Connection errors are treated as retriable. + * Exception classification: + * - [IOException] (connection/timeout) → [DeliveryResult.RetriableFailure] + * - [InterruptedException] → [DeliveryResult.RetriableFailure] (interrupt flag restored) + * - other (corrupt metadata, unknown service, malformed URI) → [DeliveryResult.PermanentFailure] */ class HttpMessageDeliverer @JvmOverloads constructor( private val urlResolver: ServiceUrlResolver, @@ -26,36 +31,42 @@ class HttpMessageDeliverer @JvmOverloads constructor( ) : MessageDeliverer { override val type: String = HttpDeliveryInfo.TYPE - override fun deliver(entry: OutboxEntry): DeliveryResult { + override fun deliver(entry: OutboxEntry): DeliveryResult = try { val info = HttpDeliveryInfo.deserialize(entry.deliveryMetadata) val url = urlResolver.resolve(info.serviceName) + info.endpointPath - return try { - val request = - HttpRequest - .newBuilder() - .uri(URI.create(url)) - .timeout(REQUEST_TIMEOUT) - .header("Content-Type", "application/json") - .method( - info.httpMethod.name, - HttpRequest.BodyPublishers.ofString(entry.payload), - ) - .apply { info.headers.forEach { (k, v) -> header(k, v) } } - .build() + val request = + HttpRequest + .newBuilder() + .uri(URI.create(url)) + .timeout(REQUEST_TIMEOUT) + .header("Content-Type", "application/json") + .method( + info.httpMethod.name, + HttpRequest.BodyPublishers.ofString(entry.payload), + ) + .apply { info.headers.forEach { (k, v) -> header(k, v) } } + .build() - val response = httpClient.send(request, HttpResponse.BodyHandlers.ofString()) - val status = response.statusCode() - val body = response.body() + val response = httpClient.send(request, HttpResponse.BodyHandlers.ofString()) + val status = response.statusCode() + val body = response.body() - when { - status in 200..299 -> DeliveryResult.Success - status in retriableStatusCodes -> DeliveryResult.RetriableFailure("HTTP $status: $body") - else -> DeliveryResult.PermanentFailure("HTTP $status: $body") - } - } catch (e: Exception) { - DeliveryResult.RetriableFailure(e.message ?: "Connection failed") + when { + status in 200..299 -> DeliveryResult.Success + status in retriableStatusCodes -> DeliveryResult.RetriableFailure("HTTP $status: $body") + else -> DeliveryResult.PermanentFailure("HTTP $status: $body") } + } catch (e: JsonProcessingException) { + // Subtype of IOException, but corrupt metadata won't fix itself — classify before IOException. + DeliveryResult.PermanentFailure(e.message ?: e.javaClass.simpleName) + } catch (e: IOException) { + DeliveryResult.RetriableFailure(e.message ?: "Connection failed") + } catch (e: InterruptedException) { + Thread.currentThread().interrupt() + DeliveryResult.RetriableFailure(e.message ?: "Interrupted") + } catch (e: Exception) { + DeliveryResult.PermanentFailure(e.message ?: e.javaClass.simpleName) } companion object { diff --git a/okapi-http/src/test/kotlin/com/softwaremill/okapi/http/HttpMessageDelivererTest.kt b/okapi-http/src/test/kotlin/com/softwaremill/okapi/http/HttpMessageDelivererTest.kt index a5858d2..e6e2e8c 100644 --- a/okapi-http/src/test/kotlin/com/softwaremill/okapi/http/HttpMessageDelivererTest.kt +++ b/okapi-http/src/test/kotlin/com/softwaremill/okapi/http/HttpMessageDelivererTest.kt @@ -90,4 +90,15 @@ class HttpMessageDelivererTest : FunSpec({ ) deliverer.deliver(entry()).shouldBeInstanceOf() } + + test("corrupt delivery metadata -> PermanentFailure (does not throw)") { + val poisoned = entry().copy(deliveryMetadata = "{not valid json") + deliverer.deliver(poisoned).shouldBeInstanceOf() + } + + test("urlResolver throwing -> PermanentFailure (does not throw)") { + val throwingResolver = ServiceUrlResolver { name -> error("Unknown service: $name") } + val d = HttpMessageDeliverer(throwingResolver) + d.deliver(entry()).shouldBeInstanceOf() + } })