Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve response data on error. RESTClient still has response data for HTTP >= 400 status #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

michaelcameron
Copy link
Contributor

There's still data in HTTP responses >= 400. RESTClient will preserve that data and then throw the exception. I know this is duplicate code for the try, so ripe for refactoring. I tried playing around with failure handlers. I want to do what RESTClient.defaultFailureHandler() without throwing the exception, but it relies on HTTPBuilder.defaultSuccessHandler() which is protected and non-trivial. The only clean way I can think of would be to subclass RESTClient and override defaultFailureHandler, then having access to HTTPBuilder.defaultSuccessHandler(). I know you have the TODO in there to change/remove the failure handler. If you want to go down that route. I could make the changes and submit a pull request.

@marcpalmer
Copy link
Member

Is there a JIRA for this issue?

@michaelcameron
Copy link
Contributor Author

I don't believe so, no

@darxriggs
Copy link

You could also make the RESTClient always use the defaultSuccessHandler in case it's not required to have the ContentChangedEvent have a different evensSource when status >= 400.

client.defaultResponseHandlers[Status.FAILURE] = client.defaultResponseHandlers[Status.SUCCESS]

christoph-frick pushed a commit to christoph-frick/grails-functional-test that referenced this pull request Mar 24, 2015
small improvements in the create-functional-test script
@lucasperalta
Copy link

Please make this happen!
We did a workaorund, in the test class add this method, this way we can have the response data when http status is >=400

private void assertResponseContains(containText) {
String responseString = this.getCurrentClient().response.data.text
assertTrue responseString.contains(containText)
}
not the best but , is working ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants