-
Notifications
You must be signed in to change notification settings - Fork 31
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
Conversation
type HTTPStatusMappingTable map[string]map[uint32]int | ||
|
||
var ( | ||
table = HTTPStatusMappingTable{ |
There was a problem hiding this comment.
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
- 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.
- 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.
There was a problem hiding this comment.
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
- 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.
- 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,
},
If line/link ran CI with the version tag 0.0.1 already, it should be bumped up 0.0.2 |
The CI has already run with tag 0.0.1. we may bump up the version. |
* feat: add codespace to broadcast response (#6) * chore: expose usedCodes for document (#7) * fix: query error (#9) * fix: check internalABCICodespace (#10) * fix: check UndefinedCodespace too * Update types/errors/abci.go Co-authored-by: Yongwoo Lee <whylee@linecorp.com> Co-authored-by: Yongwoo Lee <whylee@linecorp.com> * fix: Send response with 404 status when quering non-exist account (#14) * chore: enable github action ci * chore: remove github access token from Dockerfile Co-authored-by: Yongwoo Lee <whylee@linecorp.com> Co-authored-by: Marshall Kim <kfangw@gmail.com> Co-authored-by: wonkuk_seo <bhb07@naver.com>
Description
Mapping error and http status to send a reponse with proper http status
in this PR, map a one error. but, to be mapped all of errors
Motivation and context
https://github.com/line/link/issues/286
How has this been tested?
TBD
Screenshots (if appropriate):
Checklist: