Skip to content

Commit 4698808

Browse files
mduesterhoeftmduesterhoeft
authored
Make error responses more flexible. (#7)
* Make error responses more flexible. So that we could also serialize protobuf errors and a client would be able to customize the error format. * Remove println
1 parent e66e855 commit 4698808

File tree

10 files changed

+182
-108
lines changed

10 files changed

+182
-108
lines changed

‎build.gradle.kts

+1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ subprojects {
4545

4646
withType<Test> {
4747
useJUnitPlatform()
48+
testLogging.showStandardStreams = true
4849
}
4950

5051
}

‎router-protobuf/src/main/kotlin/io/moia/router/proto/ProtoSerializationHandler.kt

+4-5
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,18 @@ package io.moia.router.proto
22

33
import com.google.common.net.MediaType
44
import com.google.protobuf.GeneratedMessageV3
5-
import io.moia.router.ResponseEntity
65
import io.moia.router.SerializationHandler
76
import java.util.Base64
87

98
class ProtoSerializationHandler : SerializationHandler {
109

1110
private val json = MediaType.parse("application/json")
1211

13-
override fun supports(acceptHeader: MediaType, response: ResponseEntity<*>): Boolean =
14-
response.body is GeneratedMessageV3
12+
override fun supports(acceptHeader: MediaType, body: Any): Boolean =
13+
body is GeneratedMessageV3
1514

16-
override fun serialize(acceptHeader: MediaType, response: ResponseEntity<*>): String {
17-
val message = response.body as GeneratedMessageV3
15+
override fun serialize(acceptHeader: MediaType, body: Any): String {
16+
val message = body as GeneratedMessageV3
1817
return if (acceptHeader.`is`(json)) {
1918
ProtoBufUtils.toJsonWithoutWrappers(message)
2019
} else {

‎router-protobuf/src/test/kotlin/io/moia/router/proto/RequestHandlerTest.kt

+54-6
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ import io.moia.router.Router.Companion.router
99
import io.moia.router.bodyAsBytes
1010
import io.moia.router.proto.sample.SampleOuterClass.Sample
1111
import io.mockk.mockk
12+
import io.moia.router.ApiError
13+
import io.moia.router.ApiException
14+
import io.moia.router.GET
15+
import io.moia.router.UnprocessableEntityError
1216
import org.junit.jupiter.api.Test
1317
import java.util.Base64
1418

@@ -64,18 +68,62 @@ class RequestHandlerTest {
6468
assert(Sample.parseFrom(response.bodyAsBytes())).isEqualTo(request)
6569
}
6670

71+
@Test
72+
fun `should return 406-unacceptable error in proto`() {
73+
74+
val response = testRequestHandler.handleRequest(
75+
GET("/some-proto")
76+
.withHeaders(mapOf(
77+
"Accept" to "text/plain"
78+
)), mockk()
79+
)
80+
81+
assert(response.statusCode).isEqualTo(406)
82+
assert(io.moia.router.proto.sample.SampleOuterClass.ApiError.parseFrom(response.bodyAsBytes()).getCode()).isEqualTo("NOT_ACCEPTABLE")
83+
}
84+
85+
@Test
86+
fun `should return api error in prots`() {
87+
88+
val response = testRequestHandler.handleRequest(
89+
GET("/some-error")
90+
.withHeaders(mapOf(
91+
"Accept" to "application/x-protobuf"
92+
)), mockk()
93+
)
94+
95+
assert(response.statusCode).isEqualTo(400)
96+
with(io.moia.router.proto.sample.SampleOuterClass.ApiError.parseFrom(response.bodyAsBytes())) {
97+
assert(getCode()).isEqualTo("BOOM")
98+
assert(getMessage()).isEqualTo("boom")
99+
}
100+
}
101+
67102
class TestRequestHandler : ProtoEnabledRequestHandler() {
68103

69104
override val router = router {
70105
defaultProducing = setOf("application/x-protobuf")
71106
defaultConsuming = setOf("application/x-protobuf")
72107

73-
GET("/some-proto") { _: Request<Unit> ->
74-
ResponseEntity.ok(Sample.newBuilder().setHello("Hello").build())
75-
}.producing("application/x-protobuf", "application/json")
76-
POST("/some-proto") { r: Request<Sample> ->
77-
ResponseEntity.ok(r.body)
78-
}
108+
defaultContentType = "application/x-protobuf"
109+
110+
GET("/some-proto") { _: Request<Unit> -> ResponseEntity.ok(Sample.newBuilder().setHello("Hello").build()) }
111+
.producing("application/x-protobuf", "application/json")
112+
POST("/some-proto") { r: Request<Sample> -> ResponseEntity.ok(r.body) }
113+
GET<Unit, Unit>("/some-error") { _: Request<Unit> -> throw ApiException("boom", "BOOM", 400) }
79114
}
115+
116+
override fun createErrorBody(error: ApiError): Any =
117+
io.moia.router.proto.sample.SampleOuterClass.ApiError.newBuilder()
118+
.setMessage(error.message)
119+
.setCode(error.code)
120+
.build()
121+
122+
override fun createUnprocessableEntityErrorBody(error: UnprocessableEntityError): Any =
123+
io.moia.router.proto.sample.SampleOuterClass.UnprocessableEntityError.newBuilder()
124+
.setMessage(error.message)
125+
.setCode(error.code)
126+
.setPath(error.path)
127+
.build()
80128
}
81129
}

‎router-protobuf/src/test/proto/Sample.proto

+11
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,15 @@ package io.moia.router.proto.sample;
55
message Sample {
66
string hello = 1;
77
string request = 2;
8+
}
9+
10+
message ApiError {
11+
string message = 1;
12+
string code = 2;
13+
}
14+
15+
message UnprocessableEntityError {
16+
string message = 1;
17+
string code = 2;
18+
string path = 3;
819
}

‎router/src/main/kotlin/io/moia/router/APIGatewayProxyEventExtensions.kt

+6
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,16 @@ package io.moia.router
22

33
import com.amazonaws.services.lambda.runtime.events.APIGatewayProxyRequestEvent
44
import com.amazonaws.services.lambda.runtime.events.APIGatewayProxyResponseEvent
5+
import com.google.common.net.MediaType
56
import java.net.URI
67
import java.util.Base64
78

89
fun APIGatewayProxyRequestEvent.acceptHeader() = getHeaderCaseInsensitive("accept")
10+
fun APIGatewayProxyRequestEvent.acceptedMediaTypes() = acceptHeader()
11+
?.split(",")
12+
?.map { it.trim() }
13+
?.map { MediaType.parse(it) }
14+
.orEmpty()
915
fun APIGatewayProxyRequestEvent.contentType() = getHeaderCaseInsensitive("content-type")
1016

1117
fun APIGatewayProxyRequestEvent.getHeaderCaseInsensitive(httpHeader: String): String? =

‎router/src/main/kotlin/io/moia/router/ApiException.kt

+15-1
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,18 @@ open class ApiException(
1111
override fun toString(): String {
1212
return "ApiException(message='$message', code='$code', httpResponseStatus=$httpResponseStatus, details=$details, cause=$cause)"
1313
}
14-
}
14+
15+
fun toApiError() =
16+
ApiError(super.message!!, code, details)
17+
}
18+
19+
data class ApiError(
20+
val message: String,
21+
val code: String,
22+
val details: Map<String, Any> = emptyMap())
23+
24+
data class UnprocessableEntityError(
25+
val message: String,
26+
val code: String,
27+
val path: String
28+
)

‎router/src/main/kotlin/io/moia/router/RequestHandler.kt

+74-77
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import org.slf4j.LoggerFactory
1212
import kotlin.reflect.KClass
1313
import kotlin.reflect.jvm.reflect
1414

15+
@Suppress("UnstableApiUsage")
1516
abstract class RequestHandler : RequestHandler<APIGatewayProxyRequestEvent, APIGatewayProxyResponseEvent> {
1617

1718
open val objectMapper = jacksonObjectMapper()
@@ -29,41 +30,39 @@ abstract class RequestHandler : RequestHandler<APIGatewayProxyRequestEvent, APIG
2930
val matchResult = routerFunction.requestPredicate.match(input)
3031
log.debug("match result for route '$routerFunction' is '$matchResult'")
3132
if (matchResult.match) {
33+
val matchedAcceptType = routerFunction.requestPredicate.matchedAcceptType(input.acceptedMediaTypes())
34+
?: MediaType.parse(router.defaultContentType)
3235
if (!permissionHandlerSupplier()(input).hasAnyRequiredPermission(routerFunction.requestPredicate.requiredPermissions))
33-
return createApiExceptionErrorResponse(input, ApiException("unauthorized", "UNAUTHORIZED", 401))
36+
return createApiExceptionErrorResponse(matchedAcceptType, input, ApiException("unauthorized", "UNAUTHORIZED", 401))
3437

3538
val handler: HandlerFunction<Any, Any> = routerFunction.handler
3639
return try {
3740
val requestBody = deserializeRequest(handler, input)
3841
val request =
3942
Request(input, requestBody, routerFunction.requestPredicate.pathPattern)
4043
val response = router.filter.then(handler as HandlerFunction<*, *>).invoke(request)
41-
createResponse(routerFunction.requestPredicate.matchedAcceptType(input.acceptHeader()), input, response)
44+
createResponse(matchedAcceptType, input, response)
4245
} catch (e: Exception) {
4346
when (e) {
44-
is ApiException -> createApiExceptionErrorResponse(input, e)
47+
is ApiException -> createApiExceptionErrorResponse(matchedAcceptType, input, e)
4548
.also { log.info("Caught api error while handling ${input.httpMethod} ${input.path} - $e") }
46-
else -> createUnexpectedErrorResponse(input, e)
49+
else -> createUnexpectedErrorResponse(matchedAcceptType, input, e)
4750
.also { log.error("Caught exception handling ${input.httpMethod} ${input.path} - $e", e) }
4851
}
4952
}
5053
}
5154

5255
matchResult
5356
}
54-
return handleNonDirectMatch(matchResults, input)
57+
return handleNonDirectMatch(MediaType.parse(router.defaultContentType), matchResults, input)
5558
}
5659

5760
open fun serializationHandlers(): List<SerializationHandler> = listOf(
58-
JsonSerializationHandler(
59-
objectMapper
60-
)
61+
JsonSerializationHandler(objectMapper)
6162
)
6263

6364
open fun deserializationHandlers(): List<DeserializationHandler> = listOf(
64-
JsonDeserializationHandler(
65-
objectMapper
66-
)
65+
JsonDeserializationHandler(objectMapper)
6766
)
6867

6968
open fun permissionHandlerSupplier(): (r: APIGatewayProxyRequestEvent) -> PermissionHandler =
@@ -80,89 +79,87 @@ abstract class RequestHandler : RequestHandler<APIGatewayProxyRequestEvent, APIG
8079
}
8180
}
8281

83-
private fun handleNonDirectMatch(matchResults: List<RequestMatchResult>, input: APIGatewayProxyRequestEvent): APIGatewayProxyResponseEvent {
82+
private fun handleNonDirectMatch(defaultContentType: MediaType, matchResults: List<RequestMatchResult>, input: APIGatewayProxyRequestEvent): APIGatewayProxyResponseEvent {
8483
// no direct match
85-
if (matchResults.any { it.matchPath && it.matchMethod && !it.matchContentType }) {
86-
return createApiExceptionErrorResponse(
87-
input, ApiException(
88-
httpResponseStatus = 415,
89-
message = "Unsupported Media Type",
90-
code = "UNSUPPORTED_MEDIA_TYPE"
84+
val apiException =
85+
when {
86+
matchResults.any { it.matchPath && it.matchMethod && !it.matchContentType } ->
87+
ApiException(
88+
httpResponseStatus = 415,
89+
message = "Unsupported Media Type",
90+
code = "UNSUPPORTED_MEDIA_TYPE"
91+
)
92+
matchResults.any { it.matchPath && it.matchMethod && !it.matchAcceptType } ->
93+
ApiException(
94+
httpResponseStatus = 406,
95+
message = "Not Acceptable",
96+
code = "NOT_ACCEPTABLE"
97+
)
98+
matchResults.any { it.matchPath && !it.matchMethod } ->
99+
ApiException(
100+
httpResponseStatus = 405,
101+
message = "Method Not Allowed",
102+
code = "METHOD_NOT_ALLOWED"
91103
)
92-
)
93-
}
94-
if (matchResults.any { it.matchPath && it.matchMethod && !it.matchAcceptType }) {
95-
return createApiExceptionErrorResponse(
96-
input, ApiException(
97-
httpResponseStatus = 406,
98-
message = "Not Acceptable",
99-
code = "NOT_ACCEPTABLE"
104+
else -> ApiException(
105+
httpResponseStatus = 404,
106+
message = "Not found",
107+
code = "NOT_FOUND"
100108
)
101-
)
102-
}
103-
if (matchResults.any { it.matchPath && !it.matchMethod }) {
104-
return createApiExceptionErrorResponse(
105-
input, ApiException(
106-
httpResponseStatus = 405,
107-
message = "Method Not Allowed",
108-
code = "METHOD_NOT_ALLOWED"
109+
}
110+
val contentType = input.acceptedMediaTypes().firstOrNull() ?: defaultContentType
111+
return createApiExceptionErrorResponse(contentType, input, apiException)
112+
}
113+
114+
/**
115+
* Customize the format of an api error
116+
*/
117+
open fun createErrorBody(error: ApiError): Any = error
118+
119+
/**
120+
* Customize the format of an unprocessable entity error
121+
*/
122+
open fun createUnprocessableEntityErrorBody(error: UnprocessableEntityError): Any =
123+
error
124+
125+
open fun createApiExceptionErrorResponse(contentType: MediaType, input: APIGatewayProxyRequestEvent, ex: ApiException): APIGatewayProxyResponseEvent =
126+
createErrorBody(ex.toApiError()).let {
127+
APIGatewayProxyResponseEvent()
128+
.withBody(
129+
// in case of 406 we might find no serializer so fall back to the default
130+
if (serializationHandlerChain.supports(contentType, it))
131+
serializationHandlerChain.serialize(contentType, it)
132+
else
133+
serializationHandlerChain.serialize(MediaType.parse(router.defaultContentType), it)
109134
)
110-
)
135+
.withStatusCode(ex.httpResponseStatus)
136+
.withHeaders(mapOf("Content-Type" to contentType.toString()))
111137
}
112-
return createApiExceptionErrorResponse(
113-
input, ApiException(
114-
httpResponseStatus = 404,
115-
message = "Not found",
116-
code = "NOT_FOUND"
117-
)
118-
)
119-
}
120138

121-
open fun createApiExceptionErrorResponse(input: APIGatewayProxyRequestEvent, ex: ApiException): APIGatewayProxyResponseEvent =
122-
APIGatewayProxyResponseEvent()
123-
.withBody(objectMapper.writeValueAsString(mapOf(
124-
"message" to ex.message,
125-
"code" to ex.code,
126-
"details" to ex.details
127-
)))
128-
.withStatusCode(ex.httpResponseStatus)
129-
.withHeaders(mapOf("Content-Type" to "application/json"))
130-
131-
open fun createUnexpectedErrorResponse(input: APIGatewayProxyRequestEvent, ex: Exception): APIGatewayProxyResponseEvent =
139+
open fun createUnexpectedErrorResponse(contentType: MediaType, input: APIGatewayProxyRequestEvent, ex: Exception): APIGatewayProxyResponseEvent =
132140
when (ex) {
133141
is MissingKotlinParameterException ->
134-
APIGatewayProxyResponseEvent()
135-
.withBody(objectMapper.writeValueAsString(
136-
listOf(mapOf(
137-
"path" to ex.parameter.name.orEmpty(),
138-
"message" to "Missing required field",
139-
"code" to "MISSING_REQUIRED_FIELDS"
140-
))))
141-
.withStatusCode(422)
142-
.withHeaders(mapOf("Content-Type" to "application/json"))
143-
else ->
144-
APIGatewayProxyResponseEvent()
145-
.withBody(objectMapper.writeValueAsString(mapOf(
146-
"message" to ex.message,
147-
"code" to "INTERNAL_SERVER_ERROR"
148-
)))
149-
.withStatusCode(500)
150-
.withHeaders(mapOf("Content-Type" to "application/json"))
142+
createResponse(contentType, input,
143+
ResponseEntity(422, createUnprocessableEntityErrorBody(UnprocessableEntityError(
144+
message = "Missing required field",
145+
code = "MISSING_REQUIRED_FIELDS",
146+
path = ex.parameter.name.orEmpty()))))
147+
else -> createResponse(contentType, input,
148+
ResponseEntity(500, createErrorBody(ApiError(ex.message.orEmpty(), "INTERNAL_SERVER_ERROR"))))
151149
}
152150

153151
open fun <T> createResponse(contentType: MediaType?, input: APIGatewayProxyRequestEvent, response: ResponseEntity<T>): APIGatewayProxyResponseEvent {
154-
// TODO add default accept type
155152
return when {
156153
// no-content response
157-
response.body == null && response.statusCode == 204 -> APIGatewayProxyResponseEvent()
158-
.withStatusCode(204)
154+
response.body == null -> APIGatewayProxyResponseEvent()
155+
.withStatusCode(response.statusCode)
159156
.withHeaders(response.headers)
160-
serializationHandlerChain.supports(contentType!!, response) ->
157+
serializationHandlerChain.supports(contentType!!, response.body) ->
161158
APIGatewayProxyResponseEvent()
162159
.withStatusCode(response.statusCode)
163-
.withBody(serializationHandlerChain.serialize(contentType, response))
160+
.withBody(serializationHandlerChain.serialize(contentType, response.body))
164161
.withHeaders(response.headers + ("Content-Type" to contentType.toString()))
165-
else -> throw IllegalArgumentException("unsupported response $response")
162+
else -> throw IllegalArgumentException("unsupported response ${response.body.let { (it as Any)::class.java }} and $contentType")
166163
}
167164
}
168165

0 commit comments

Comments
 (0)