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

fix: Send response with 404 status when quering non-exist account #14

Merged
merged 1 commit into from
Jul 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions types/rest/http_status_table.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package rest

//TODO : Intergrate http status mapping for every REST API
import (
"net/http"

"github.com/cosmos/cosmos-sdk/client/context"
"github.com/cosmos/cosmos-sdk/types/errors"
)

//HTTPStatusMappingTable is map to mapping an error type and a http status
type HTTPStatusMappingTable map[string]map[uint32]int

var (
table = HTTPStatusMappingTable{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
However, For better structure, you'd better consider below when you work on next PR

  1. Do not use a constant value like 9 here. It is an error code and should be provided as a variable or named constant by the error definition.
  2. Provide register function which maps given error to HTTP status. If then, in each module when the error is registered the HTTP status code can be passed also.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
However, For better structure, you'd better consider below when you work on next PR

  1. Do not use a constant value like 9 here. It is an error code and should be provided as a variable or named constant by the error definition.
  2. Provide register function which maps given error to HTTP status. If then, in each module when the error is registered the HTTP status code can be passed also.

👍

Can we do 1 like the below?

		errors.RootCodespace: {
			errors.ErrUnknownAddress.ABCICode(): http.StatusNotFound,
		},

errors.RootCodespace: {
9: http.StatusNotFound,
},
}
)

func parsingError(rawErr error) *errors.Error {
if rawErr == nil {
return nil
}
if err, ok := rawErr.(*context.Error); ok {
return errors.New(err.Codespace, err.Code, err.Message)
}
if err, ok := rawErr.(*errors.Error); ok {
return err
}
return errors.New(errors.UndefinedCodespace, 1, "internal")
}

//GetHTTPStatus is method to get http status for given error
func GetHTTPStatusWithError(err error) int {
abciErr := parsingError(err)
if abciErr == nil {
return http.StatusOK
}
result := http.StatusInternalServerError
if codeTable, ok := table[abciErr.Codespace()]; ok {
if status, ok := codeTable[abciErr.ABCICode()]; ok {
result = status
}
}
return result
}
11 changes: 2 additions & 9 deletions x/auth/client/rest/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,8 @@ func QueryAccountRequestHandlerFn(storeName string, cliCtx context.CLIContext) h

account, height, err := accGetter.GetAccountWithHeight(addr)
if err != nil {
// TODO: Handle more appropriately based on the error type.
// Ref: https://github.com/cosmos/cosmos-sdk/issues/4923
if err := accGetter.EnsureExists(addr); err != nil {
cliCtx = cliCtx.WithHeight(height)
rest.PostProcessResponse(w, cliCtx, types.BaseAccount{})
return
}

rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
httpStatus := rest.GetHTTPStatusWithError(err)
rest.WriteErrorResponse(w, httpStatus, err.Error())
return
}

Expand Down