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

cmd/serv: actually exit after fatal errors #7460

Merged
merged 1 commit into from
Jul 14, 2019

Conversation

aswild
Copy link
Contributor

@aswild aswild commented Jul 14, 2019

Regression in 356854f, where
a log.Fatal call was removed. log.Fatal calls os.Exit(1) as intended,
but without it the fail() function returns normally and execution
continues past the fatal error, causing a panic.

This is visible as a go panic log and stack trace returned to the SSH
client, which is not only ugly, it leaks server and build system
information.

Fix by removing the stray return statement so that the fail() function
always calls os.Exit(1).

Backport: 2186a99
Fixes: #7457
Signed-off-by: Allen Wild allenwild93@gmail.com

Please check the following:

Regression in 356854f, where
a log.Fatal call was removed. log.Fatal calls os.Exit(1) as intended,
but without it the fail() function returns normally and execution
continues past the fatal error, causing a panic.

This is visible as a go panic log and stack trace returned to the SSH
client, which is not only ugly, it leaks server and build system
information.

Fix by removing the stray return statement so that the fail() function
always calls os.Exit(1).

Backport: 2186a99
Fixes: go-gitea#7457
Signed-off-by: Allen Wild <allenwild93@gmail.com>
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jul 14, 2019
@techknowlogick
Copy link
Member

@aswild thanks for PR and backport :)

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 14, 2019
@lunny lunny merged commit ee1a8d7 into go-gitea:release/v1.9 Jul 14, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants