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

Response.IsSuccess proposal #169

Closed
mxpv opened this issue Jul 28, 2018 · 5 comments
Closed

Response.IsSuccess proposal #169

mxpv opened this issue Jul 28, 2018 · 5 comments
Assignees

Comments

@mxpv
Copy link

mxpv commented Jul 28, 2018

Hey.

I have an API which returns body in case of success or 4xx/5xx and error status text in case of error.
It would be nice to add IsSuccess helper, something like:

	if resp.IsSuccess() {
		return nil, errors.New(resp.Status())
	} else {
		return resp Body
	}

This is very similar IsSuccessStatusCode in https://stackoverflow.com/questions/32569860/checking-if-httpstatuscode-represents-success-or-failure

The implementation is trivial:

return statusCode >= 200 && statusCode <= 299;

What do you think?
Thanks.

@jeevatkm
Copy link
Member

@mxpv I understand your recommendation.

However, I would like to clarify about resty existing capabilities.

Resty has builtin support JSON and XML response handling(automatic unmarshal) for Success and Error scenario based on status code (Like you have described in the recommendation).

Let's say, if you have response struct for Success and Error.

For example:

resp, err := resty.R().
		SetHeader(hdrContentTypeKey, jsonContentType).
		SetBody(user).
		SetResult(AuthSuccess{}).
		SetError(AuthError{}).
		Post("http://localhost:8080/login")

// Success
if resp.Result() != nil {
      result := resp.Result().(*AuthSuccess)
}

// Error
if resp.Error() != nil {
     error := resp.Error().(*AuthError)
}

Does it satisfy your use case?

@mxpv
Copy link
Author

mxpv commented Jul 29, 2018

Not really.
I don't have an error body, backend sends error description as http status, something like:

HTTP/1.0 400 login or password field empty!
Date: Sat, 28 Jul 2018 17:56:19 GMT
Content-Type: application/json

So right now I have something like this:

func (c Client) checkErr(resp *resty.Response) error {
	code := resp.StatusCode()
	if code >= 200 && code <= 299 {
		return nil
	}

	return &ProvError{Code: code, Message: resp.Status()}
}

@jeevatkm jeevatkm self-assigned this Jul 29, 2018
@jeevatkm
Copy link
Member

Okay, I will add IsSuccess method into Response.

FYI SetResult and SetError can be used separately.

@jeevatkm jeevatkm added this to the v1.8 Milestone milestone Jul 29, 2018
@jeevatkm
Copy link
Member

@mxpv helper methods added to master branch. Will be released in v1.8.0.

@mxpv
Copy link
Author

mxpv commented Jul 29, 2018

@jeevatkm thanks a lot!

@mxpv mxpv closed this as completed Jul 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants