eric-maynard commented on code in PR #631:
URL: https://github.com/apache/polaris/pull/631#discussion_r1909337660
##########
dropwizard/service/src/test/java/org/apache/polaris/service/dropwizard/PolarisApplicationIntegrationTest.java:
##########
@@ -724,13 +725,16 @@ public void testRequestBodyTooLarge() {
.header("Authorization", "Bearer " + userToken)
.header(REALM_PROPERTY_KEY, realm)
.post(largeRequest)) {
- assertThat(response)
- .returns(Response.Status.BAD_REQUEST.getStatusCode(),
Response::getStatus)
- .matches(
- r ->
- r.readEntity(RequestThrottlingErrorResponse.class)
- .errorType()
- .equals(REQUEST_TOO_LARGE));
+ // Note we only validate the status code here because per RFC 9110, the
server MAY not provide
+ // a response body. The HTTP status line is still expected to be
provided.
+ assertThat(response.getStatus())
+ .isEqualTo(Response.Status.REQUEST_ENTITY_TOO_LARGE.getStatusCode());
+ } catch (ProcessingException e) {
+ // Per RFC 9110 servers MAY close the connection in case of 413
responses, which
+ // might cause the client to fail to read the status code (cf. RFC 9112,
section 9.6).
+ // TODO: servers are expected to close connections gracefully. It might
be worth investigating
+ // whether "connection closed" exceptions are a client-side bug.
+ assertThat(e).hasMessageContaining("Connection was closed");
Review Comment:
This check makes sense to me. It might be worth filing an issue to see if we
can reproduce the disconnect elsewhere
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
