Skip to content

Conversation

@daheige
Copy link
Contributor

@daheige daheige commented Jul 11, 2021

1.export ErrUnknownType unknown request type
2.code adjust for broken pipe and json invalid and use http.MethodGet instead of raw GET string
3.IsWebsocket check

@codecov
Copy link

codecov bot commented Jul 11, 2021

Codecov Report

Merging #2785 (5060a60) into master (696d37e) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

❗ Current head 5060a60 differs from pull request most recent head 21fe9c2. Consider uploading reports for the commit 21fe9c2 to get more accurate results

@@            Coverage Diff             @@
##           master    #2785      +/-   ##
==========================================
- Coverage   98.77%   98.71%   -0.07%     
==========================================
  Files          41       41              
  Lines        3106     2094    -1012     
==========================================
- Hits         3068     2067    -1001     
+ Misses         26       15      -11     
  Partials       12       12              
Flag Coverage Δ
go-1.14 ?
go-1.15 ?
go-1.16 ?
go-1.17 ?
go-1.18 ?
macos-latest ?
nomsgpack ?
ubuntu-latest ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
binding/binding_nomsgpack.go 100.00% <100.00%> (ø)
binding/form_mapping.go 100.00% <100.00%> (ø)
binding/json.go 100.00% <100.00%> (ø)
context.go 97.68% <100.00%> (-0.24%) ⬇️
recovery.go 97.22% <100.00%> (-0.86%) ⬇️
utils.go 96.82% <0.00%> (-0.18%) ⬇️
response_writer.go 93.33% <0.00%> (-0.15%) ⬇️
gin.go 99.18% <0.00%> (-0.01%) ⬇️
... and 36 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 696d37e...21fe9c2. Read the comment docs.

@daheige
Copy link
Contributor Author

daheige commented Jul 12, 2021

@appleboy @thinkerou

binding/json.go Outdated
EnableDecoderDisallowUnknownFields = false

// JsonReqInvalid json invalid when request is nil or request body is nil.
JsonReqInvalid = errors.New("invalid request")
Copy link
Member

Choose a reason for hiding this comment

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

Maybe InvaildRequest?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I've adjusted it.

@appleboy appleboy added this to the v1.8 milestone Jul 13, 2021
@daheige
Copy link
Contributor Author

daheige commented Jul 13, 2021 via email

}

// IsBroken Check error for a broken connection
func IsBroken(err interface{}) bool {
Copy link
Member

Choose a reason for hiding this comment

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

public method?

@daheige
Copy link
Contributor Author

daheige commented Jul 22, 2021 via email

binding/json.go Outdated
func (jsonBinding) Bind(req *http.Request, obj interface{}) error {
if req == nil || req.Body == nil {
return errors.New("invalid request")
return InvalidRequest
Copy link
Contributor

Choose a reason for hiding this comment

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

How about:

Suggested change
return InvalidRequest
return ErrInvalidRequest

I think it's a common way to name errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok,thinks @kszafran

@appleboy
Copy link
Member

@daheige Please help to resolve conflicts.

@daheige
Copy link
Contributor Author

daheige commented Apr 18, 2022 via email

@thinkerou thinkerou modified the milestones: v1.8, v1.9 May 28, 2022
@thinkerou thinkerou modified the milestones: v1.9, v1.10 Jan 17, 2023
@appleboy appleboy modified the milestones: v1.10, v1.11 Mar 21, 2024
@appleboy appleboy modified the milestones: v1.11, v1.12 Jun 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants