Skip to content

Commit acbca8e

Browse files
authored
fix: HTTP exception classification — separate permanent from retriable failures (#44)
## Summary `HttpMessageDeliverer.deliver()` previously caught all exceptions as `RetriableFailure`. This meant **corrupt delivery metadata or an unknown service triggered an infinite retry loop** instead of moving the entry to `FAILED`. This PR classifies exceptions explicitly: | Exception | Classification | Why | |---|---|---| | `JsonProcessingException` | `PermanentFailure` | Corrupt metadata won't fix itself (caught before `IOException` — it's a subtype) | | `IOException` | `RetriableFailure` | Connection / timeout — transient | | `InterruptedException` | `RetriableFailure` | Interrupt flag restored; consistent with `KafkaMessageDeliverer` | | other `Exception` | `PermanentFailure` | Malformed URI, unknown service, `IllegalStateException` from `ServiceUrlResolver`, etc. | ## Why this matters Before: `okapi-http` consumer with malformed JSON in `deliveryMetadata` → infinite retries → retry storm in logs, entry never marked `FAILED`, operator alert fatigue. After: corrupt input → `PermanentFailure` → entry moves to `FAILED` per `RetryPolicy`, ops visibility preserved. ## Scope - `HttpMessageDeliverer.kt`: refactor to expression-body, granular `catch` blocks, updated KDoc - `HttpMessageDelivererTest.kt`: 2 new tests - corrupt metadata → `PermanentFailure` (does not throw) - `urlResolver` throwing → `PermanentFailure` (does not throw) Independent of the Kafka `deliverBatch` work in PR #40. ## Test plan - [x] `./gradlew :okapi-http:test ktlintCheck` — all 13 unit tests pass (11 existing + 2 new)
1 parent fa9afda commit acbca8e

2 files changed

Lines changed: 47 additions & 25 deletions

File tree

‎okapi-http/src/main/kotlin/com/softwaremill/okapi/http/HttpMessageDeliverer.kt‎

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package com.softwaremill.okapi.http
22

3+
import com.fasterxml.jackson.core.JsonProcessingException
34
import com.softwaremill.okapi.core.DeliveryResult
45
import com.softwaremill.okapi.core.MessageDeliverer
56
import com.softwaremill.okapi.core.OutboxEntry
7+
import java.io.IOException
68
import java.net.URI
79
import java.net.http.HttpClient
810
import java.net.http.HttpRequest
@@ -17,7 +19,10 @@ import java.time.Duration
1719
* - 5xx, 429, 408 → [DeliveryResult.RetriableFailure] (configurable via [retriableStatusCodes])
1820
* - other → [DeliveryResult.PermanentFailure]
1921
*
20-
* Connection errors are treated as retriable.
22+
* Exception classification:
23+
* - [IOException] (connection/timeout) → [DeliveryResult.RetriableFailure]
24+
* - [InterruptedException] → [DeliveryResult.RetriableFailure] (interrupt flag restored)
25+
* - other (corrupt metadata, unknown service, malformed URI) → [DeliveryResult.PermanentFailure]
2126
*/
2227
class HttpMessageDeliverer @JvmOverloads constructor(
2328
private val urlResolver: ServiceUrlResolver,
@@ -26,36 +31,42 @@ class HttpMessageDeliverer @JvmOverloads constructor(
2631
) : MessageDeliverer {
2732
override val type: String = HttpDeliveryInfo.TYPE
2833

29-
override fun deliver(entry: OutboxEntry): DeliveryResult {
34+
override fun deliver(entry: OutboxEntry): DeliveryResult = try {
3035
val info = HttpDeliveryInfo.deserialize(entry.deliveryMetadata)
3136
val url = urlResolver.resolve(info.serviceName) + info.endpointPath
3237

33-
return try {
34-
val request =
35-
HttpRequest
36-
.newBuilder()
37-
.uri(URI.create(url))
38-
.timeout(REQUEST_TIMEOUT)
39-
.header("Content-Type", "application/json")
40-
.method(
41-
info.httpMethod.name,
42-
HttpRequest.BodyPublishers.ofString(entry.payload),
43-
)
44-
.apply { info.headers.forEach { (k, v) -> header(k, v) } }
45-
.build()
38+
val request =
39+
HttpRequest
40+
.newBuilder()
41+
.uri(URI.create(url))
42+
.timeout(REQUEST_TIMEOUT)
43+
.header("Content-Type", "application/json")
44+
.method(
45+
info.httpMethod.name,
46+
HttpRequest.BodyPublishers.ofString(entry.payload),
47+
)
48+
.apply { info.headers.forEach { (k, v) -> header(k, v) } }
49+
.build()
4650

47-
val response = httpClient.send(request, HttpResponse.BodyHandlers.ofString())
48-
val status = response.statusCode()
49-
val body = response.body()
51+
val response = httpClient.send(request, HttpResponse.BodyHandlers.ofString())
52+
val status = response.statusCode()
53+
val body = response.body()
5054

51-
when {
52-
status in 200..299 -> DeliveryResult.Success
53-
status in retriableStatusCodes -> DeliveryResult.RetriableFailure("HTTP $status: $body")
54-
else -> DeliveryResult.PermanentFailure("HTTP $status: $body")
55-
}
56-
} catch (e: Exception) {
57-
DeliveryResult.RetriableFailure(e.message ?: "Connection failed")
55+
when {
56+
status in 200..299 -> DeliveryResult.Success
57+
status in retriableStatusCodes -> DeliveryResult.RetriableFailure("HTTP $status: $body")
58+
else -> DeliveryResult.PermanentFailure("HTTP $status: $body")
5859
}
60+
} catch (e: JsonProcessingException) {
61+
// Subtype of IOException, but corrupt metadata won't fix itself — classify before IOException.
62+
DeliveryResult.PermanentFailure(e.message ?: e.javaClass.simpleName)
63+
} catch (e: IOException) {
64+
DeliveryResult.RetriableFailure(e.message ?: "Connection failed")
65+
} catch (e: InterruptedException) {
66+
Thread.currentThread().interrupt()
67+
DeliveryResult.RetriableFailure(e.message ?: "Interrupted")
68+
} catch (e: Exception) {
69+
DeliveryResult.PermanentFailure(e.message ?: e.javaClass.simpleName)
5970
}
6071

6172
companion object {

‎okapi-http/src/test/kotlin/com/softwaremill/okapi/http/HttpMessageDelivererTest.kt‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,4 +90,15 @@ class HttpMessageDelivererTest : FunSpec({
9090
)
9191
deliverer.deliver(entry()).shouldBeInstanceOf<DeliveryResult.RetriableFailure>()
9292
}
93+
94+
test("corrupt delivery metadata -> PermanentFailure (does not throw)") {
95+
val poisoned = entry().copy(deliveryMetadata = "{not valid json")
96+
deliverer.deliver(poisoned).shouldBeInstanceOf<DeliveryResult.PermanentFailure>()
97+
}
98+
99+
test("urlResolver throwing -> PermanentFailure (does not throw)") {
100+
val throwingResolver = ServiceUrlResolver { name -> error("Unknown service: $name") }
101+
val d = HttpMessageDeliverer(throwingResolver)
102+
d.deliver(entry()).shouldBeInstanceOf<DeliveryResult.PermanentFailure>()
103+
}
93104
})

0 commit comments

Comments
 (0)