Skip to content
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

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
# for free to subscribe to this conversation on GitHub. Already have an account? #.
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