-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Stop logging EOFs and exit(1)s in ssh handler (#20476) #20529
Conversation
Backport go-gitea#20476 The code in modules/ssh/ssh.go:sessionHandler() currently cause an error to be logged if `gitea serv` exits with a exit(1). This logging is useless because the accompanying stderr is not provided and in any case the exit(1) is most likely due to permissions errors. Further it then causes the EOF to be logged - even though this is not helpful. This PR simply checks the errors returned and stops logging them. In the case of misconfigurations causing `gitea serv` to fail with exit(1) the current logging is not helpful at determining this and users should simply review the message passed over the ssh connection. Fix go-gitea#20473 Signed-off-by: Andrew Thornton <art27@cantab.net>
} | ||
|
||
if err := session.Exit(getExitStatusFromError(err)); err != nil { | ||
if err := session.Exit(getExitStatusFromError(err)); err != nil && !errors.Is(err, io.EOF) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is using error.Is correct here? golang/go#39155 suggests that io.EOF never should be wrapped(and currently standard library shouldn't do that)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just because io.EOF shouldn't be wrapped does not mean that it cannot be wrapped and errors.Is will still work on it.
We don't care about io.EOF. we don't care about errors that accidentally wrap io.EOF.
Therefore we should ignore them.
Therefore errors.Is is correct
🚀 |
Backport #20476
The code in modules/ssh/ssh.go:sessionHandler() currently cause an error to be
logged if
gitea serv
exits with a exit(1). This logging is useless because theaccompanying stderr is not provided and in any case the exit(1) is most likely due
to permissions errors.
Further it then causes the EOF to be logged - even though this is not helpful.
This PR simply checks the errors returned and stops logging them.
In the case of misconfigurations causing
gitea serv
to fail with exit(1)the current logging is not helpful at determining this and users should simply
review the message passed over the ssh connection.
Fix #20473
Signed-off-by: Andrew Thornton art27@cantab.net