Skip to content

panics in timeout middleware are not recovered and cause application to crash #1795

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged

Conversation

aldas
Copy link
Contributor

@aldas aldas commented Mar 1, 2021

Fix #1794: panics in timeout middleware are not recovered and cause application to crash (as those happen in different goroutine outside of recover middleware)

@codecov
Copy link

codecov bot commented Mar 1, 2021

Codecov Report

Merging #1795 (c239a87) into master (c79ffed) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1795      +/-   ##
==========================================
+ Coverage   89.73%   89.75%   +0.02%     
==========================================
  Files          32       32              
  Lines        2669     2675       +6     
==========================================
+ Hits         2395     2401       +6     
  Misses        175      175              
  Partials       99       99              
Impacted Files Coverage Δ
middleware/timeout.go 100.00% <100.00%> (ø)

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 c79ffed...c239a87. Read the comment docs.

@aldas
Copy link
Contributor Author

aldas commented Mar 1, 2021

This can be reproduced with following app setup

func main() {
	e := echo.New()

	e.Use(middleware.TimeoutWithConfig(middleware.TimeoutConfig{
		Timeout: 10 * time.Millisecond,
	}))

	e.GET("/", func(c echo.Context) error {
		panic("panic!!!")
	})

	e.Start(":8080")
}
curl -v http://localhost:8080/

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.

Looks good, now the error of the spun-off go routine is even logged.

This is real Go wizardry ;-) Great @aldas !

@aldas aldas merged commit b2444d8 into labstack:master Mar 2, 2021
@aldas aldas deleted the fix_issue_1794_recover_timeoutmiddleware branch May 23, 2021 06:07
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic in Timeout middleware is causing application to exit
2 participants