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(tree): Keep panic infos consistent when wildcard type build faild #4077

Merged
merged 1 commit into from
Oct 26, 2024

Conversation

kingcanfish
Copy link
Contributor

@kingcanfish kingcanfish commented Oct 17, 2024

fix(tree): Keep panic infos consistent when wildcard type build faild

When I run the demo1

// demo1
func main() {
	router := gin.Default()
	router.GET("/abc/bar", nil)
	router.GET("/abc/x*z", nil)

	router.Run("localhost:8080")
}

it got the panic:panic: no / before catch-all in path '/abc/x*z'

but when i run the demo2

// demo2
func main() {
	router := gin.Default()
	router.GET("/abc/bar", nil)
	router.GET("/abc/b*r", nil)

	router.Run("localhost:8080")
} 

it got the panic: panic: runtime error: index out of range [-1]
Although the end result is the same(panic), the different panic informations may cause some confusion.

So, add the i < 0 condition to keep the panic infos same

@kingcanfish
Copy link
Contributor Author

#4066 This PR fixed the ci-lint faild code

@appleboy
Copy link
Member

build failed

@appleboy
Copy link
Member

waiting for #4081

@appleboy appleboy added this to the v1.11 milestone Oct 25, 2024
@appleboy
Copy link
Member

please update the branch to the latest version.

@kingcanfish
Copy link
Contributor Author

please update the branch to the latest version.

thank you for your review. I have updated to the latest version, but the golangci-lint still fails.

in my machine, golangci-lint (v1.60.2) is success, the same version as on github(v1.58.1) failed to run,
so may be we need update the lint to the latest version?

Copy link

codecov bot commented Oct 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.96%. Comparing base (3dc1cd6) to head (26f5476).
Report is 78 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4077      +/-   ##
==========================================
- Coverage   99.21%   98.96%   -0.25%     
==========================================
  Files          42       44       +2     
  Lines        3182     3478     +296     
==========================================
+ Hits         3157     3442     +285     
- Misses         17       25       +8     
- Partials        8       11       +3     
Flag Coverage Δ
?
-tags "sonic avx" 98.95% <100.00%> (?)
-tags go_json 98.95% <100.00%> (?)
-tags nomsgpack 98.95% <100.00%> (?)
go-1.18 ?
go-1.19 ?
go-1.20 ?
go-1.21 98.96% <100.00%> (-0.25%) ⬇️
go-1.22 98.96% <100.00%> (?)
macos-latest 98.96% <100.00%> (-0.25%) ⬇️
ubuntu-latest 98.96% <100.00%> (-0.25%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kingcanfish
Copy link
Contributor Author

please update the branch to the latest version.

thank you for your review. I have updated to the latest version, but the golangci-lint still fails.

in my machine, golangci-lint (v1.60.2) is success, the same version as on github(v1.58.1) failed to run, so may be we need update the lint to the latest version?

update the golangci-lint to v1.62.0 :)

@appleboy appleboy added the bug label Oct 26, 2024
@appleboy appleboy merged commit ea53388 into gin-gonic:master Oct 26, 2024
24 of 25 checks passed
@appleboy
Copy link
Member

@kingcanfish Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants