artin/sourcegraph
1
0
Fork 0
mirror of https://github.com/sourcegraph/sourcegraph.git synced 2026-02-06 19:21:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

gitserverproxy: Handle panic (#44350)

Browse Source 
When the remote resp.Body cannot be read, the http proxy panics with `ErrAbortHandler`. That is, because at that point in time, the HTTP headers have already been sent and there is no other way out. This will then cause a connection reset error.
Since this error is not "as bad", we should recover it and log something useful instead.
This commit is contained in:
Erik Seliger 2022-11-16 01:17:12 +01:00 committed by GitHub
parent db73c83c78
commit 53df9beeb1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 32 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
enterprise/cmd/frontend/internal/executorqueue/gitserverproxy.go
View File

@ -6,9 +6,12 @@ import (
"net/http/httputil"
"net/url"
"path"
"runtime"
"github.com/gorilla/mux"
"github.com/sourcegraph/log"
"github.com/sourcegraph/sourcegraph/internal/api"
"github.com/sourcegraph/sourcegraph/internal/httpcli"
)
@ -20,7 +23,7 @@ type GitserverClient interface {
// gitserverProxy creates an HTTP handler that will proxy requests to the correct
// gitserver at the given gitPath.
func gitserverProxy(gitserverClient GitserverClient, gitPath string) http.Handler {
func gitserverProxy(logger log.Logger, gitserverClient GitserverClient, gitPath string) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
repo := getRepoName(r)
@ -42,6 +45,19 @@ func gitserverProxy(gitserverClient GitserverClient, gitPath string) http.Handle
},
Transport: httpcli.InternalClient.Transport,
}
defer func() {
e := recover()
if e != nil {
if e == http.ErrAbortHandler {
logger.Warn("failed to read gitserver response")
} else {
const size = 64 << 10
buf := make([]byte, size)
buf = buf[:runtime.Stack(buf, false)]
logger.Error("reverseproxy: panic reading response", log.String("stack", string(buf)))
}
}
}()
p.ServeHTTP(w, r)
})
}

10
enterprise/cmd/frontend/internal/executorqueue/gitserverproxy_test.go
View File

@ -8,6 +8,8 @@ import (
"net/url"
"path/filepath"
"testing"
"github.com/sourcegraph/log/logtest"
)
func TestGitserverProxySimple(t *testing.T) {
@ -24,7 +26,7 @@ func TestGitserverProxySimple(t *testing.T) {
gs := NewMockGitserverClient()
gs.AddrForRepoFunc.PushReturn(url.Host, nil)
proxyServer := httptest.NewServer(gitserverProxy(gs, "/info/refs"))
proxyServer := httptest.NewServer(gitserverProxy(logtest.Scoped(t), gs, "/info/refs"))
defer proxyServer.Close()
req, err := http.NewRequest("GET", proxyServer.URL, nil)
@ -63,7 +65,7 @@ func TestGitserverProxyTargetPath(t *testing.T) {
gs := NewMockGitserverClient()
gs.AddrForRepoFunc.PushReturn(url.Host, nil)
proxyServer := httptest.NewServer(gitserverProxy(gs, "/foo"))
proxyServer := httptest.NewServer(gitserverProxy(logtest.Scoped(t), gs, "/foo"))
defer proxyServer.Close()
req, err := http.NewRequest("GET", proxyServer.URL, nil)
@ -95,7 +97,7 @@ func TestGitserverProxyHeaders(t *testing.T) {
gs := NewMockGitserverClient()
gs.AddrForRepoFunc.PushReturn(url.Host, nil)
proxyServer := httptest.NewServer(gitserverProxy(gs, "/test"))
proxyServer := httptest.NewServer(gitserverProxy(logtest.Scoped(t), gs, "/test"))
defer proxyServer.Close()
req, err := http.NewRequest("GET", proxyServer.URL, nil)
@ -143,7 +145,7 @@ func TestGitserverProxyRedirectWithPayload(t *testing.T) {
gs := NewMockGitserverClient()
gs.AddrForRepoFunc.PushReturn(url.Host, nil)
proxyServer := httptest.NewServer(gitserverProxy(gs, "/test"))
proxyServer := httptest.NewServer(gitserverProxy(logtest.Scoped(t), gs, "/test"))
defer proxyServer.Close()
req, err := http.NewRequest("POST", proxyServer.URL, bytes.NewReader([]byte("foobarbaz")))

4
enterprise/cmd/frontend/internal/executorqueue/init.go
View File

@ -3,6 +3,8 @@ package executorqueue
import (
"context"
"github.com/sourcegraph/log"
"github.com/sourcegraph/sourcegraph/internal/conf/conftypes"
"github.com/sourcegraph/sourcegraph/internal/database"
"github.com/sourcegraph/sourcegraph/internal/observation"
@ -25,6 +27,7 @@ func Init(
batchesWorkspaceFileGetHandler := enterpriseServices.BatchesChangesFileGetHandler
batchesWorkspaceFileExistsHandler := enterpriseServices.BatchesChangesFileGetHandler
accessToken := func() string { return conf.SiteConfig().ExecutorsAccessToken }
logger := log.Scoped("executorqueue", "")
// Register queues. If this set changes, be sure to also update the list of valid
// queue names in ./metrics/queue_allocation.go, and register a metrics exporter
@ -37,6 +40,7 @@ func Init(
}
queueHandler, err := newExecutorQueueHandler(
logger,
db,
queueOptions,
accessToken,

8
enterprise/cmd/frontend/internal/executorqueue/queuehandler.go
View File

@ -8,6 +8,8 @@ import (
"github.com/gorilla/mux"
"github.com/inconshreveable/log15"
"github.com/sourcegraph/log"
"github.com/sourcegraph/sourcegraph/enterprise/cmd/frontend/internal/executorqueue/handler"
"github.com/sourcegraph/sourcegraph/internal/actor"
"github.com/sourcegraph/sourcegraph/internal/database"
@ -15,7 +17,7 @@ import (
metricsstore "github.com/sourcegraph/sourcegraph/internal/metrics/store"
)
func newExecutorQueueHandler(db database.DB, queueOptions []handler.QueueOptions, accessToken func() string, uploadHandler http.Handler, batchesWorkspaceFileGetHandler http.Handler, batchesWorkspaceFileExistsHandler http.Handler) (func() http.Handler, error) {
func newExecutorQueueHandler(logger log.Logger, db database.DB, queueOptions []handler.QueueOptions, accessToken func() string, uploadHandler http.Handler, batchesWorkspaceFileGetHandler http.Handler, batchesWorkspaceFileExistsHandler http.Handler) (func() http.Handler, error) {
metricsStore := metricsstore.NewDistributedStore("executors:")
executorStore := db.Executors()
gitserverClient := gitserver.NewClient(db)
@ -26,8 +28,8 @@ func newExecutorQueueHandler(db database.DB, queueOptions []handler.QueueOptions
base.StrictSlash(true)
// Proxy /info/refs and /git-upload-pack to gitservice for git clone/fetch.
base.Path("/git/{RepoName:.*}/info/refs").Handler(gitserverProxy(gitserverClient, "/info/refs"))
base.Path("/git/{RepoName:.*}/git-upload-pack").Handler(gitserverProxy(gitserverClient, "/git-upload-pack"))
base.Path("/git/{RepoName:.*}/info/refs").Handler(gitserverProxy(logger, gitserverClient, "/info/refs"))
base.Path("/git/{RepoName:.*}/git-upload-pack").Handler(gitserverProxy(logger, gitserverClient, "/git-upload-pack"))
// Serve the executor queue API.
handler.SetupRoutes(executorStore, metricsStore, queueOptions, base.PathPrefix("/queue/").Subrouter())