Fail when there's an unexpected response body #3095
Conversation
| case Some(bodyFromHeaders) => ServerResponse(statusCode, headers, Some(bodyFromHeaders(Headers(headers))), Some(output)).unit | ||
| case None => ServerResponse(statusCode, headers, None: Option[B], Some(output)).unit | ||
| (statusCode, outputValues.body) match { | ||
| case (StatusCode.NoContent, Some(_)) => monad.error(new IllegalStateException("Unexpected response body when status code == NoContent (204)")) |
There was a problem hiding this comment.
maybe there are more status codes which prohibit a body? https://stackoverflow.com/questions/8628725/comprehensive-list-of-http-status-codes-that-dont-include-a-response-body has some candidates, but it would be good to verify in the spec
There was a problem hiding this comment.
Looks like 304 and 1xx are bodyless by specification as well. I don't think we need to account for 1xx, but for 304 it might be worth it. As far as I understand, it's a status typically provided by caching mechanisms and probably shouldn't be set manually by Tapir users, but let's not underestimate their creativity :)
|
We could also extend |
kciesielski
changed the title
| case Some(bodyFromHeaders) => ServerResponse(statusCode, headers, Some(bodyFromHeaders(Headers(headers))), Some(output)).unit | ||
| case None => ServerResponse(statusCode, headers, None: Option[B], Some(output)).unit | ||
| (statusCode, outputValues.body) match { | ||
| case (StatusCode.NoContent | StatusCode.NotModified, Some(_)) => monad.error(new IllegalStateException(s"Unexpected response body when status code == $statusCode")) |
There was a problem hiding this comment.
Maybe it would be better to say: The status code XYZ doesn't allow a body
Also ... what if the response body is defined as an Option? Or if the body is returned as an empty string "" - shouldn't we special case on this?
There was a problem hiding this comment.
That would be hard to check in this general function. The body is of type B which is backend-dependent (like akka.http.scaladsl.model.HttpEntity).
| val e2 = endpoint.in("endpoint2_Ok").out(stringBody).out(statusCode(StatusCode.BadRequest)) | ||
| val e3 = endpoint.in("endpoint3_Err").out(stringBody).out(statusCode(StatusCode.NotModified)) | ||
| val e4 = endpoint.in("endpoint4_ok").out(emptyOutputAs(NoContent)).out(statusCode(StatusCode.NoContent)) | ||
| val e5 = endpoint |
There was a problem hiding this comment.
Would the checker pass for an endpoint such as:
val e6 = endpoint
.in("endpoint6_ok")
.errorOut(
sttp.tapir.oneOf[ErrorInfo](
oneOfVariant(statusCode(StatusCode.NotFound).and(jsonBody[NotFound])),
oneOfVariant(statusCode(StatusCode.NoContent).and(emptyOutputAs[NoContent]))
)
)There was a problem hiding this comment.
Good idea to check this, I have added a test.
|
Thanks :) |
Initiated by: https://softwaremill.community/t/is-it-possible-to-raise-an-error-when-content-negotiation-is-invalid/261