gitservice: use an error hook instead of a logger (#56611)

src-cli doesn't use logger, so this changes the pattern to instead make
how we log injectable by the client. Currently the logger is just not
set in src-cli leading to a nil pointer panic. After this is merged a
follow up PR will go out for that.

Additionally this PR fixes a use for sourcegraph app which didn't set
the logger as well.

Test Plan: CI
This commit is contained in:
Keegan Carruthers-Smith 2023-09-14 12:36:05 +02:00 committed by GitHub
parent 6d252b7ec1
commit e3a8bc516d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 17 additions and 17 deletions

View File

@ -74,12 +74,14 @@ func (s *Server) gitServiceHandler() *gitservice.Handler {
logger := s.Logger.Scoped("gitServiceHandler", "smart Git HTTP transfer protocol")
return &gitservice.Handler{
Logger: logger,
Dir: func(d string) string {
return string(repoDirFromName(s.ReposDir, api.RepoName(d)))
},
ErrorHook: func(err error, stderr string) {
logger.Error("git-service error", log.Error(err), log.String("stderr", stderr))
},
// Limit rate of stdout from git.
CommandHook: func(cmd *exec.Cmd) {
cmd.Stdout = flowrateWriter(logger, cmd.Stdout)

View File

@ -167,6 +167,9 @@ func (s *Serve) handler() http.Handler {
// calling FromSlash.
return filepath.FromSlash("/" + name)
},
ErrorHook: func(err error, stderr string) {
s.Logger.Error("git-service error", log.Error(err), log.String("stderr", stderr))
},
Trace: func(ctx context.Context, svc, repo, protocol string) func(error) {
start := time.Now()
return func(err error) {

View File

@ -6,18 +6,12 @@ go_library(
srcs = ["gitservice.go"],
importpath = "github.com/sourcegraph/sourcegraph/lib/gitservice",
visibility = ["//visibility:public"],
deps = [
"//lib/errors",
"@com_github_sourcegraph_log//:log",
],
deps = ["//lib/errors"],
)
go_test(
name = "gitservice_test",
timeout = "short",
srcs = ["gitservice_test.go"],
deps = [
":gitservice",
"@com_github_sourcegraph_log//logtest",
],
deps = [":gitservice"],
)

View File

@ -11,8 +11,6 @@ import (
"strconv"
"strings"
"github.com/sourcegraph/log"
"github.com/sourcegraph/sourcegraph/lib/errors"
)
@ -42,12 +40,17 @@ var uploadPackArgs = []string{
// protocol. We aim to support modern git features such as protocol v2 to
// minimize traffic.
type Handler struct {
Logger log.Logger
// Dir is a funcion which takes a repository name and returns an absolute
// path to the GIT_DIR for it.
Dir func(string) string
// ErrorHook is called if we fail to run the git command. The main use of
// this is to inject logging. For example in src-cli we don't use
// sourcegraph/log so this allows us to use stdlib log.
//
// Note: This is required to be set
ErrorHook func(err error, stderr string)
// CommandHook if non-nil will run with the git upload command before we
// start the command.
//
@ -148,7 +151,7 @@ func (s *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
err = cmd.Run()
if err != nil {
err = errors.Errorf("error running git service command args=%q: %w", args, err)
s.Logger.Error("git-service error", log.Error(err), log.String("stderr", stderr.String()))
s.ErrorHook(err, stderr.String())
_, _ = w.Write([]byte("\n" + err.Error() + "\n"))
}
}

View File

@ -9,7 +9,6 @@ import (
"strings"
"testing"
"github.com/sourcegraph/log/logtest"
"github.com/sourcegraph/sourcegraph/lib/gitservice"
)
@ -34,7 +33,6 @@ func TestHandler(t *testing.T) {
}
ts := httptest.NewServer(&gitservice.Handler{
Logger: logtest.Scoped(t),
Dir: func(s string) string {
return filepath.Join(root, s, ".git")
},