AUTH /api/clusters returns response with panic error

[nurali-techie]

Call to https://auth.prod-preview.openshift.io/api/clusters

Returns response with below invalid json format:

{
  "data": []
}
{
  "errors": []
}

Where errors contains panic error stack trace as below:

panic: net/http: abort Handler
panic(0xbc30c0, 0xc000030ab0)
	/usr/lib/golang/src/runtime/panic.go:513 +0x1b9
net/http/httputil.(*ReverseProxy).ServeHTTP(0xc0005c9a90, 0xdff980, 0xc0008723c0, 0xc0001c3400)
	/usr/lib/golang/src/net/http/httputil/reverseproxy.go:284 +0x1058
github.com/fabric8-services/fabric8-auth/vendor/github.com/fabric8-services/fabric8-common/httpsupport.route(0xe00ec0, 0xc0003af0a0, 0xcce642, 0x10, 0x0, 0x0, 0x0, 0x0, 0xcce642, 0x10)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/vendor/github.com/fabric8-services/fabric8-common/httpsupport/proxy.go:85 +0x2d0
github.com/fabric8-services/fabric8-auth/vendor/github.com/fabric8-services/fabric8-common/httpsupport.RouteHTTP(0xe00ec0, 0xc0003af0a0, 0xcce642, 0x10, 0x0, 0x0, 0x0, 0xc0003af0a0, 0xc9a240)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/vendor/github.com/fabric8-services/fabric8-common/httpsupport/proxy.go:43 +0x7d
github.com/fabric8-services/fabric8-auth/controller.(*ClustersController).Show(0xc0003d0680, 0xc0003af0a0, 0xc0001c3400, 0xc0000863c0)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/controller/clusters.go:33 +0x8e
github.com/fabric8-services/fabric8-auth/app.MountClustersController.func1(0xe00300, 0xc0008f2480, 0xdfdf40, 0xc00037db80, 0xc0001c3400, 0x0, 0x0)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/app/controllers.go:132 +0xdd
github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa/middleware/security/jwt.New.func1.1(0xe00300, 0xc0008f2240, 0xdfdf40, 0xc00037db80, 0xc0001c3400, 0xe00300, 0xc0008f2240)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa/middleware/security/jwt/jwt.go:123 +0x4c3
github.com/fabric8-services/fabric8-auth/app.handleSecurity.func1(0xe00300, 0xc0008f2240, 0xdfdf40, 0xc00037db80, 0xc0001c3400, 0xc222a0, 0xc00037db80)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/app/security.go:49 +0x177
github.com/fabric8-services/fabric8-auth/app.handleClustersOrigin.func1(0xe00300, 0xc0008f2090, 0xdfdf40, 0xc00037db80, 0xc0001c3400, 0x0, 0x0)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/app/controllers.go:148 +0x4e4
github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa.(*Controller).MuxHandler.func1.1(0xe00300, 0xc0008f2090, 0xdfdf40, 0xc00037db80, 0xc0001c3400, 0x0, 0x0)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa/service.go:270 +0x9b
github.com/fabric8-services/fabric8-auth/log.LogRequest.func1.1(0xe00300, 0xc0008f2090, 0xdfdf40, 0xc00037db80, 0xc0001c3400, 0xc0008f2000, 0xe00300)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/log/log_request.go:90 +0xd0a
github.com/fabric8-services/fabric8-auth/authorization/token/manager.InjectTokenManager.func1.1(0xe00300, 0xc000939fb0, 0xdfdf40, 0xc00037db80, 0xc0001c3400, 0xc0005c9680, 0x0)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/authorization/token/manager/token_manager.go:170 +0x98
github.com/fabric8-services/fabric8-auth/goamiddleware.handler.func1(0xe00300, 0xc000939d40, 0xdfdf40, 0xc00037db80, 0xc0001c3400, 0xd9, 0x0)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/goamiddleware/jwt_token_context.go:49 +0x272
github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa/middleware.Recover.func1.1(0xe00300, 0xc000939d40, 0xdfdf40, 0xc00037db80, 0xc0001c3400, 0x0, 0x0)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa/middleware/recover.go:37 +0x93
github.com/fabric8-services/fabric8-auth/jsonapi.ErrorHandler.func1.1(0xe00300, 0xc000939d40, 0xdfdf40, 0xc00037db80, 0xc0001c3400, 0x1, 0xc00048fb40)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/jsonapi/error_handler.go:37 +0xb4
github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa/middleware/gzip.Middleware.func2.1(0xe00300, 0xc000939d40, 0xdfdf40, 0xc00037db80, 0xc0001c3400, 0x0, 0x0)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa/middleware/gzip/middleware.go:98 +0x3a3
github.com/fabric8-services/fabric8-auth/log.LogRequest.func1.1(0xe00300, 0xc000939d40, 0xdfdf40, 0xc00037db80, 0xc0001c3400, 0xc00046f830, 0xe00300)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/log/log_request.go:90 +0xd0a
github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa/middleware.RequestIDWithHeaderAndLengthLimit.func1.1(0xe00300, 0xc000939c80, 0xdfdf40, 0xc00037db80, 0xc0001c3400, 0xc000939bf0, 0xe00300)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa/middleware/request_id.go:63 +0x144
github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa.(*Controller).MuxHandler.func1(0xdff880, 0xc0005e37a0, 0xc0001c3400, 0xc000939bf0)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa/service.go:303 +0x31e
github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa.(*mux).Handle.func1(0xdff880, 0xc0005e37a0, 0xc0001c3400, 0x0)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa/mux.go:59 +0x1c4
github.com/fabric8-services/fabric8-auth/vendor/github.com/dimfeld/httptreemux.(*TreeMux).ServeHTTP(0xc0003b4640, 0xdff880, 0xc0005e37a0, 0xc0001c3400)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/vendor/github.com/dimfeld/httptreemux/router.go:200 +0xe5
github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa.(*mux).ServeHTTP(0xc0003ea260, 0xdff880, 0xc0005e37a0, 0xc0001c3400)
	/tmp/go/src/github.com/fabric8-services/fabric8-auth/vendor/github.com/goadesign/goa/mux.go:85 +0x4c
net/http.(*ServeMux).ServeHTTP(0x1401ee0, 0xdff880, 0xc0005e37a0, 0xc0001c3400)
	/usr/lib/golang/src/net/http/server.go:2361 +0x127
net/http.serverHandler.ServeHTTP(0xc00009d5f0, 0xdff880, 0xc0005e37a0, 0xc0001c3400)
	/usr/lib/golang/src/net/http/server.go:2741 +0xab
net/http.(*conn).serve(0xc000872000, 0xe00240, 0xc00037dac0)
	/usr/lib/golang/src/net/http/server.go:1847 +0x646
created by net/http.(*Server).Serve
	/usr/lib/golang/src/net/http/server.go:2851 +0x2f5

[MatousJobanek]

This panic seems to be a result of using our gunzipResponseWriter as a writer that extracts the body and writes tot the response in a combination of the ReverseProxy. The reason is that ReverseProxy checks the length of the original buffer to be copied and the actual bytes that were written. if it doesn't match, then it returns ErrShortWrite. The legth cannot equal, because for writting we use the gunzipResponseWriter that changes the resulting length.
It wouldn't fail if this commit wasn't merged - it checks if there was an error during the copy of the body and if there was, then it ends with panic.
Unfortunately, our tests are passing because the panic is supressed and only logged for tests: golang/go@eab57b2#diff-d863507a61be206d112f6e00e6d812a2

[xcoulon]

@alexeykazakov may I ask: what is the initial reason for having this gunzipResponseWriter ? AFAIK, the proxy should just forward the response back to the requesting client, maybe with a few changes in the returned response headers, but why do we even need to read the content of the response?

[xcoulon]

I mean, AFAICT, using the regular response writer seems to work fine: the client will get the response body compressed, but it's its own responsibility to decompress it upon reception, isn't it?

[MatousJobanek]

what is the initial reason for having this gunzipResponseWriter ?

isn't it because we need to gunzip the response to not have it gzipped twice?

[xcoulon]

what is the initial reason for having this gunzipResponseWriter ?

isn't it because we need to gunzip the response to not have it gzipped twice?

why would it be gzipped twice? ah, because the middleware on auth would gzip it again, of course! (🤦‍♂️ )

[MatousJobanek]

why would it be gzipped twice? ah, because the middleware on auth would gzip it again, of course! (man_facepalming )

yes - the gzip middleware is used for all endpoints in auth service: https://github.com/fabric8-services/fabric8-auth/blob/master/main.go#L135

But looking at the code of middleware: https://github.com/goadesign/goa/blob/master/middleware/gzip/middleware.go#L277
// Skip compression if the client doesn't accept gzip encoding,
we could remove the header from the request that is being forwarded, so we wouldn't need to uncompress it

[xcoulon]

why would it be gzipped twice? ah, because the middleware on auth would gzip it again, of course! (man_facepalming )

yes - the gzip middleware is used for all endpoints in auth service: https://github.com/fabric8-services/fabric8-auth/blob/master/main.go#L135

But looking at the code of middleware: https://github.com/goadesign/goa/blob/master/middleware/gzip/middleware.go#L277
// Skip compression if the client doesn't accept gzip encoding,
we could remove the header from the request that is being forwarded, so we wouldn't need to uncompress it

yes, that's exactly what I just found as well, and I confirm I can get the clean response (decoded with gzip -d) when not using our custom gunzipResponseWriter

[MatousJobanek]

Good job with it @alexeykazakov @xcoulon - it's great that we got rid of the gunzipResponseWriter 👍

[xcoulon]

reopening, since #735 did not correctly fixed the issue.

[alexeykazakov]

The issue seems to be fixed by fabric8-services/fabric8-common#80 and #736 but we need to investigate if we can get rid of our custom gunzip writer in the proxy at all.