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

If rendering has failed due to a net.OpError stop rendering (attempt 2) #19049

Merged

Conversation

zeripath
Copy link
Contributor

Unfortunately #18642 does not work because a *net.OpError does not implement
the Is interface to make errors.Is work correctly - thus leading to the
irritating conclusion that a *net.OpError is not a *net.OpError.

Here we keep the errors.Is because presumably this will be fixed at
some point in the golang main source code but also we add a simply type
cast to also check.

Fix #18629

Signed-off-by: Andrew Thornton art27@cantab.net

Unfortunately go-gitea#18642 does not work because a `*net.OpError` does not implement
the `Is` interface to make `errors.Is` work correctly - thus leading to the
irritating conclusion that a `*net.OpError` is not a `*net.OpError`.

Here we keep the `errors.Is` because presumably this will be fixed at
some point in the golang main source code but also we add a simply type
cast to also check.

Fix go-gitea#18629

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
Copy link
Contributor Author

Here's a simple test case:

package main

import (
	"errors"
	"fmt"
	"net"
)

func main() {
	opError := &net.OpError{Op: "write", Net: "", Err: fmt.Errorf("random")}
	fmt.Println(opError)

	fmt.Println(errors.Is(opError, &net.OpError{}))
}

a *net.OpError does not pass errors.Is(err, &net.OpError) and thus it could be said that a *net.OpError is not a *net.OpError.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 10, 2022
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 10, 2022
@@ -269,7 +269,7 @@ func (ctx *Context) ServerError(logMsg string, logErr error) {
func (ctx *Context) serverErrorInternal(logMsg string, logErr error) {
if logErr != nil {
log.ErrorWithSkip(2, "%s: %v", logMsg, logErr)
if errors.Is(logErr, &net.OpError{}) {
if _, ok := logErr.(*net.OpError); ok || errors.Is(logErr, &net.OpError{}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to add a test case for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I could have easily reproduced the problem I would have resolved this issue the first time and known that the original solution didn't work.

@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 Mar 10, 2022
@zeripath
Copy link
Contributor Author

make lgtm work

@zeripath zeripath merged commit a0db075 into go-gitea:main Mar 10, 2022
@zeripath zeripath deleted the refix-18629-net.OpError-is-not-a-net.OpError branch March 10, 2022 20:23
zeripath added a commit to zeripath/gitea that referenced this pull request Mar 10, 2022
…2) (go-gitea#19049)

Backport go-gitea#19049

Unfortunately go-gitea#18642 does not work because a `*net.OpError` does not implement
the `Is` interface to make `errors.Is` work correctly - thus leading to the
irritating conclusion that a `*net.OpError` is not a `*net.OpError`.

Here we keep the `errors.Is` because presumably this will be fixed at
some point in the golang main source code but also we add a simply type
cast to also check.

Fix go-gitea#18629

Signed-off-by: Andrew Thornton <art27@cantab.net>
6543 pushed a commit that referenced this pull request Mar 10, 2022
…2) (#19049) (#19056)

Backport #19049

Unfortunately #18642 does not work because a `*net.OpError` does not implement
the `Is` interface to make `errors.Is` work correctly - thus leading to the
irritating conclusion that a `*net.OpError` is not a `*net.OpError`.

Here we keep the `errors.Is` because presumably this will be fixed at
some point in the golang main source code but also we add a simply type
cast to also check.

Fix #18629

Signed-off-by: Andrew Thornton <art27@cantab.net>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 11, 2022
* giteaofficial/main:
  Prevent 500 when there is an error during new auth source post (go-gitea#19041)
  Update the webauthn_credential_id_sequence in Postgres (go-gitea#19048)
  If rendering has failed due to a net.OpError stop rendering (attempt 2) (go-gitea#19049)
  use xorm builder for models.getReviewers() (go-gitea#19033)
  RSS/Atom support for Orgs (go-gitea#17714)
  Fix flag validation (go-gitea#19046)
  Improve SyncMirrors logging (go-gitea#19045)
@zeripath zeripath added the backport/done All backports for this PR have been created label Mar 19, 2022
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
…2) (go-gitea#19049)

Unfortunately go-gitea#18642 does not work because a `*net.OpError` does not implement
the `Is` interface to make `errors.Is` work correctly - thus leading to the
irritating conclusion that a `*net.OpError` is not a `*net.OpError`.

Here we keep the `errors.Is` because presumably this will be fixed at
some point in the golang main source code but also we add a simply type
cast to also check.

Fix go-gitea#18629

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created 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.

Error: invalid memory address or nil pointer dereference
4 participants