Skip to content

Commit

Permalink
[#5] Improve error handling on POST
Browse files Browse the repository at this point in the history
Also covered by tests.
  • Loading branch information
szpak committed Mar 19, 2017
1 parent d241d27 commit adbacfb
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package io.codearte.gradle.nexus.infra

import groovy.transform.CompileStatic

/**
* Custom exception to propagate server errors.
*
* Created as groovyx.net.http.HttpResponseException contains in a message only a reason phrase (e.g. Server Error) without response body
* which in many cases is crucial to determine the resons why error was returned.
*
* It may be made redundant once migrated to other HTTP library.
*/
@CompileStatic
class NexusHttpResponseException extends NexusStagingException {

final int statusCode

NexusHttpResponseException(int statusCode, String message, Throwable cause) {
super(message, cause)
this.statusCode = statusCode
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
package io.codearte.gradle.nexus.infra

import org.apache.http.client.HttpResponseException;

import groovy.transform.CompileStatic
import groovy.util.logging.Slf4j
import groovyx.net.http.ContentType
import groovyx.net.http.HttpResponseDecorator
import groovyx.net.http.HttpResponseException
import groovyx.net.http.RESTClient

/**
Expand Down Expand Up @@ -52,19 +51,17 @@ class SimplifiedHttpJsonRestClient {
setUriAndAuthentication(uri)
Map params = createAndInitializeCallParametersMap()
params.body = content
log.debug("POST request content: $content")
try {
//TODO: Add better error handling (e.g. display error message received from server, not only 500 + not fail on 404 in 'text/html')
HttpResponseDecorator response = (HttpResponseDecorator)restClient.post(params)
log.warn("POST response data: ${response.data}")
} catch(groovyx.net.http.HttpResponseException e) {
//Apache' HttpResponseException ONLY puts the 2nd param in the e.getMessage which will be printed so
//put all information there (status code, error str, body of response in case they put more error information there)

HttpResponseDecorator resp = e.getResponse();
String message = "${resp.statusLine.statusCode}:${resp.statusLine.reasonPhrase} body=${resp.data}"
log.error("POST response failed. ${message}")
throw new HttpResponseException(e.getStatusCode(), message)
}
try {
log.debug("POST request content: $content")
HttpResponseDecorator response = (HttpResponseDecorator) restClient.post(params)
log.debug("POST response status ${response.status}, data: ${response.data}")
} catch (HttpResponseException e) {
//Enhance rethrown exception to contain also response body - #5
//TODO: Still better handle response content type on 404 and 50x - server returns 'text/plain', but RESTClient from Groovy Builder tries to parse it as JSON
HttpResponseDecorator resp = e.getResponse();
String message = "${resp.statusLine.statusCode}: ${resp.statusLine.reasonPhrase}, body: ${resp.data}"
log.warn("POST response failed. ${message}")
throw new NexusHttpResponseException(e.getStatusCode(), message, e)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package io.codearte.gradle.nexus.functional

import com.github.tomakehurst.wiremock.junit.WireMockRule
import groovy.transform.NotYetImplemented
import groovyx.net.http.HttpResponseException
import groovyx.net.http.RESTClient
import io.codearte.gradle.nexus.infra.NexusHttpResponseException
import io.codearte.gradle.nexus.infra.SimplifiedHttpJsonRestClient
import io.codearte.gradle.nexus.logic.RepositoryCloser
import org.junit.Rule
import spock.lang.Specification

import static com.github.tomakehurst.wiremock.client.WireMock.aResponse
import static com.github.tomakehurst.wiremock.client.WireMock.containing
import static com.github.tomakehurst.wiremock.client.WireMock.equalTo
import static com.github.tomakehurst.wiremock.client.WireMock.post
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo

class MockedResponseErrorHandlingSpec extends Specification {

private static final String TEST_MOCKED_USERNAME = ''
private static final String TEST_MOCKED_PASSWORD = ''
private static final String TEST_MOCKED_STAGING_PROFILE_ID = "93c08fdebde1ff"
private static final String TEST_MOCKED_NOT_EXISTING_REPOSITORY_ID = "xxx"
private static final String TEST_MOCKED_SERVER_ERROR_JSON_RESPONSE = """
{
"errors": [
{
"id": "*",
"msg": "Unhandled: Missing staging repository: $TEST_MOCKED_NOT_EXISTING_REPOSITORY_ID"
}
]
}
""".stripIndent()

@Rule
public WireMockRule wireMockRule = new WireMockRule(8089)

//Using private Options as server is not started yet
private String mockedUrl = "http://localhost:${wireMockRule.options.portNumber()}/"

def "should present response body on 500 server error"() {
given:
SimplifiedHttpJsonRestClient client = new SimplifiedHttpJsonRestClient(new RESTClient(), TEST_MOCKED_USERNAME, TEST_MOCKED_PASSWORD)
RepositoryCloser closer = new RepositoryCloser(client, mockedUrl)
and:
stubFor(post(urlEqualTo("/staging/profiles/$TEST_MOCKED_STAGING_PROFILE_ID/finish"))
.withHeader("Content-Type", equalTo("application/json"))
.withHeader("Accept", containing("application/json"))
.willReturn(aResponse()
.withStatus(500)
.withBody(TEST_MOCKED_SERVER_ERROR_JSON_RESPONSE)
.withHeader("Content-Type", "application/json")))
when:
closer.closeRepositoryWithIdAndStagingProfileId(TEST_MOCKED_NOT_EXISTING_REPOSITORY_ID, TEST_MOCKED_STAGING_PROFILE_ID)
then:
NexusHttpResponseException e = thrown()
e.statusCode == 500
e.message.contains("Missing staging repository: $TEST_MOCKED_NOT_EXISTING_REPOSITORY_ID")
e.cause instanceof HttpResponseException
}

@NotYetImplemented
def "should present response body on 400 or 500 errors with plain text response"() {}
}

0 comments on commit adbacfb

Please sign in to comment.