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

Add more http error values #2277

Merged
merged 3 commits into from
Feb 21, 2023
Merged

Conversation

siyul-park
Copy link
Contributor

@siyul-park siyul-park commented Sep 19, 2022

  • Add more http error values.

@siyul-park siyul-park changed the title Add more http error values. Add more http error values Sep 19, 2022
@aldas
Copy link
Contributor

aldas commented Sep 19, 2022

You can not rename existing public variables - this is not backwards compatible and would break others repos compiling

@siyul-park
Copy link
Contributor Author

siyul-park commented Sep 19, 2022

Ok, I rollback the "ErrStatusRequestEntityTooLarge"

@lammel
Copy link
Contributor

lammel commented Nov 30, 2022

I personally like the addition of the additional error codes. Adding the HTTP Status Code as a comment might help to visualize the sort order.

@siyual-park Could you add comments for the HTTP status codes too (for all errors) to make it obvious where to fit in further codes:

var (
	ErrUnsupportedMediaType        = NewHTTPError(http.StatusUnsupportedMediaType) // HTTP 415 Unsupported Media Type
	ErrNotFound                    = NewHTTPError(http.StatusNotFound)             // HTTP 404 Not Found
	ErrUnauthorized                = NewHTTPError(http.StatusUnauthorized)         // HTTP 401 Unauthorized
	ErrForbidden                   = NewHTTPError(http.StatusForbidden)            // HTTP 403 Forbidden```

@siyul-park
Copy link
Contributor Author

Thanks for the good suggestions. i add comments for the HTTP status codes.

@codecov
Copy link

codecov bot commented Feb 21, 2023

Codecov Report

Base: 92.35% // Head: 92.80% // Increases project coverage by +0.45% 🎉

Coverage data is based on head (128d6fc) compared to base (666938e).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2277      +/-   ##
==========================================
+ Coverage   92.35%   92.80%   +0.45%     
==========================================
  Files          37       38       +1     
  Lines        4436     4506      +70     
==========================================
+ Hits         4097     4182      +85     
+ Misses        247      236      -11     
+ Partials       92       88       -4     
Impacted Files Coverage Δ
echo.go 95.47% <ø> (+0.29%) ⬆️
middleware/logger.go 85.10% <0.00%> (-1.47%) ⬇️
ip.go 100.00% <0.00%> (ø)
middleware/jwt.go 92.98% <0.00%> (ø)
middleware/compress.go 81.15% <0.00%> (ø)
middleware/body_dump.go 90.47% <0.00%> (ø)
middleware/rate_limiter.go 100.00% <0.00%> (ø)
middleware/context_timeout.go 100.00% <0.00%> (ø)
middleware/extractor.go 99.13% <0.00%> (+0.01%) ⬆️
middleware/request_logger.go 97.56% <0.00%> (+0.08%) ⬆️
... and 6 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@lammel lammel left a comment

Choose a reason for hiding this comment

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

LGTM!

The Go 1.20 CI errors are being worked on and unrelated.

@lammel lammel merged commit 04ba8e2 into labstack:master Feb 21, 2023
@lammel
Copy link
Contributor

lammel commented Feb 21, 2023

@siyual-park Thanks for your contribution!

@siyul-park siyul-park deleted the feature/more_error_type branch February 23, 2023 06:39
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.

None yet

3 participants