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

Jump to bottom

net/http: panic: runtime error: comparing uncomparable type when the underlying type has a map #29768

Closed
vadmeste opened this issue Jan 16, 2019 · 4 comments
Closed

net/http: panic: runtime error: comparing uncomparable type when the underlying type has a map #29768

vadmeste opened this issue Jan 16, 2019 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.13

Comments

@vadmeste
Copy link

vadmeste commented Jan 16, 2019 •
edited

What version of Go are you using (go version)?

go version go1.10.4 linux/amd64

Does this issue reproduce with the latest release?

Not sure, the error is too rare for me but it seems the reason of the error is easy to discern.

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/vadmeste/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/vadmeste/work/gospace"
GOPROXY=""
GORACE=""
GOROOT="/home/vadmeste/work/go"
GOTMPDIR=""
GOTOOLDIR="/home/vadmeste/work/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/mnt/p4/vadmeste/tmp/go-build653380043=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Still, the error is so rare to be reproduced but it seems the reason of the error is easy to discern.

What did you expect to see?

No panic

What did you see instead?

panic: runtime error: comparing uncomparable type minio.ErrorResponse

goroutine 352943 [running]:
net/http.(*Request).write(0xc4229baf00, 0xcadce0, 0xc42179c840, 0x0, 0x0, 0x0, 0xcaf100, 0xc4208d4280)
    /opt/go/src/net/http/request.go:624 +0x71b
net/http.(*persistConn).writeLoop(0xc421cf3200)
    /opt/go/src/net/http/transport.go:1825 +0x1ea
created by net/http.(*Transport).dialConn
    /opt/go/src/net/http/transport.go:1238 +0x97f

More information

The following is the definition of minio.ErrorResponse (https://github.com/minio/minio-go/blob/master/api-error-response.go#L38)

// ErrorResponse - Is the typed error returned by all API operations.
type ErrorResponse struct {
	XMLName    xml.Name `xml:"Error" json:"-"`
	Code       string
	Message    string
	BucketName string
	Key        string
	RequestID  string `xml:"RequestId"`
	HostID     string `xml:"HostId"`

	// Region where the bucket is located. This header is returned
	// only in HEAD bucket and ListObjects response.
	Region string

	// Underlying HTTP status code for the returned error
	StatusCode int `xml:"-" json:"-"`

	// Headers of the returned S3 XML error
	Headers http.Header `xml:"-" json:"-"`
}
@vadmeste vadmeste mentioned this issue Jan 17, 2019
Closed
@agnivade
Copy link
Contributor

Yes, this is because ErrorResponse has Headers which is actually a map. And maps are not comparable.

So, when both bodyReadError and err are internally of type ErrorResponse, the comparison crashes.

if tw.bodyReadError == err {
	err = requestBodyReadError{err}
}

https://github.com/golang/go/blob/master/src/net/http/request.go#L652

A small reproducer -

package main

import (
	"fmt"
	"net/http"
)

type ErrorResponse struct {
	Message string
	Headers http.Header `xml:"-" json:"-"`
}

// Error - Returns S3 error string.
func (e ErrorResponse) Error() string {
	return e.Message
}

func main() {
	hh := make(http.Header)
	hh.Add("hi", "kk")
	var e1 error = ErrorResponse{
		Headers: hh,
	}
	fmt.Println(e1 == e1)
}

@bradfitz - thoughts ? Since we are not in control of the underlying type, things like these can happen.
reflect.DeepEqual does bypass the panic, but do we want something like that inside net/http ?

@agnivade agnivade changed the title Comparing uncomparable type panic inside golang http net/http: panic: runtime error: comparing uncomparable type when the underlying type has a map Jan 18, 2019
@agnivade agnivade added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 18, 2019
@agnivade agnivade added this to the Go1.13 milestone Jan 18, 2019
@bradfitz
Copy link
Contributor

Yeah, sorry, maps aren't comparable. This isn't an HTTP issue, really.

@vadmeste
Copy link
Author

Someone would easily define an error struct which has a map or a slice inside it to facilitate the construction of the error message.

Since we cannot write a custom comparator in Golang for my custom error struct (ErrorResponse), it doesn't seem there is a way to avoid the panic without restructuring my code.

With this additional context description, @bradfitz do you still feel this is not a http pkg issue ?

@bradfitz
Copy link
Contributor

My opinion is unchanged. You'll have to restructure your code so you don't do things not allowed by the language.

@golang golang locked and limited conversation to collaborators Jan 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
4 participants
@bradfitz @vadmeste @agnivade @gopherbot