Refactor internal API for git commands, use meaningful messages instead of "Internal Server Error" (#23687)

# Why this PR comes

At first, I'd like to help users like #23636 (there are a lot)

The unclear "Internal Server Error" is quite anonying, scare users,
frustrate contributors, nobody knows what happens.

So, it's always good to provide meaningful messages to end users (of
course, do not leak sensitive information).

When I started working on the "response message to end users", I found
that the related code has a lot of technical debt. A lot of copy&paste
code, unclear fields and usages.

So I think it's good to make everything clear.

# Tech Backgrounds

Gitea has many sub-commands, some are used by admins, some are used by
SSH servers or Git Hooks. Many sub-commands use "internal API" to
communicate with Gitea web server.

Before, Gitea server always use `StatusCode + Json "err" field` to
return messages.

* The CLI sub-commands: they expect to show all error related messages
to site admin
* The Serv/Hook sub-commands (for git clients): they could only show
safe messages to end users, the error log could only be recorded by
"SSHLog" to Gitea web server.

In the old design, it assumes that:

* If the StatusCode is 500 (in some functions), then the "err" field is
error log, shouldn't be exposed to git client.
* If the StatusCode is 40x, then the "err" field could be exposed. And
some functions always read the "err" no matter what the StatusCode is.

The old code is not strict, and it's difficult to distinguish the
messages clearly and then output them correctly.

# This PR

To help to remove duplicate code and make everything clear, this PR
introduces `ResponseExtra` and `requestJSONResp`.

* `ResponseExtra` is a struct which contains "extra" information of a
internal API response, including StatusCode, UserMsg, Error
* `requestJSONResp` is a generic function which can be used for all
cases to help to simplify the calls.
* Remove all `map["err"]`, always use `private.Response{Err}` to
construct error messages.
* User messages and error messages are separated clearly, the `fail` and
`handleCliResponseExtra` will output correct messages.
* Replace all `Internal Server Error` messages with meaningful (still
safe) messages.

This PR saves more than 300 lines, while makes the git client messages
more clear.

Many gitea-serv/git-hook related essential functions are covered by
tests.

---------

Co-authored-by: delvh <dev.lh@web.de>

build/generate-emoji.go

@@ -60,13 +60,13 @@ func main() {
 	// generate data
 	buf, err := generate()
 	if err != nil {
-		log.Fatal(err)
+		log.Fatalf("generate err: %v", err)
 	}
 
 	// write
 	err = os.WriteFile(*flagOut, buf, 0o644)
 	if err != nil {
-		log.Fatal(err)
+		log.Fatalf("WriteFile err: %v", err)
 	}
 }
 

cmd/hook.go

@@ -6,9 +6,9 @@ package cmd
 import (
 	"bufio"
 	"bytes"
+	"context"
 	"fmt"
 	"io"
-	"net/http"
 	"os"
 	"strconv"
 	"strings"
@@ -167,11 +167,11 @@ func runHookPreReceive(c *cli.Context) error {
 	ctx, cancel := installSignals()
 	defer cancel()
 
-	setup("hooks/pre-receive.log", c.Bool("debug"))
+	setup(ctx, c.Bool("debug"))
 
 	if len(os.Getenv("SSH_ORIGINAL_COMMAND")) == 0 {
 		if setting.OnlyAllowPushIfGiteaEnvironmentSet {
-			return fail(`Rejecting changes as Gitea environment not set.
+			return fail(ctx, `Rejecting changes as Gitea environment not set.
 If you are pushing over SSH you must push with a key managed by
 Gitea or set your environment appropriately.`, "")
 		}
@@ -257,14 +257,9 @@ Gitea or set your environment appropriately.`, "")
 				hookOptions.OldCommitIDs = oldCommitIDs
 				hookOptions.NewCommitIDs = newCommitIDs
 				hookOptions.RefFullNames = refFullNames
-				statusCode, msg := private.HookPreReceive(ctx, username, reponame, hookOptions)
+				extra := private.HookPreReceive(ctx, username, reponame, hookOptions)
-				switch statusCode {
+				if extra.HasError() {
-				case http.StatusOK:
+					return fail(ctx, extra.UserMsg, "HookPreReceive(batch) failed: %v", extra.Error)
-					// no-op
-				case http.StatusInternalServerError:
-					return fail("Internal Server Error", msg)
-				default:
-					return fail(msg, "")
 				}
 				count = 0
 				lastline = 0
@@ -285,12 +280,9 @@ Gitea or set your environment appropriately.`, "")
 
 		fmt.Fprintf(out, " Checking %d references\n", count)
 
-		statusCode, msg := private.HookPreReceive(ctx, username, reponame, hookOptions)
+		extra := private.HookPreReceive(ctx, username, reponame, hookOptions)
-		switch statusCode {
+		if extra.HasError() {
-		case http.StatusInternalServerError:
+			return fail(ctx, extra.UserMsg, "HookPreReceive(last) failed: %v", extra.Error)
-			return fail("Internal Server Error", msg)
-		case http.StatusForbidden:
-			return fail(msg, "")
 		}
 	} else if lastline > 0 {
 		fmt.Fprintf(out, "\n")
@@ -309,7 +301,7 @@ func runHookPostReceive(c *cli.Context) error {
 	ctx, cancel := installSignals()
 	defer cancel()
 
-	setup("hooks/post-receive.log", c.Bool("debug"))
+	setup(ctx, c.Bool("debug"))
 
 	// First of all run update-server-info no matter what
 	if _, _, err := git.NewCommand(ctx, "update-server-info").RunStdString(nil); err != nil {
@@ -323,7 +315,7 @@ func runHookPostReceive(c *cli.Context) error {
 
 	if len(os.Getenv("SSH_ORIGINAL_COMMAND")) == 0 {
 		if setting.OnlyAllowPushIfGiteaEnvironmentSet {
-			return fail(`Rejecting changes as Gitea environment not set.
+			return fail(ctx, `Rejecting changes as Gitea environment not set.
 If you are pushing over SSH you must push with a key managed by
 Gitea or set your environment appropriately.`, "")
 		}
@@ -394,11 +386,11 @@ Gitea or set your environment appropriately.`, "")
 			hookOptions.OldCommitIDs = oldCommitIDs
 			hookOptions.NewCommitIDs = newCommitIDs
 			hookOptions.RefFullNames = refFullNames
-			resp, err := private.HookPostReceive(ctx, repoUser, repoName, hookOptions)
+			resp, extra := private.HookPostReceive(ctx, repoUser, repoName, hookOptions)
-			if resp == nil {
+			if extra.HasError() {
 				_ = dWriter.Close()
 				hookPrintResults(results)
-				return fail("Internal Server Error", err)
+				return fail(ctx, extra.UserMsg, "HookPostReceive failed: %v", extra.Error)
 			}
 			wasEmpty = wasEmpty || resp.RepoWasEmpty
 			results = append(results, resp.Results...)
@@ -409,9 +401,9 @@ Gitea or set your environment appropriately.`, "")
 	if count == 0 {
 		if wasEmpty && masterPushed {
 			// We need to tell the repo to reset the default branch to master
-			err := private.SetDefaultBranch(ctx, repoUser, repoName, "master")
+			extra := private.SetDefaultBranch(ctx, repoUser, repoName, "master")
-			if err != nil {
+			if extra.HasError() {
-				return fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err)
+				return fail(ctx, extra.UserMsg, "SetDefaultBranch failed: %v", extra.Error)
 			}
 		}
 		fmt.Fprintf(out, "Processed %d references in total\n", total)
@@ -427,11 +419,11 @@ Gitea or set your environment appropriately.`, "")
 
 	fmt.Fprintf(out, " Processing %d references\n", count)
 
-	resp, err := private.HookPostReceive(ctx, repoUser, repoName, hookOptions)
+	resp, extra := private.HookPostReceive(ctx, repoUser, repoName, hookOptions)
 	if resp == nil {
 		_ = dWriter.Close()
 		hookPrintResults(results)
-		return fail("Internal Server Error", err)
+		return fail(ctx, extra.UserMsg, "HookPostReceive failed: %v", extra.Error)
 	}
 	wasEmpty = wasEmpty || resp.RepoWasEmpty
 	results = append(results, resp.Results...)
@@ -440,9 +432,9 @@ Gitea or set your environment appropriately.`, "")
 
 	if wasEmpty && masterPushed {
 		// We need to tell the repo to reset the default branch to master
-		err := private.SetDefaultBranch(ctx, repoUser, repoName, "master")
+		extra := private.SetDefaultBranch(ctx, repoUser, repoName, "master")
-		if err != nil {
+		if extra.HasError() {
-			return fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err)
+			return fail(ctx, extra.UserMsg, "SetDefaultBranch failed: %v", extra.Error)
 		}
 	}
 	_ = dWriter.Close()
@@ -485,22 +477,22 @@ func pushOptions() map[string]string {
 }
 
 func runHookProcReceive(c *cli.Context) error {
-	setup("hooks/proc-receive.log", c.Bool("debug"))
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	setup(ctx, c.Bool("debug"))
 
 	if len(os.Getenv("SSH_ORIGINAL_COMMAND")) == 0 {
 		if setting.OnlyAllowPushIfGiteaEnvironmentSet {
-			return fail(`Rejecting changes as Gitea environment not set.
+			return fail(ctx, `Rejecting changes as Gitea environment not set.
 If you are pushing over SSH you must push with a key managed by
 Gitea or set your environment appropriately.`, "")
 		}
 		return nil
 	}
 
-	ctx, cancel := installSignals()
-	defer cancel()
-
 	if git.CheckGitVersionAtLeast("2.29") != nil {
-		return fail("Internal Server Error", "git not support proc-receive.")
+		return fail(ctx, "No proc-receive support", "current git version doesn't support proc-receive.")
 	}
 
 	reader := bufio.NewReader(os.Stdin)
@@ -515,7 +507,7 @@ Gitea or set your environment appropriately.`, "")
 	// H: PKT-LINE(version=1\0push-options...)
 	// H: flush-pkt
 
-	rs, err := readPktLine(reader, pktLineTypeData)
+	rs, err := readPktLine(ctx, reader, pktLineTypeData)
 	if err != nil {
 		return err
 	}
@@ -530,19 +522,19 @@ Gitea or set your environment appropriately.`, "")
 
 	index := bytes.IndexByte(rs.Data, byte(0))
 	if index >= len(rs.Data) {
-		return fail("Internal Server Error", "pkt-line: format error "+fmt.Sprint(rs.Data))
+		return fail(ctx, "Protocol: format error", "pkt-line: format error "+fmt.Sprint(rs.Data))
 	}
 
 	if index < 0 {
 		if len(rs.Data) == 10 && rs.Data[9] == '\n' {
 			index = 9
 		} else {
-			return fail("Internal Server Error", "pkt-line: format error "+fmt.Sprint(rs.Data))
+			return fail(ctx, "Protocol: format error", "pkt-line: format error "+fmt.Sprint(rs.Data))
 		}
 	}
 
 	if string(rs.Data[0:index]) != VersionHead {
-		return fail("Internal Server Error", "Received unsupported version: %s", string(rs.Data[0:index]))
+		return fail(ctx, "Protocol: version error", "Received unsupported version: %s", string(rs.Data[0:index]))
 	}
 	requestOptions = strings.Split(string(rs.Data[index+1:]), " ")
 
@@ -555,17 +547,17 @@ Gitea or set your environment appropriately.`, "")
 	}
 	response = append(response, '\n')
 
-	_, err = readPktLine(reader, pktLineTypeFlush)
+	_, err = readPktLine(ctx, reader, pktLineTypeFlush)
 	if err != nil {
 		return err
 	}
 
-	err = writeDataPktLine(os.Stdout, response)
+	err = writeDataPktLine(ctx, os.Stdout, response)
 	if err != nil {
 		return err
 	}
 
-	err = writeFlushPktLine(os.Stdout)
+	err = writeFlushPktLine(ctx, os.Stdout)
 	if err != nil {
 		return err
 	}
@@ -588,7 +580,7 @@ Gitea or set your environment appropriately.`, "")
 
 	for {
 		// note: pktLineTypeUnknow means pktLineTypeFlush and pktLineTypeData all allowed
-		rs, err = readPktLine(reader, pktLineTypeUnknow)
+		rs, err = readPktLine(ctx, reader, pktLineTypeUnknow)
 		if err != nil {
 			return err
 		}
@@ -609,7 +601,7 @@ Gitea or set your environment appropriately.`, "")
 
 	if hasPushOptions {
 		for {
-			rs, err = readPktLine(reader, pktLineTypeUnknow)
+			rs, err = readPktLine(ctx, reader, pktLineTypeUnknow)
 			if err != nil {
 				return err
 			}
@@ -626,9 +618,9 @@ Gitea or set your environment appropriately.`, "")
 	}
 
 	// 3. run hook
-	resp, err := private.HookProcReceive(ctx, repoUser, repoName, hookOptions)
+	resp, extra := private.HookProcReceive(ctx, repoUser, repoName, hookOptions)
-	if err != nil {
+	if extra.HasError() {
-		return fail("Internal Server Error", "run proc-receive hook failed :%v", err)
+		return fail(ctx, extra.UserMsg, "HookProcReceive failed: %v", extra.Error)
 	}
 
 	// 4. response result to service
@@ -649,7 +641,7 @@ Gitea or set your environment appropriately.`, "")
 
 	for _, rs := range resp.Results {
 		if len(rs.Err) > 0 {
-			err = writeDataPktLine(os.Stdout, []byte("ng "+rs.OriginalRef+" "+rs.Err))
+			err = writeDataPktLine(ctx, os.Stdout, []byte("ng "+rs.OriginalRef+" "+rs.Err))
 			if err != nil {
 				return err
 			}
@@ -657,43 +649,43 @@ Gitea or set your environment appropriately.`, "")
 		}
 
 		if rs.IsNotMatched {
-			err = writeDataPktLine(os.Stdout, []byte("ok "+rs.OriginalRef))
+			err = writeDataPktLine(ctx, os.Stdout, []byte("ok "+rs.OriginalRef))
 			if err != nil {
 				return err
 			}
-			err = writeDataPktLine(os.Stdout, []byte("option fall-through"))
+			err = writeDataPktLine(ctx, os.Stdout, []byte("option fall-through"))
 			if err != nil {
 				return err
 			}
 			continue
 		}
 
-		err = writeDataPktLine(os.Stdout, []byte("ok "+rs.OriginalRef))
+		err = writeDataPktLine(ctx, os.Stdout, []byte("ok "+rs.OriginalRef))
 		if err != nil {
 			return err
 		}
-		err = writeDataPktLine(os.Stdout, []byte("option refname "+rs.Ref))
+		err = writeDataPktLine(ctx, os.Stdout, []byte("option refname "+rs.Ref))
 		if err != nil {
 			return err
 		}
 		if rs.OldOID != git.EmptySHA {
-			err = writeDataPktLine(os.Stdout, []byte("option old-oid "+rs.OldOID))
+			err = writeDataPktLine(ctx, os.Stdout, []byte("option old-oid "+rs.OldOID))
 			if err != nil {
 				return err
 			}
 		}
-		err = writeDataPktLine(os.Stdout, []byte("option new-oid "+rs.NewOID))
+		err = writeDataPktLine(ctx, os.Stdout, []byte("option new-oid "+rs.NewOID))
 		if err != nil {
 			return err
 		}
 		if rs.IsForcePush {
-			err = writeDataPktLine(os.Stdout, []byte("option forced-update"))
+			err = writeDataPktLine(ctx, os.Stdout, []byte("option forced-update"))
 			if err != nil {
 				return err
 			}
 		}
 	}
-	err = writeFlushPktLine(os.Stdout)
+	err = writeFlushPktLine(ctx, os.Stdout)
 
 	return err
 }
@@ -718,7 +710,7 @@ type gitPktLine struct {
 	Data   []byte
 }
 
-func readPktLine(in *bufio.Reader, requestType pktLineType) (*gitPktLine, error) {
+func readPktLine(ctx context.Context, in *bufio.Reader, requestType pktLineType) (*gitPktLine, error) {
 	var (
 		err error
 		r   *gitPktLine
@@ -729,33 +721,33 @@ func readPktLine(in *bufio.Reader, requestType pktLineType) (*gitPktLine, error)
 	for i := 0; i < 4; i++ {
 		lengthBytes[i], err = in.ReadByte()
 		if err != nil {
-			return nil, fail("Internal Server Error", "Pkt-Line: read stdin failed : %v", err)
+			return nil, fail(ctx, "Protocol: stdin error", "Pkt-Line: read stdin failed : %v", err)
 		}
 	}
 
 	r = new(gitPktLine)
 	r.Length, err = strconv.ParseUint(string(lengthBytes), 16, 32)
 	if err != nil {
-		return nil, fail("Internal Server Error", "Pkt-Line format is wrong :%v", err)
+		return nil, fail(ctx, "Protocol: format parse error", "Pkt-Line format is wrong :%v", err)
 	}
 
 	if r.Length == 0 {
 		if requestType == pktLineTypeData {
-			return nil, fail("Internal Server Error", "Pkt-Line format is wrong")
+			return nil, fail(ctx, "Protocol: format data error", "Pkt-Line format is wrong")
 		}
 		r.Type = pktLineTypeFlush
 		return r, nil
 	}
 
 	if r.Length <= 4 || r.Length > 65520 || requestType == pktLineTypeFlush {
-		return nil, fail("Internal Server Error", "Pkt-Line format is wrong")
+		return nil, fail(ctx, "Protocol: format length error", "Pkt-Line format is wrong")
 	}
 
 	r.Data = make([]byte, r.Length-4)
 	for i := range r.Data {
 		r.Data[i], err = in.ReadByte()
 		if err != nil {
-			return nil, fail("Internal Server Error", "Pkt-Line: read stdin failed : %v", err)
+			return nil, fail(ctx, "Protocol: data error", "Pkt-Line: read stdin failed : %v", err)
 		}
 	}
 
@@ -764,19 +756,15 @@ func readPktLine(in *bufio.Reader, requestType pktLineType) (*gitPktLine, error)
 	return r, nil
 }
 
-func writeFlushPktLine(out io.Writer) error {
+func writeFlushPktLine(ctx context.Context, out io.Writer) error {
 	l, err := out.Write([]byte("0000"))
-	if err != nil {
+	if err != nil || l != 4 {
-		return fail("Internal Server Error", "Pkt-Line response failed: %v", err)
+		return fail(ctx, "Protocol: write error", "Pkt-Line response failed: %v", err)
 	}
-	if l != 4 {
-		return fail("Internal Server Error", "Pkt-Line response failed: %v", err)
-	}
-
 	return nil
 }
 
-func writeDataPktLine(out io.Writer, data []byte) error {
+func writeDataPktLine(ctx context.Context, out io.Writer, data []byte) error {
 	hexchar := []byte("0123456789abcdef")
 	hex := func(n uint64) byte {
 		return hexchar[(n)&15]
@@ -790,19 +778,13 @@ func writeDataPktLine(out io.Writer, data []byte) error {
 	tmp[3] = hex(length)
 
 	lr, err := out.Write(tmp)
-	if err != nil {
+	if err != nil || lr != 4 {
-		return fail("Internal Server Error", "Pkt-Line response failed: %v", err)
+		return fail(ctx, "Protocol: write error", "Pkt-Line response failed: %v", err)
-	}
-	if lr != 4 {
-		return fail("Internal Server Error", "Pkt-Line response failed: %v", err)
 	}
 
 	lr, err = out.Write(data)
-	if err != nil {
+	if err != nil || int(length-4) != lr {
-		return fail("Internal Server Error", "Pkt-Line response failed: %v", err)
+		return fail(ctx, "Protocol: write error", "Pkt-Line response failed: %v", err)
-	}
-	if int(length-4) != lr {
-		return fail("Internal Server Error", "Pkt-Line response failed: %v", err)
 	}
 
 	return nil

cmd/hook_test.go

@@ -6,6 +6,7 @@ package cmd
 import (
 	"bufio"
 	"bytes"
+	"context"
 	"strings"
 	"testing"
 
@@ -14,27 +15,28 @@ import (
 
 func TestPktLine(t *testing.T) {
 	// test read
+	ctx := context.Background()
 	s := strings.NewReader("0000")
 	r := bufio.NewReader(s)
-	result, err := readPktLine(r, pktLineTypeFlush)
+	result, err := readPktLine(ctx, r, pktLineTypeFlush)
 	assert.NoError(t, err)
 	assert.Equal(t, pktLineTypeFlush, result.Type)
 
 	s = strings.NewReader("0006a\n")
 	r = bufio.NewReader(s)
-	result, err = readPktLine(r, pktLineTypeData)
+	result, err = readPktLine(ctx, r, pktLineTypeData)
 	assert.NoError(t, err)
 	assert.Equal(t, pktLineTypeData, result.Type)
 	assert.Equal(t, []byte("a\n"), result.Data)
 
 	// test write
 	w := bytes.NewBuffer([]byte{})
-	err = writeFlushPktLine(w)
+	err = writeFlushPktLine(ctx, w)
 	assert.NoError(t, err)
 	assert.Equal(t, []byte("0000"), w.Bytes())
 
 	w.Reset()
-	err = writeDataPktLine(w, []byte("a\nb"))
+	err = writeDataPktLine(ctx, w, []byte("a\nb"))
 	assert.NoError(t, err)
 	assert.Equal(t, []byte("0007a\nb"), w.Bytes())
 }

cmd/keys.go

@@ -64,11 +64,12 @@ func runKeys(c *cli.Context) error {
 	ctx, cancel := installSignals()
 	defer cancel()
 
-	setup("keys.log", false)
+	setup(ctx, false)
 
-	authorizedString, err := private.AuthorizedPublicKeyByContent(ctx, content)
+	authorizedString, extra := private.AuthorizedPublicKeyByContent(ctx, content)
-	if err != nil {
+	// do not use handleCliResponseExtra or cli.NewExitError, if it exists immediately, it breaks some tests like Test_CmdKeys
-		return err
+	if extra.Error != nil {
+		return extra.Error
 	}
 	fmt.Println(strings.TrimSpace(authorizedString))
 	return nil

cmd/mailer.go

@@ -5,7 +5,6 @@ package cmd
 
 import (
 	"fmt"
-	"net/http"
 
 	"code.gitea.io/gitea/modules/private"
 	"code.gitea.io/gitea/modules/setting"
@@ -43,13 +42,10 @@ func runSendMail(c *cli.Context) error {
 		}
 	}
 
-	status, message := private.SendEmail(ctx, subject, body, nil)
+	respText, extra := private.SendEmail(ctx, subject, body, nil)
-	if status != http.StatusOK {
+	if extra.HasError() {
-		fmt.Printf("error: %s\n", message)
+		return handleCliResponseExtra(extra)
-		return nil
 	}
-
+	_, _ = fmt.Printf("Sent %s email(s) to all users\n", respText)
-	fmt.Printf("Success: %s\n", message)
-
 	return nil
 }

cmd/manager.go

@@ -4,8 +4,6 @@
 package cmd
 
 import (
-	"fmt"
-	"net/http"
 	"os"
 	"time"
 
@@ -103,57 +101,34 @@ func runShutdown(c *cli.Context) error {
 	ctx, cancel := installSignals()
 	defer cancel()
 
-	setup("manager", c.Bool("debug"))
+	setup(ctx, c.Bool("debug"))
-	statusCode, msg := private.Shutdown(ctx)
+	extra := private.Shutdown(ctx)
-	switch statusCode {
+	return handleCliResponseExtra(extra)
-	case http.StatusInternalServerError:
-		return fail("InternalServerError", msg)
-	}
-
-	fmt.Fprintln(os.Stdout, msg)
-	return nil
 }
 
 func runRestart(c *cli.Context) error {
 	ctx, cancel := installSignals()
 	defer cancel()
 
-	setup("manager", c.Bool("debug"))
+	setup(ctx, c.Bool("debug"))
-	statusCode, msg := private.Restart(ctx)
+	extra := private.Restart(ctx)
-	switch statusCode {
+	return handleCliResponseExtra(extra)
-	case http.StatusInternalServerError:
-		return fail("InternalServerError", msg)
-	}
-
-	fmt.Fprintln(os.Stdout, msg)
-	return nil
 }
 
 func runFlushQueues(c *cli.Context) error {
 	ctx, cancel := installSignals()
 	defer cancel()
 
-	setup("manager", c.Bool("debug"))
+	setup(ctx, c.Bool("debug"))
-	statusCode, msg := private.FlushQueues(ctx, c.Duration("timeout"), c.Bool("non-blocking"))
+	extra := private.FlushQueues(ctx, c.Duration("timeout"), c.Bool("non-blocking"))
-	switch statusCode {
+	return handleCliResponseExtra(extra)
-	case http.StatusInternalServerError:
-		return fail("InternalServerError", msg)
-	}
-
-	fmt.Fprintln(os.Stdout, msg)
-	return nil
 }
 
 func runProcesses(c *cli.Context) error {
 	ctx, cancel := installSignals()
 	defer cancel()
 
-	setup("manager", c.Bool("debug"))
+	setup(ctx, c.Bool("debug"))
-	statusCode, msg := private.Processes(ctx, os.Stdout, c.Bool("flat"), c.Bool("no-system"), c.Bool("stacktraces"), c.Bool("json"), c.String("cancel"))
+	extra := private.Processes(ctx, os.Stdout, c.Bool("flat"), c.Bool("no-system"), c.Bool("stacktraces"), c.Bool("json"), c.String("cancel"))
-	switch statusCode {
+	return handleCliResponseExtra(extra)
-	case http.StatusInternalServerError:
-		return fail("InternalServerError", msg)
-	}
-
-	return nil
 }

cmd/manager_logging.go

@@ -5,7 +5,6 @@ package cmd
 
 import (
 	"fmt"
-	"net/http"
 	"os"
 
 	"code.gitea.io/gitea/modules/log"
@@ -191,27 +190,25 @@ var (
 )
 
 func runRemoveLogger(c *cli.Context) error {
-	setup("manager", c.Bool("debug"))
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	setup(ctx, c.Bool("debug"))
 	group := c.String("group")
 	if len(group) == 0 {
 		group = log.DEFAULT
 	}
 	name := c.Args().First()
-	ctx, cancel := installSignals()
-	defer cancel()
 
-	statusCode, msg := private.RemoveLogger(ctx, group, name)
+	extra := private.RemoveLogger(ctx, group, name)
-	switch statusCode {
+	return handleCliResponseExtra(extra)
-	case http.StatusInternalServerError:
-		return fail("InternalServerError", msg)
-	}
-
-	fmt.Fprintln(os.Stdout, msg)
-	return nil
 }
 
 func runAddSMTPLogger(c *cli.Context) error {
-	setup("manager", c.Bool("debug"))
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	setup(ctx, c.Bool("debug"))
 	vals := map[string]interface{}{}
 	mode := "smtp"
 	if c.IsSet("host") {
@@ -242,7 +239,10 @@ func runAddSMTPLogger(c *cli.Context) error {
 }
 
 func runAddConnLogger(c *cli.Context) error {
-	setup("manager", c.Bool("debug"))
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	setup(ctx, c.Bool("debug"))
 	vals := map[string]interface{}{}
 	mode := "conn"
 	vals["net"] = "tcp"
@@ -269,7 +269,10 @@ func runAddConnLogger(c *cli.Context) error {
 }
 
 func runAddFileLogger(c *cli.Context) error {
-	setup("manager", c.Bool("debug"))
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	setup(ctx, c.Bool("debug"))
 	vals := map[string]interface{}{}
 	mode := "file"
 	if c.IsSet("filename") {
@@ -299,7 +302,10 @@ func runAddFileLogger(c *cli.Context) error {
 }
 
 func runAddConsoleLogger(c *cli.Context) error {
-	setup("manager", c.Bool("debug"))
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	setup(ctx, c.Bool("debug"))
 	vals := map[string]interface{}{}
 	mode := "console"
 	if c.IsSet("stderr") && c.Bool("stderr") {
@@ -338,28 +344,17 @@ func commonAddLogger(c *cli.Context, mode string, vals map[string]interface{}) e
 	ctx, cancel := installSignals()
 	defer cancel()
 
-	statusCode, msg := private.AddLogger(ctx, group, name, mode, vals)
+	extra := private.AddLogger(ctx, group, name, mode, vals)
-	switch statusCode {
+	return handleCliResponseExtra(extra)
-	case http.StatusInternalServerError:
-		return fail("InternalServerError", msg)
-	}
-
-	fmt.Fprintln(os.Stdout, msg)
-	return nil
 }
 
 func runPauseLogging(c *cli.Context) error {
 	ctx, cancel := installSignals()
 	defer cancel()
 
-	setup("manager", c.Bool("debug"))
+	setup(ctx, c.Bool("debug"))
-	statusCode, msg := private.PauseLogging(ctx)
+	userMsg := private.PauseLogging(ctx)
-	switch statusCode {
+	_, _ = fmt.Fprintln(os.Stdout, userMsg)
-	case http.StatusInternalServerError:
-		return fail("InternalServerError", msg)
-	}
-
-	fmt.Fprintln(os.Stdout, msg)
 	return nil
 }
 
@@ -367,14 +362,9 @@ func runResumeLogging(c *cli.Context) error {
 	ctx, cancel := installSignals()
 	defer cancel()
 
-	setup("manager", c.Bool("debug"))
+	setup(ctx, c.Bool("debug"))
-	statusCode, msg := private.ResumeLogging(ctx)
+	userMsg := private.ResumeLogging(ctx)
-	switch statusCode {
+	_, _ = fmt.Fprintln(os.Stdout, userMsg)
-	case http.StatusInternalServerError:
-		return fail("InternalServerError", msg)
-	}
-
-	fmt.Fprintln(os.Stdout, msg)
 	return nil
 }
 
@@ -382,28 +372,17 @@ func runReleaseReopenLogging(c *cli.Context) error {
 	ctx, cancel := installSignals()
 	defer cancel()
 
-	setup("manager", c.Bool("debug"))
+	setup(ctx, c.Bool("debug"))
-	statusCode, msg := private.ReleaseReopenLogging(ctx)
+	userMsg := private.ReleaseReopenLogging(ctx)
-	switch statusCode {
+	_, _ = fmt.Fprintln(os.Stdout, userMsg)
-	case http.StatusInternalServerError:
-		return fail("InternalServerError", msg)
-	}
-
-	fmt.Fprintln(os.Stdout, msg)
 	return nil
 }
 
 func runSetLogSQL(c *cli.Context) error {
 	ctx, cancel := installSignals()
 	defer cancel()
-	setup("manager", c.Bool("debug"))
+	setup(ctx, c.Bool("debug"))
 
-	statusCode, msg := private.SetLogSQL(ctx, !c.Bool("off"))
+	extra := private.SetLogSQL(ctx, !c.Bool("off"))
-	switch statusCode {
+	return handleCliResponseExtra(extra)
-	case http.StatusInternalServerError:
-		return fail("InternalServerError", msg)
-	}
-
-	fmt.Fprintln(os.Stdout, msg)
-	return nil
 }

cmd/restore_repo.go

@@ -4,11 +4,8 @@
 package cmd
 
 import (
-	"errors"
-	"net/http"
 	"strings"
 
-	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/private"
 	"code.gitea.io/gitea/modules/setting"
 
@@ -60,7 +57,7 @@ func runRestoreRepository(c *cli.Context) error {
 	if s := c.String("units"); s != "" {
 		units = strings.Split(s, ",")
 	}
-	statusCode, errStr := private.RestoreRepo(
+	extra := private.RestoreRepo(
 		ctx,
 		c.String("repo_dir"),
 		c.String("owner_name"),
@@ -68,10 +65,5 @@ func runRestoreRepository(c *cli.Context) error {
 		units,
 		c.Bool("validation"),
 	)
-	if statusCode == http.StatusOK {
+	return handleCliResponseExtra(extra)
-		return nil
-	}
-
-	log.Fatal("Failed to restore repository: %v", errStr)
-	return errors.New(errStr)
 }

cmd/serv.go

@@ -7,7 +7,6 @@ package cmd
 import (
 	"context"
 	"fmt"
-	"net/http"
 	"net/url"
 	"os"
 	"os/exec"
@@ -16,6 +15,7 @@ import (
 	"strconv"
 	"strings"
 	"time"
+	"unicode"
 
 	asymkey_model "code.gitea.io/gitea/models/asymkey"
 	git_model "code.gitea.io/gitea/models/git"
@@ -55,7 +55,7 @@ var CmdServ = cli.Command{
 	},
 }
 
-func setup(logPath string, debug bool) {
+func setup(ctx context.Context, debug bool) {
 	_ = log.DelLogger("console")
 	if debug {
 		_ = log.NewLogger(1000, "console", "console", `{"level":"trace","stacktracelevel":"NONE","stderr":true}`)
@@ -72,15 +72,15 @@ func setup(logPath string, debug bool) {
 	// `[repository]` `ROOT` is a relative path and $GITEA_WORK_DIR isn't passed to the SSH connection.
 	if _, err := os.Stat(setting.RepoRootPath); err != nil {
 		if os.IsNotExist(err) {
-			_ = fail("Incorrect configuration, no repository directory.", "Directory `[repository].ROOT` %q was not found, please check if $GITEA_WORK_DIR is passed to the SSH connection or make `[repository].ROOT` an absolute value.", setting.RepoRootPath)
+			_ = fail(ctx, "Incorrect configuration, no repository directory.", "Directory `[repository].ROOT` %q was not found, please check if $GITEA_WORK_DIR is passed to the SSH connection or make `[repository].ROOT` an absolute value.", setting.RepoRootPath)
 		} else {
-			_ = fail("Incorrect configuration, repository directory is inaccessible", "Directory `[repository].ROOT` %q is inaccessible. err: %v", setting.RepoRootPath, err)
+			_ = fail(ctx, "Incorrect configuration, repository directory is inaccessible", "Directory `[repository].ROOT` %q is inaccessible. err: %v", setting.RepoRootPath, err)
 		}
 		return
 	}
 
 	if err := git.InitSimple(context.Background()); err != nil {
-		_ = fail("Failed to init git", "Failed to init git, err: %v", err)
+		_ = fail(ctx, "Failed to init git", "Failed to init git, err: %v", err)
 	}
 }
 
@@ -94,32 +94,54 @@ var (
 	alphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`)
 )
 
-func fail(userMessage, logMessage string, args ...interface{}) error {
+// fail prints message to stdout, it's mainly used for git serv and git hook commands.
+// The output will be passed to git client and shown to user.
+func fail(ctx context.Context, userMessage, logMsgFmt string, args ...interface{}) error {
+	if userMessage == "" {
+		userMessage = "Internal Server Error (no specific error)"
+	}
+
 	// There appears to be a chance to cause a zombie process and failure to read the Exit status
 	// if nothing is outputted on stdout.
 	_, _ = fmt.Fprintln(os.Stdout, "")
 	_, _ = fmt.Fprintln(os.Stderr, "Gitea:", userMessage)
 
-	if len(logMessage) > 0 {
+	if logMsgFmt != "" {
+		logMsg := fmt.Sprintf(logMsgFmt, args...)
 		if !setting.IsProd {
-			_, _ = fmt.Fprintf(os.Stderr, logMessage+"\n", args...)
+			_, _ = fmt.Fprintln(os.Stderr, "Gitea:", logMsg)
 		}
-	}
+		if userMessage != "" {
-	ctx, cancel := installSignals()
+			if unicode.IsPunct(rune(userMessage[len(userMessage)-1])) {
-	defer cancel()
+				logMsg = userMessage + " " + logMsg
-
+			} else {
-	if len(logMessage) > 0 {
+				logMsg = userMessage + ". " + logMsg
-		_ = private.SSHLog(ctx, true, fmt.Sprintf(logMessage+": ", args...))
+			}
+		}
+		_ = private.SSHLog(ctx, true, logMsg)
 	}
 	return cli.NewExitError("", 1)
 }
 
+// handleCliResponseExtra handles the extra response from the cli sub-commands
+// If there is a user message it will be printed to stdout
+// If the command failed it will return an error (the error will be printed by cli framework)
+func handleCliResponseExtra(extra private.ResponseExtra) error {
+	if extra.UserMsg != "" {
+		_, _ = fmt.Fprintln(os.Stdout, extra.UserMsg)
+	}
+	if extra.HasError() {
+		return cli.NewExitError(extra.Error, 1)
+	}
+	return nil
+}
+
 func runServ(c *cli.Context) error {
 	ctx, cancel := installSignals()
 	defer cancel()
 
 	// FIXME: This needs to internationalised
-	setup("serv.log", c.Bool("debug"))
+	setup(ctx, c.Bool("debug"))
 
 	if setting.SSH.Disabled {
 		println("Gitea: SSH has been disabled")
@@ -135,18 +157,18 @@ func runServ(c *cli.Context) error {
 
 	keys := strings.Split(c.Args()[0], "-")
 	if len(keys) != 2 || keys[0] != "key" {
-		return fail("Key ID format error", "Invalid key argument: %s", c.Args()[0])
+		return fail(ctx, "Key ID format error", "Invalid key argument: %s", c.Args()[0])
 	}
 	keyID, err := strconv.ParseInt(keys[1], 10, 64)
 	if err != nil {
-		return fail("Key ID format error", "Invalid key argument: %s", c.Args()[1])
+		return fail(ctx, "Key ID parsing error", "Invalid key argument: %s", c.Args()[1])
 	}
 
 	cmd := os.Getenv("SSH_ORIGINAL_COMMAND")
 	if len(cmd) == 0 {
 		key, user, err := private.ServNoCommand(ctx, keyID)
 		if err != nil {
-			return fail("Internal error", "Failed to check provided key: %v", err)
+			return fail(ctx, "Key check failed", "Failed to check provided key: %v", err)
 		}
 		switch key.Type {
 		case asymkey_model.KeyTypeDeploy:
@@ -164,7 +186,7 @@ func runServ(c *cli.Context) error {
 
 	words, err := shellquote.Split(cmd)
 	if err != nil {
-		return fail("Error parsing arguments", "Failed to parse arguments: %v", err)
+		return fail(ctx, "Error parsing arguments", "Failed to parse arguments: %v", err)
 	}
 
 	if len(words) < 2 {
@@ -175,7 +197,7 @@ func runServ(c *cli.Context) error {
 				return nil
 			}
 		}
-		return fail("Too few arguments", "Too few arguments in cmd: %s", cmd)
+		return fail(ctx, "Too few arguments", "Too few arguments in cmd: %s", cmd)
 	}
 
 	verb := words[0]
@@ -187,7 +209,7 @@ func runServ(c *cli.Context) error {
 	var lfsVerb string
 	if verb == lfsAuthenticateVerb {
 		if !setting.LFS.StartServer {
-			return fail("Unknown git command", "LFS authentication request over SSH denied, LFS support is disabled")
+			return fail(ctx, "Unknown git command", "LFS authentication request over SSH denied, LFS support is disabled")
 		}
 
 		if len(words) > 2 {
@@ -200,37 +222,37 @@ func runServ(c *cli.Context) error {
 
 	rr := strings.SplitN(repoPath, "/", 2)
 	if len(rr) != 2 {
-		return fail("Invalid repository path", "Invalid repository path: %v", repoPath)
+		return fail(ctx, "Invalid repository path", "Invalid repository path: %v", repoPath)
 	}
 
 	username := strings.ToLower(rr[0])
 	reponame := strings.ToLower(strings.TrimSuffix(rr[1], ".git"))
 
 	if alphaDashDotPattern.MatchString(reponame) {
-		return fail("Invalid repo name", "Invalid repo name: %s", reponame)
+		return fail(ctx, "Invalid repo name", "Invalid repo name: %s", reponame)
 	}
 
 	if c.Bool("enable-pprof") {
 		if err := os.MkdirAll(setting.PprofDataPath, os.ModePerm); err != nil {
-			return fail("Error while trying to create PPROF_DATA_PATH", "Error while trying to create PPROF_DATA_PATH: %v", err)
+			return fail(ctx, "Error while trying to create PPROF_DATA_PATH", "Error while trying to create PPROF_DATA_PATH: %v", err)
 		}
 
 		stopCPUProfiler, err := pprof.DumpCPUProfileForUsername(setting.PprofDataPath, username)
 		if err != nil {
-			return fail("Internal Server Error", "Unable to start CPU profile: %v", err)
+			return fail(ctx, "Unable to start CPU profiler", "Unable to start CPU profile: %v", err)
 		}
 		defer func() {
 			stopCPUProfiler()
 			err := pprof.DumpMemProfileForUsername(setting.PprofDataPath, username)
 			if err != nil {
-				_ = fail("Internal Server Error", "Unable to dump Mem Profile: %v", err)
+				_ = fail(ctx, "Unable to dump Mem profile", "Unable to dump Mem Profile: %v", err)
 			}
 		}()
 	}
 
 	requestedMode, has := allowedCommands[verb]
 	if !has {
-		return fail("Unknown git command", "Unknown git command %s", verb)
+		return fail(ctx, "Unknown git command", "Unknown git command %s", verb)
 	}
 
 	if verb == lfsAuthenticateVerb {
@@ -239,20 +261,13 @@ func runServ(c *cli.Context) error {
 		} else if lfsVerb == "download" {
 			requestedMode = perm.AccessModeRead
 		} else {
-			return fail("Unknown LFS verb", "Unknown lfs verb %s", lfsVerb)
+			return fail(ctx, "Unknown LFS verb", "Unknown lfs verb %s", lfsVerb)
 		}
 	}
 
-	results, err := private.ServCommand(ctx, keyID, username, reponame, requestedMode, verb, lfsVerb)
+	results, extra := private.ServCommand(ctx, keyID, username, reponame, requestedMode, verb, lfsVerb)
-	if err != nil {
+	if extra.HasError() {
-		if private.IsErrServCommand(err) {
+		return fail(ctx, extra.UserMsg, "ServCommand failed: %s", extra.Error)
-			errServCommand := err.(private.ErrServCommand)
-			if errServCommand.StatusCode != http.StatusInternalServerError {
-				return fail("Unauthorized", "%s", errServCommand.Error())
-			}
-			return fail("Internal Server Error", "%s", errServCommand.Error())
-		}
-		return fail("Internal Server Error", "%s", err.Error())
 	}
 
 	// LFS token authentication
@@ -274,7 +289,7 @@ func runServ(c *cli.Context) error {
 		// Sign and get the complete encoded token as a string using the secret
 		tokenString, err := token.SignedString(setting.LFS.JWTSecretBytes)
 		if err != nil {
-			return fail("Internal error", "Failed to sign JWT token: %v", err)
+			return fail(ctx, "Failed to sign JWT Token", "Failed to sign JWT token: %v", err)
 		}
 
 		tokenAuthentication := &git_model.LFSTokenResponse{
@@ -286,7 +301,7 @@ func runServ(c *cli.Context) error {
 		enc := json.NewEncoder(os.Stdout)
 		err = enc.Encode(tokenAuthentication)
 		if err != nil {
-			return fail("Internal error", "Failed to encode LFS json response: %v", err)
+			return fail(ctx, "Failed to encode LFS json response", "Failed to encode LFS json response: %v", err)
 		}
 		return nil
 	}
@@ -332,13 +347,13 @@ func runServ(c *cli.Context) error {
 	gitcmd.Env = append(gitcmd.Env, git.CommonCmdServEnvs()...)
 
 	if err = gitcmd.Run(); err != nil {
-		return fail("Internal error", "Failed to execute git command: %v", err)
+		return fail(ctx, "Failed to execute git command", "Failed to execute git command: %v", err)
 	}
 
 	// Update user key activity.
 	if results.KeyID > 0 {
 		if err = private.UpdatePublicKeyInRepo(ctx, results.KeyID, results.RepoID); err != nil {
-			return fail("Internal error", "UpdatePublicKeyInRepo: %v", err)
+			return fail(ctx, "Failed to update public key", "UpdatePublicKeyInRepo: %v", err)
 		}
 	}
 

modules/httplib/httplib.go

@@ -8,6 +8,7 @@ import (
 	"bytes"
 	"context"
 	"crypto/tls"
+	"fmt"
 	"io"
 	"net"
 	"net/http"
@@ -68,6 +69,11 @@ func (r *Request) SetTimeout(connectTimeout, readWriteTimeout time.Duration) *Re
 	return r
 }
 
+func (r *Request) SetReadWriteTimeout(readWriteTimeout time.Duration) *Request {
+	r.setting.ReadWriteTimeout = readWriteTimeout
+	return r
+}
+
 // SetTLSClientConfig sets tls connection configurations if visiting https url.
 func (r *Request) SetTLSClientConfig(config *tls.Config) *Request {
 	r.setting.TLSClientConfig = config
@@ -138,11 +144,11 @@ func (r *Request) getResponse() (*http.Response, error) {
 		r.Body(paramBody)
 	}
 
-	url, err := url.Parse(r.url)
+	var err error
+	r.req.URL, err = url.Parse(r.url)
 	if err != nil {
 		return nil, err
 	}
-	r.req.URL = url
 
 	trans := r.setting.Transport
 	if trans == nil {
@@ -194,3 +200,7 @@ func TimeoutDialer(cTimeout time.Duration) func(ctx context.Context, net, addr s
 		return conn, nil
 	}
 }
+
+func (r *Request) GoString() string {
+	return fmt.Sprintf("%s %s", r.req.Method, r.url)
+}

modules/private/hook.go

@@ -5,14 +5,11 @@ package private
 
 import (
 	"context"
-	"errors"
 	"fmt"
-	"net/http"
 	"net/url"
 	"strconv"
 	"time"
 
-	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/setting"
 )
 
@@ -99,126 +96,46 @@ type HookProcReceiveRefResult struct {
 }
 
 // HookPreReceive check whether the provided commits are allowed
-func HookPreReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) (int, string) {
+func HookPreReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) ResponseExtra {
-	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/pre-receive/%s/%s",
+	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/pre-receive/%s/%s", url.PathEscape(ownerName), url.PathEscape(repoName))
-		url.PathEscape(ownerName),
+	req := newInternalRequest(ctx, reqURL, "POST", opts)
-		url.PathEscape(repoName),
+	req.SetReadWriteTimeout(time.Duration(60+len(opts.OldCommitIDs)) * time.Second)
-	)
+	_, extra := requestJSONResp(req, &responseText{})
-	req := newInternalRequest(ctx, reqURL, "POST")
+	return extra
-	req = req.Header("Content-Type", "application/json")
-	jsonBytes, _ := json.Marshal(opts)
-	req.Body(jsonBytes)
-	req.SetTimeout(60*time.Second, time.Duration(60+len(opts.OldCommitIDs))*time.Second)
-	resp, err := req.Response()
-	if err != nil {
-		return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error())
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		return resp.StatusCode, decodeJSONError(resp).Err
-	}
-
-	return http.StatusOK, ""
 }
 
 // HookPostReceive updates services and users
-func HookPostReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) (*HookPostReceiveResult, string) {
+func HookPostReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) (*HookPostReceiveResult, ResponseExtra) {
-	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/post-receive/%s/%s",
+	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/post-receive/%s/%s", url.PathEscape(ownerName), url.PathEscape(repoName))
-		url.PathEscape(ownerName),
+	req := newInternalRequest(ctx, reqURL, "POST", opts)
-		url.PathEscape(repoName),
+	req.SetReadWriteTimeout(time.Duration(60+len(opts.OldCommitIDs)) * time.Second)
-	)
+	return requestJSONResp(req, &HookPostReceiveResult{})
-
-	req := newInternalRequest(ctx, reqURL, "POST")
-	req = req.Header("Content-Type", "application/json")
-	req.SetTimeout(60*time.Second, time.Duration(60+len(opts.OldCommitIDs))*time.Second)
-	jsonBytes, _ := json.Marshal(opts)
-	req.Body(jsonBytes)
-	resp, err := req.Response()
-	if err != nil {
-		return nil, fmt.Sprintf("Unable to contact gitea: %v", err.Error())
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		return nil, decodeJSONError(resp).Err
-	}
-	res := &HookPostReceiveResult{}
-	_ = json.NewDecoder(resp.Body).Decode(res)
-
-	return res, ""
 }
 
 // HookProcReceive proc-receive hook
-func HookProcReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) (*HookProcReceiveResult, error) {
+func HookProcReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) (*HookProcReceiveResult, ResponseExtra) {
-	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/proc-receive/%s/%s",
+	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/proc-receive/%s/%s", url.PathEscape(ownerName), url.PathEscape(repoName))
-		url.PathEscape(ownerName),
-		url.PathEscape(repoName),
-	)
 
-	req := newInternalRequest(ctx, reqURL, "POST")
+	req := newInternalRequest(ctx, reqURL, "POST", opts)
-	req = req.Header("Content-Type", "application/json")
+	req.SetReadWriteTimeout(time.Duration(60+len(opts.OldCommitIDs)) * time.Second)
-	req.SetTimeout(60*time.Second, time.Duration(60+len(opts.OldCommitIDs))*time.Second)
+	return requestJSONResp(req, &HookProcReceiveResult{})
-	jsonBytes, _ := json.Marshal(opts)
-	req.Body(jsonBytes)
-	resp, err := req.Response()
-	if err != nil {
-		return nil, fmt.Errorf("Unable to contact gitea: %w", err)
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		return nil, errors.New(decodeJSONError(resp).Err)
-	}
-	res := &HookProcReceiveResult{}
-	_ = json.NewDecoder(resp.Body).Decode(res)
-
-	return res, nil
 }
 
 // SetDefaultBranch will set the default branch to the provided branch for the provided repository
-func SetDefaultBranch(ctx context.Context, ownerName, repoName, branch string) error {
+func SetDefaultBranch(ctx context.Context, ownerName, repoName, branch string) ResponseExtra {
 	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/set-default-branch/%s/%s/%s",
 		url.PathEscape(ownerName),
 		url.PathEscape(repoName),
 		url.PathEscape(branch),
 	)
 	req := newInternalRequest(ctx, reqURL, "POST")
-	req = req.Header("Content-Type", "application/json")
+	return requestJSONUserMsg(req, "")
-
-	req.SetTimeout(60*time.Second, 60*time.Second)
-	resp, err := req.Response()
-	if err != nil {
-		return fmt.Errorf("Unable to contact gitea: %w", err)
-	}
-	defer resp.Body.Close()
-	if resp.StatusCode != http.StatusOK {
-		return fmt.Errorf("Error returned from gitea: %v", decodeJSONError(resp).Err)
-	}
-	return nil
 }
 
 // SSHLog sends ssh error log response
 func SSHLog(ctx context.Context, isErr bool, msg string) error {
 	reqURL := setting.LocalURL + "api/internal/ssh/log"
-	req := newInternalRequest(ctx, reqURL, "POST")
+	req := newInternalRequest(ctx, reqURL, "POST", &SSHLogOption{IsError: isErr, Message: msg})
-	req = req.Header("Content-Type", "application/json")
+	_, extra := requestJSONResp(req, &responseText{})
-
+	return extra.Error
-	jsonBytes, _ := json.Marshal(&SSHLogOption{
-		IsError: isErr,
-		Message: msg,
-	})
-	req.Body(jsonBytes)
-
-	req.SetTimeout(60*time.Second, 60*time.Second)
-	resp, err := req.Response()
-	if err != nil {
-		return fmt.Errorf("unable to contact gitea: %w", err)
-	}
-
-	defer resp.Body.Close()
-	if resp.StatusCode != http.StatusOK {
-		return fmt.Errorf("Error returned from gitea: %v", decodeJSONError(resp).Err)
-	}
-	return nil
 }

modules/private/internal.go

@@ -11,6 +11,7 @@ import (
 	"net/http"
 	"os"
 	"strings"
+	"time"
 
 	"code.gitea.io/gitea/modules/httplib"
 	"code.gitea.io/gitea/modules/json"
@@ -19,29 +20,10 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 )
 
-func newRequest(ctx context.Context, url, method, sourceIP string) *httplib.Request {
+// Response is used for internal request response (for user message and error message)
-	if setting.InternalToken == "" {
-		log.Fatal(`The INTERNAL_TOKEN setting is missing from the configuration file: %q.
-Ensure you are running in the correct environment or set the correct configuration file with -c.`, setting.CustomConf)
-	}
-	return httplib.NewRequest(url, method).
-		SetContext(ctx).
-		Header("X-Real-IP", sourceIP).
-		Header("Authorization", fmt.Sprintf("Bearer %s", setting.InternalToken))
-}
-
-// Response internal request response
 type Response struct {
-	Err string `json:"err"`
+	Err     string `json:"err,omitempty"`      // server-side error log message, it won't be exposed to end users
-}
+	UserMsg string `json:"user_msg,omitempty"` // meaningful error message for end users, it will be shown in git client's output.
-
-func decodeJSONError(resp *http.Response) *Response {
-	var res Response
-	err := json.NewDecoder(resp.Body).Decode(&res)
-	if err != nil {
-		res.Err = err.Error()
-	}
-	return &res
 }
 
 func getClientIP() string {
@@ -52,11 +34,21 @@ func getClientIP() string {
 	return strings.Fields(sshConnEnv)[0]
 }
 
-func newInternalRequest(ctx context.Context, url, method string) *httplib.Request {
+func newInternalRequest(ctx context.Context, url, method string, body ...any) *httplib.Request {
-	req := newRequest(ctx, url, method, getClientIP()).SetTLSClientConfig(&tls.Config{
+	if setting.InternalToken == "" {
-		InsecureSkipVerify: true,
+		log.Fatal(`The INTERNAL_TOKEN setting is missing from the configuration file: %q.
-		ServerName:         setting.Domain,
+Ensure you are running in the correct environment or set the correct configuration file with -c.`, setting.CustomConf)
-	})
+	}
+
+	req := httplib.NewRequest(url, method).
+		SetContext(ctx).
+		Header("X-Real-IP", getClientIP()).
+		Header("Authorization", fmt.Sprintf("Bearer %s", setting.InternalToken)).
+		SetTLSClientConfig(&tls.Config{
+			InsecureSkipVerify: true,
+			ServerName:         setting.Domain,
+		})
+
 	if setting.Protocol == setting.HTTPUnix {
 		req.SetTransport(&http.Transport{
 			DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
@@ -90,5 +82,15 @@ func newInternalRequest(ctx context.Context, url, method string) *httplib.Reques
 			},
 		})
 	}
+
+	if len(body) == 1 {
+		req.Header("Content-Type", "application/json")
+		jsonBytes, _ := json.Marshal(body[0])
+		req.Body(jsonBytes)
+	} else if len(body) > 1 {
+		log.Fatal("Too many arguments for newInternalRequest")
+	}
+
+	req.SetTimeout(10*time.Second, 60*time.Second)
 	return req
 }

modules/private/key.go

@@ -6,8 +6,6 @@ package private
 import (
 	"context"
 	"fmt"
-	"io"
-	"net/http"
 
 	"code.gitea.io/gitea/modules/setting"
 )
@@ -16,39 +14,18 @@ import (
 func UpdatePublicKeyInRepo(ctx context.Context, keyID, repoID int64) error {
 	// Ask for running deliver hook and test pull request tasks.
 	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/ssh/%d/update/%d", keyID, repoID)
-	resp, err := newInternalRequest(ctx, reqURL, "POST").Response()
+	req := newInternalRequest(ctx, reqURL, "POST")
-	if err != nil {
+	_, extra := requestJSONResp(req, &responseText{})
-		return err
+	return extra.Error
-	}
-
-	defer resp.Body.Close()
-
-	// All 2XX status codes are accepted and others will return an error
-	if resp.StatusCode/100 != 2 {
-		return fmt.Errorf("Failed to update public key: %s", decodeJSONError(resp).Err)
-	}
-	return nil
 }
 
 // AuthorizedPublicKeyByContent searches content as prefix (leak e-mail part)
 // and returns public key found.
-func AuthorizedPublicKeyByContent(ctx context.Context, content string) (string, error) {
+func AuthorizedPublicKeyByContent(ctx context.Context, content string) (string, ResponseExtra) {
 	// Ask for running deliver hook and test pull request tasks.
 	reqURL := setting.LocalURL + "api/internal/ssh/authorized_keys"
 	req := newInternalRequest(ctx, reqURL, "POST")
 	req.Param("content", content)
-	resp, err := req.Response()
+	resp, extra := requestJSONResp(req, &responseText{})
-	if err != nil {
+	return resp.Text, extra
-		return "", err
-	}
-
-	defer resp.Body.Close()
-
-	// All 2XX status codes are accepted and others will return an error
-	if resp.StatusCode != http.StatusOK {
-		return "", fmt.Errorf("Failed to update public key: %s", decodeJSONError(resp).Err)
-	}
-	bs, err := io.ReadAll(resp.Body)
-
-	return string(bs), err
 }

modules/private/mail.go

@@ -5,11 +5,7 @@ package private
 
 import (
 	"context"
-	"fmt"
-	"io"
-	"net/http"
 
-	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/setting"
 )
 
@@ -21,38 +17,18 @@ type Email struct {
 }
 
 // SendEmail calls the internal SendEmail function
-//
 // It accepts a list of usernames.
 // If DB contains these users it will send the email to them.
-//
+// If to list == nil, it's supposed to send emails to every user present in DB
-// If to list == nil its supposed to send an email to every
+func SendEmail(ctx context.Context, subject, message string, to []string) (string, ResponseExtra) {
-// user present in DB
-func SendEmail(ctx context.Context, subject, message string, to []string) (int, string) {
 	reqURL := setting.LocalURL + "api/internal/mail/send"
 
-	req := newInternalRequest(ctx, reqURL, "POST")
+	req := newInternalRequest(ctx, reqURL, "POST", Email{
-	req = req.Header("Content-Type", "application/json")
-	jsonBytes, _ := json.Marshal(Email{
 		Subject: subject,
 		Message: message,
 		To:      to,
 	})
-	req.Body(jsonBytes)
-	resp, err := req.Response()
-	if err != nil {
-		return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error())
-	}
-	defer resp.Body.Close()
 
-	body, err := io.ReadAll(resp.Body)
+	resp, extra := requestJSONResp(req, &responseText{})
-	if err != nil {
+	return resp.Text, extra
-		return http.StatusInternalServerError, fmt.Sprintf("Response body error: %v", err.Error())
-	}
-
-	users := fmt.Sprintf("%d", len(to))
-	if len(to) == 0 {
-		users = "all"
-	}
-
-	return http.StatusOK, fmt.Sprintf("Sent %s email(s) to %s users", body, users)
 }

modules/private/manager.go

@@ -12,44 +12,21 @@ import (
 	"strconv"
 	"time"
 
-	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/setting"
 )
 
 // Shutdown calls the internal shutdown function
-func Shutdown(ctx context.Context) (int, string) {
+func Shutdown(ctx context.Context) ResponseExtra {
 	reqURL := setting.LocalURL + "api/internal/manager/shutdown"
-
 	req := newInternalRequest(ctx, reqURL, "POST")
-	resp, err := req.Response()
+	return requestJSONUserMsg(req, "Shutting down")
-	if err != nil {
-		return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error())
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		return resp.StatusCode, decodeJSONError(resp).Err
-	}
-
-	return http.StatusOK, "Shutting down"
 }
 
 // Restart calls the internal restart function
-func Restart(ctx context.Context) (int, string) {
+func Restart(ctx context.Context) ResponseExtra {
 	reqURL := setting.LocalURL + "api/internal/manager/restart"
-
 	req := newInternalRequest(ctx, reqURL, "POST")
-	resp, err := req.Response()
+	return requestJSONUserMsg(req, "Restarting")
-	if err != nil {
-		return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error())
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		return resp.StatusCode, decodeJSONError(resp).Err
-	}
-
-	return http.StatusOK, "Restarting"
 }
 
 // FlushOptions represents the options for the flush call
@@ -59,102 +36,41 @@ type FlushOptions struct {
 }
 
 // FlushQueues calls the internal flush-queues function
-func FlushQueues(ctx context.Context, timeout time.Duration, nonBlocking bool) (int, string) {
+func FlushQueues(ctx context.Context, timeout time.Duration, nonBlocking bool) ResponseExtra {
 	reqURL := setting.LocalURL + "api/internal/manager/flush-queues"
-
+	req := newInternalRequest(ctx, reqURL, "POST", FlushOptions{Timeout: timeout, NonBlocking: nonBlocking})
-	req := newInternalRequest(ctx, reqURL, "POST")
 	if timeout > 0 {
-		req.SetTimeout(timeout+10*time.Second, timeout+10*time.Second)
+		req.SetReadWriteTimeout(timeout + 10*time.Second)
 	}
-	req = req.Header("Content-Type", "application/json")
+	return requestJSONUserMsg(req, "Flushed")
-	jsonBytes, _ := json.Marshal(FlushOptions{
-		Timeout:     timeout,
-		NonBlocking: nonBlocking,
-	})
-	req.Body(jsonBytes)
-	resp, err := req.Response()
-	if err != nil {
-		return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error())
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		return resp.StatusCode, decodeJSONError(resp).Err
-	}
-
-	return http.StatusOK, "Flushed"
 }
 
 // PauseLogging pauses logging
-func PauseLogging(ctx context.Context) (int, string) {
+func PauseLogging(ctx context.Context) ResponseExtra {
 	reqURL := setting.LocalURL + "api/internal/manager/pause-logging"
-
 	req := newInternalRequest(ctx, reqURL, "POST")
-	resp, err := req.Response()
+	return requestJSONUserMsg(req, "Logging Paused")
-	if err != nil {
-		return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error())
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		return resp.StatusCode, decodeJSONError(resp).Err
-	}
-
-	return http.StatusOK, "Logging Paused"
 }
 
 // ResumeLogging resumes logging
-func ResumeLogging(ctx context.Context) (int, string) {
+func ResumeLogging(ctx context.Context) ResponseExtra {
 	reqURL := setting.LocalURL + "api/internal/manager/resume-logging"
-
 	req := newInternalRequest(ctx, reqURL, "POST")
-	resp, err := req.Response()
+	return requestJSONUserMsg(req, "Logging Restarted")
-	if err != nil {
-		return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error())
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		return resp.StatusCode, decodeJSONError(resp).Err
-	}
-
-	return http.StatusOK, "Logging Restarted"
 }
 
 // ReleaseReopenLogging releases and reopens logging files
-func ReleaseReopenLogging(ctx context.Context) (int, string) {
+func ReleaseReopenLogging(ctx context.Context) ResponseExtra {
 	reqURL := setting.LocalURL + "api/internal/manager/release-and-reopen-logging"
-
 	req := newInternalRequest(ctx, reqURL, "POST")
-	resp, err := req.Response()
+	return requestJSONUserMsg(req, "Logging Restarted")
-	if err != nil {
-		return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error())
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		return resp.StatusCode, decodeJSONError(resp).Err
-	}
-
-	return http.StatusOK, "Logging Restarted"
 }
 
 // SetLogSQL sets database logging
-func SetLogSQL(ctx context.Context, on bool) (int, string) {
+func SetLogSQL(ctx context.Context, on bool) ResponseExtra {
 	reqURL := setting.LocalURL + "api/internal/manager/set-log-sql?on=" + strconv.FormatBool(on)
-
 	req := newInternalRequest(ctx, reqURL, "POST")
-	resp, err := req.Response()
+	return requestJSONUserMsg(req, "Log SQL setting set")
-	if err != nil {
-		return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error())
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		return resp.StatusCode, decodeJSONError(resp).Err
-	}
-
-	return http.StatusOK, "Log SQL setting set"
 }
 
 // LoggerOptions represents the options for the add logger call
@@ -166,67 +82,32 @@ type LoggerOptions struct {
 }
 
 // AddLogger adds a logger
-func AddLogger(ctx context.Context, group, name, mode string, config map[string]interface{}) (int, string) {
+func AddLogger(ctx context.Context, group, name, mode string, config map[string]interface{}) ResponseExtra {
 	reqURL := setting.LocalURL + "api/internal/manager/add-logger"
-
+	req := newInternalRequest(ctx, reqURL, "POST", LoggerOptions{
-	req := newInternalRequest(ctx, reqURL, "POST")
-	req = req.Header("Content-Type", "application/json")
-	jsonBytes, _ := json.Marshal(LoggerOptions{
 		Group:  group,
 		Name:   name,
 		Mode:   mode,
 		Config: config,
 	})
-	req.Body(jsonBytes)
+	return requestJSONUserMsg(req, "Added")
-	resp, err := req.Response()
-	if err != nil {
-		return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error())
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		return resp.StatusCode, decodeJSONError(resp).Err
-	}
-
-	return http.StatusOK, "Added"
 }
 
 // RemoveLogger removes a logger
-func RemoveLogger(ctx context.Context, group, name string) (int, string) {
+func RemoveLogger(ctx context.Context, group, name string) ResponseExtra {
 	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/manager/remove-logger/%s/%s", url.PathEscape(group), url.PathEscape(name))
-
 	req := newInternalRequest(ctx, reqURL, "POST")
-	resp, err := req.Response()
+	return requestJSONUserMsg(req, "Removed")
-	if err != nil {
-		return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error())
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		return resp.StatusCode, decodeJSONError(resp).Err
-	}
-
-	return http.StatusOK, "Removed"
 }
 
 // Processes return the current processes from this gitea instance
-func Processes(ctx context.Context, out io.Writer, flat, noSystem, stacktraces, json bool, cancel string) (int, string) {
+func Processes(ctx context.Context, out io.Writer, flat, noSystem, stacktraces, json bool, cancel string) ResponseExtra {
 	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/manager/processes?flat=%t&no-system=%t&stacktraces=%t&json=%t&cancel-pid=%s", flat, noSystem, stacktraces, json, url.QueryEscape(cancel))
 
 	req := newInternalRequest(ctx, reqURL, "GET")
-	resp, err := req.Response()
+	callback := func(resp *http.Response, extra *ResponseExtra) {
-	if err != nil {
+		_, extra.Error = io.Copy(out, resp.Body)
-		return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error())
 	}
-	defer resp.Body.Close()
+	_, extra := requestJSONResp(req, &callback)
-
+	return extra
-	if resp.StatusCode != http.StatusOK {
-		return resp.StatusCode, decodeJSONError(resp).Err
-	}
-
-	_, err = io.Copy(out, resp.Body)
-	if err != nil {
-		return http.StatusInternalServerError, err.Error()
-	}
-	return http.StatusOK, ""
 }

modules/private/request.go

@@ -0,0 +1,135 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package private
+
+import (
+	"fmt"
+	"io"
+	"net/http"
+	"unicode"
+
+	"code.gitea.io/gitea/modules/httplib"
+	"code.gitea.io/gitea/modules/json"
+)
+
+// responseText is used to get the response as text, instead of parsing it as JSON.
+type responseText struct {
+	Text string
+}
+
+// ResponseExtra contains extra information about the response, especially for error responses.
+type ResponseExtra struct {
+	StatusCode int
+	UserMsg    string
+	Error      error
+}
+
+type responseCallback func(resp *http.Response, extra *ResponseExtra)
+
+func (re *ResponseExtra) HasError() bool {
+	return re.Error != nil
+}
+
+type responseError struct {
+	statusCode  int
+	errorString string
+}
+
+func (re responseError) Error() string {
+	if re.errorString == "" {
+		return fmt.Sprintf("internal API error response, status=%d", re.statusCode)
+	}
+	return fmt.Sprintf("internal API error response, status=%d, err=%s", re.statusCode, re.errorString)
+}
+
+// requestJSONUserMsg sends a request to the gitea server and then parses the response.
+// If the status code is not 2xx, or any error occurs, the ResponseExtra.Error field is guaranteed to be non-nil,
+// and the ResponseExtra.UserMsg field will be set to a message for the end user.
+//
+// * If the "res" is a struct pointer, the response will be parsed as JSON
+// * If the "res" is responseText pointer, the response will be stored as text in it
+// * If the "res" is responseCallback pointer, the callback function should set the ResponseExtra fields accordingly
+func requestJSONResp[T any](req *httplib.Request, res *T) (ret *T, extra ResponseExtra) {
+	resp, err := req.Response()
+	if err != nil {
+		extra.UserMsg = "Internal Server Connection Error"
+		extra.Error = fmt.Errorf("unable to contact gitea %q: %w", req.GoString(), err)
+		return nil, extra
+	}
+	defer resp.Body.Close()
+
+	extra.StatusCode = resp.StatusCode
+
+	// if the status code is not 2xx, try to parse the error response
+	if resp.StatusCode/100 != 2 {
+		var respErr Response
+		if err := json.NewDecoder(resp.Body).Decode(&respErr); err != nil {
+			extra.UserMsg = "Internal Server Error Decoding Failed"
+			extra.Error = fmt.Errorf("unable to decode error response %q: %w", req.GoString(), err)
+			return nil, extra
+		}
+		extra.UserMsg = respErr.UserMsg
+		if extra.UserMsg == "" {
+			extra.UserMsg = "Internal Server Error (no message for end users)"
+		}
+		extra.Error = responseError{statusCode: resp.StatusCode, errorString: respErr.Err}
+		return res, extra
+	}
+
+	// now, the StatusCode must be 2xx
+	var v any = res
+	if respText, ok := v.(*responseText); ok {
+		// get the whole response as a text string
+		bs, err := io.ReadAll(resp.Body)
+		if err != nil {
+			extra.UserMsg = "Internal Server Response Reading Failed"
+			extra.Error = fmt.Errorf("unable to read response %q: %w", req.GoString(), err)
+			return nil, extra
+		}
+		respText.Text = string(bs)
+		return res, extra
+	} else if callback, ok := v.(*responseCallback); ok {
+		// pass the response to callback, and let the callback update the ResponseExtra
+		extra.StatusCode = resp.StatusCode
+		(*callback)(resp, &extra)
+		return nil, extra
+	} else if err := json.NewDecoder(resp.Body).Decode(res); err != nil {
+		// decode the response into the given struct
+		extra.UserMsg = "Internal Server Response Decoding Failed"
+		extra.Error = fmt.Errorf("unable to decode response %q: %w", req.GoString(), err)
+		return nil, extra
+	}
+
+	if respMsg, ok := v.(*Response); ok {
+		// if the "res" is Response structure, try to get the UserMsg from it and update the ResponseExtra
+		extra.UserMsg = respMsg.UserMsg
+		if respMsg.Err != "" {
+			// usually this shouldn't happen, because the StatusCode is 2xx, there should be no error.
+			// but we still handle the "err" response, in case some people return error messages by status code 200.
+			extra.Error = responseError{statusCode: resp.StatusCode, errorString: respMsg.Err}
+		}
+	}
+
+	return res, extra
+}
+
+// requestJSONUserMsg sends a request to the gitea server and then parses the response as private.Response
+// If the request succeeds, the successMsg will be used as part of ResponseExtra.UserMsg.
+func requestJSONUserMsg(req *httplib.Request, successMsg string) ResponseExtra {
+	resp, extra := requestJSONResp(req, &Response{})
+	if extra.HasError() {
+		return extra
+	}
+	if resp.UserMsg == "" {
+		extra.UserMsg = successMsg // if UserMsg is empty, then use successMsg as userMsg
+	} else if successMsg != "" {
+		// else, now UserMsg is not empty, if successMsg is not empty, then append successMsg to UserMsg
+		if unicode.IsPunct(rune(extra.UserMsg[len(extra.UserMsg)-1])) {
+			extra.UserMsg = extra.UserMsg + " " + successMsg
+		} else {
+			extra.UserMsg = extra.UserMsg + ". " + successMsg
+		}
+	}
+	return extra
+}

modules/private/restore_repo.go

@@ -6,11 +6,8 @@ package private
 import (
 	"context"
 	"fmt"
-	"io"
-	"net/http"
 	"time"
 
-	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/setting"
 )
 
@@ -24,39 +21,16 @@ type RestoreParams struct {
 }
 
 // RestoreRepo calls the internal RestoreRepo function
-func RestoreRepo(ctx context.Context, repoDir, ownerName, repoName string, units []string, validation bool) (int, string) {
+func RestoreRepo(ctx context.Context, repoDir, ownerName, repoName string, units []string, validation bool) ResponseExtra {
 	reqURL := setting.LocalURL + "api/internal/restore_repo"
 
-	req := newInternalRequest(ctx, reqURL, "POST")
+	req := newInternalRequest(ctx, reqURL, "POST", RestoreParams{
-	req.SetTimeout(3*time.Second, 0) // since the request will spend much time, don't timeout
-	req = req.Header("Content-Type", "application/json")
-	jsonBytes, _ := json.Marshal(RestoreParams{
 		RepoDir:    repoDir,
 		OwnerName:  ownerName,
 		RepoName:   repoName,
 		Units:      units,
 		Validation: validation,
 	})
-	req.Body(jsonBytes)
+	req.SetTimeout(3*time.Second, 0) // since the request will spend much time, don't timeout
-	resp, err := req.Response()
+	return requestJSONUserMsg(req, fmt.Sprintf("Restore repo %s/%s successfully", ownerName, repoName))
-	if err != nil {
-		return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v, could you confirm it's running?", err.Error())
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		ret := struct {
-			Err string `json:"err"`
-		}{}
-		body, err := io.ReadAll(resp.Body)
-		if err != nil {
-			return http.StatusInternalServerError, fmt.Sprintf("Response body error: %v", err.Error())
-		}
-		if err := json.Unmarshal(body, &ret); err != nil {
-			return http.StatusInternalServerError, fmt.Sprintf("Response body Unmarshal error: %v", err.Error())
-		}
-		return http.StatusInternalServerError, ret.Err
-	}
-
-	return http.StatusOK, fmt.Sprintf("Restore repo %s/%s successfully", ownerName, repoName)
 }

modules/private/serv.go

@@ -6,13 +6,11 @@ package private
 import (
 	"context"
 	"fmt"
-	"net/http"
 	"net/url"
 
 	asymkey_model "code.gitea.io/gitea/models/asymkey"
 	"code.gitea.io/gitea/models/perm"
 	user_model "code.gitea.io/gitea/models/user"
-	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/setting"
 )
 
@@ -24,20 +22,11 @@ type KeyAndOwner struct {
 
 // ServNoCommand returns information about the provided key
 func ServNoCommand(ctx context.Context, keyID int64) (*asymkey_model.PublicKey, *user_model.User, error) {
-	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/serv/none/%d",
+	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/serv/none/%d", keyID)
-		keyID)
+	req := newInternalRequest(ctx, reqURL, "GET")
-	resp, err := newInternalRequest(ctx, reqURL, "GET").Response()
+	keyAndOwner, extra := requestJSONResp(req, &KeyAndOwner{})
-	if err != nil {
+	if extra.HasError() {
-		return nil, nil, err
+		return nil, nil, extra.Error
-	}
-	defer resp.Body.Close()
-	if resp.StatusCode != http.StatusOK {
-		return nil, nil, fmt.Errorf("%s", decodeJSONError(resp).Err)
-	}
-
-	var keyAndOwner KeyAndOwner
-	if err := json.NewDecoder(resp.Body).Decode(&keyAndOwner); err != nil {
-		return nil, nil, err
 	}
 	return keyAndOwner.Key, keyAndOwner.Owner, nil
 }
@@ -56,53 +45,19 @@ type ServCommandResults struct {
 	RepoID      int64
 }
 
-// ErrServCommand is an error returned from ServCommmand.
-type ErrServCommand struct {
-	Results    ServCommandResults
-	Err        string
-	StatusCode int
-}
-
-func (err ErrServCommand) Error() string {
-	return err.Err
-}
-
-// IsErrServCommand checks if an error is a ErrServCommand.
-func IsErrServCommand(err error) bool {
-	_, ok := err.(ErrServCommand)
-	return ok
-}
-
 // ServCommand preps for a serv call
-func ServCommand(ctx context.Context, keyID int64, ownerName, repoName string, mode perm.AccessMode, verbs ...string) (*ServCommandResults, error) {
+func ServCommand(ctx context.Context, keyID int64, ownerName, repoName string, mode perm.AccessMode, verbs ...string) (*ServCommandResults, ResponseExtra) {
 	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/serv/command/%d/%s/%s?mode=%d",
 		keyID,
 		url.PathEscape(ownerName),
 		url.PathEscape(repoName),
-		mode)
+		mode,
+	)
 	for _, verb := range verbs {
 		if verb != "" {
 			reqURL += fmt.Sprintf("&verb=%s", url.QueryEscape(verb))
 		}
 	}
-
+	req := newInternalRequest(ctx, reqURL, "GET")
-	resp, err := newInternalRequest(ctx, reqURL, "GET").Response()
+	return requestJSONResp(req, &ServCommandResults{})
-	if err != nil {
-		return nil, err
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		var errServCommand ErrServCommand
-		if err := json.NewDecoder(resp.Body).Decode(&errServCommand); err != nil {
-			return nil, err
-		}
-		errServCommand.StatusCode = resp.StatusCode
-		return nil, errServCommand
-	}
-	var results ServCommandResults
-	if err := json.NewDecoder(resp.Body).Decode(&results); err != nil {
-		return nil, err
-	}
-	return &results, nil
 }

routers/private/default_branch.go

@@ -1,7 +1,6 @@
 // Copyright 2021 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead.
 package private
 
 import (
@@ -14,19 +13,6 @@ import (
 	"code.gitea.io/gitea/modules/private"
 )
 
-// ________          _____             .__   __
-// \______ \   _____/ ____\____   __ __|  |_/  |_
-//  |    |  \_/ __ \   __\\__  \ |  |  \  |\   __\
-//  |    `   \  ___/|  |   / __ \|  |  /  |_|  |
-// /_______  /\___  >__|  (____  /____/|____/__|
-//         \/     \/           \/
-// __________                             .__
-// \______   \____________    ____   ____ |  |__
-//  |    |  _/\_  __ \__  \  /    \_/ ___\|  |  \
-//  |    |   \ |  | \// __ \|   |  \  \___|   Y  \
-//  |______  / |__|  (____  /___|  /\___  >___|  /
-//         \/             \/     \/     \/     \/
-
 // SetDefaultBranch updates the default branch
 func SetDefaultBranch(ctx *gitea_context.PrivateContext) {
 	ownerName := ctx.Params(":owner")

routers/private/hook_post_receive.go

@@ -1,7 +1,6 @@
 // Copyright 2021 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead.
 package private
 
 import (

routers/private/hook_pre_receive.go

@@ -1,7 +1,6 @@
 // Copyright 2019 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead.
 package private
 
 import (
@@ -69,8 +68,8 @@ func (ctx *preReceiveContext) AssertCanWriteCode() bool {
 		if ctx.Written() {
 			return false
 		}
-		ctx.JSON(http.StatusForbidden, map[string]interface{}{
+		ctx.JSON(http.StatusForbidden, private.Response{
-			"err": "User permission denied for writing.",
+			UserMsg: "User permission denied for writing.",
 		})
 		return false
 	}
@@ -95,8 +94,8 @@ func (ctx *preReceiveContext) AssertCreatePullRequest() bool {
 		if ctx.Written() {
 			return false
 		}
-		ctx.JSON(http.StatusForbidden, map[string]interface{}{
+		ctx.JSON(http.StatusForbidden, private.Response{
-			"err": "User permission denied for creating pull-request.",
+			UserMsg: "User permission denied for creating pull-request.",
 		})
 		return false
 	}
@@ -151,7 +150,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN
 	if branchName == repo.DefaultBranch && newCommitID == git.EmptySHA {
 		log.Warn("Forbidden: Branch: %s is the default branch in %-v and cannot be deleted", branchName, repo)
 		ctx.JSON(http.StatusForbidden, private.Response{
-			Err: fmt.Sprintf("branch %s is the default branch and cannot be deleted", branchName),
+			UserMsg: fmt.Sprintf("branch %s is the default branch and cannot be deleted", branchName),
 		})
 		return
 	}
@@ -179,7 +178,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN
 	if newCommitID == git.EmptySHA {
 		log.Warn("Forbidden: Branch: %s in %-v is protected from deletion", branchName, repo)
 		ctx.JSON(http.StatusForbidden, private.Response{
-			Err: fmt.Sprintf("branch %s is protected from deletion", branchName),
+			UserMsg: fmt.Sprintf("branch %s is protected from deletion", branchName),
 		})
 		return
 	}
@@ -196,7 +195,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN
 		} else if len(output) > 0 {
 			log.Warn("Forbidden: Branch: %s in %-v is protected from force push", branchName, repo)
 			ctx.JSON(http.StatusForbidden, private.Response{
-				Err: fmt.Sprintf("branch %s is protected from force push", branchName),
+				UserMsg: fmt.Sprintf("branch %s is protected from force push", branchName),
 			})
 			return
 
@@ -217,7 +216,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN
 			unverifiedCommit := err.(*errUnverifiedCommit).sha
 			log.Warn("Forbidden: Branch: %s in %-v is protected from unverified commit %s", branchName, repo, unverifiedCommit)
 			ctx.JSON(http.StatusForbidden, private.Response{
-				Err: fmt.Sprintf("branch %s is protected from unverified commit %s", branchName, unverifiedCommit),
+				UserMsg: fmt.Sprintf("branch %s is protected from unverified commit %s", branchName, unverifiedCommit),
 			})
 			return
 		}
@@ -272,7 +271,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN
 			if changedProtectedfiles {
 				log.Warn("Forbidden: Branch: %s in %-v is protected from changing file %s", branchName, repo, protectedFilePath)
 				ctx.JSON(http.StatusForbidden, private.Response{
-					Err: fmt.Sprintf("branch %s is protected from changing file %s", branchName, protectedFilePath),
+					UserMsg: fmt.Sprintf("branch %s is protected from changing file %s", branchName, protectedFilePath),
 				})
 				return
 			}
@@ -297,7 +296,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN
 			// Or we're simply not able to push to this protected branch
 			log.Warn("Forbidden: User %d is not allowed to push to protected branch: %s in %-v", ctx.opts.UserID, branchName, repo)
 			ctx.JSON(http.StatusForbidden, private.Response{
-				Err: fmt.Sprintf("Not allowed to push to protected branch %s", branchName),
+				UserMsg: fmt.Sprintf("Not allowed to push to protected branch %s", branchName),
 			})
 			return
 		}
@@ -333,7 +332,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN
 		if !allowedMerge {
 			log.Warn("Forbidden: User %d is not allowed to push to protected branch: %s in %-v and is not allowed to merge pr #%d", ctx.opts.UserID, branchName, repo, pr.Index)
 			ctx.JSON(http.StatusForbidden, private.Response{
-				Err: fmt.Sprintf("Not allowed to push to protected branch %s", branchName),
+				UserMsg: fmt.Sprintf("Not allowed to push to protected branch %s", branchName),
 			})
 			return
 		}
@@ -347,7 +346,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN
 		if changedProtectedfiles {
 			log.Warn("Forbidden: Branch: %s in %-v is protected from changing file %s", branchName, repo, protectedFilePath)
 			ctx.JSON(http.StatusForbidden, private.Response{
-				Err: fmt.Sprintf("branch %s is protected from changing file %s", branchName, protectedFilePath),
+				UserMsg: fmt.Sprintf("branch %s is protected from changing file %s", branchName, protectedFilePath),
 			})
 			return
 		}
@@ -357,7 +356,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN
 			if models.IsErrDisallowedToMerge(err) {
 				log.Warn("Forbidden: User %d is not allowed push to protected branch %s in %-v and pr #%d is not ready to be merged: %s", ctx.opts.UserID, branchName, repo, pr.Index, err.Error())
 				ctx.JSON(http.StatusForbidden, private.Response{
-					Err: fmt.Sprintf("Not allowed to push to protected branch %s and pr #%d is not ready to be merged: %s", branchName, ctx.opts.PullRequestID, err.Error()),
+					UserMsg: fmt.Sprintf("Not allowed to push to protected branch %s and pr #%d is not ready to be merged: %s", branchName, ctx.opts.PullRequestID, err.Error()),
 				})
 				return
 			}
@@ -400,7 +399,7 @@ func preReceiveTag(ctx *preReceiveContext, oldCommitID, newCommitID, refFullName
 	if !isAllowed {
 		log.Warn("Forbidden: Tag %s in %-v is protected", tagName, ctx.Repo.Repository)
 		ctx.JSON(http.StatusForbidden, private.Response{
-			Err: fmt.Sprintf("Tag %s is protected", tagName),
+			UserMsg: fmt.Sprintf("Tag %s is protected", tagName),
 		})
 		return
 	}
@@ -412,15 +411,15 @@ func preReceivePullRequest(ctx *preReceiveContext, oldCommitID, newCommitID, ref
 	}
 
 	if ctx.Repo.Repository.IsEmpty {
-		ctx.JSON(http.StatusForbidden, map[string]interface{}{
+		ctx.JSON(http.StatusForbidden, private.Response{
-			"err": "Can't create pull request for an empty repository.",
+			UserMsg: "Can't create pull request for an empty repository.",
 		})
 		return
 	}
 
 	if ctx.opts.IsWiki {
-		ctx.JSON(http.StatusForbidden, map[string]interface{}{
+		ctx.JSON(http.StatusForbidden, private.Response{
-			"err": "Pull requests are not supported on the wiki.",
+			UserMsg: "Pull requests are not supported on the wiki.",
 		})
 		return
 	}
@@ -443,7 +442,7 @@ func preReceivePullRequest(ctx *preReceiveContext, oldCommitID, newCommitID, ref
 
 	if !baseBranchExist {
 		ctx.JSON(http.StatusForbidden, private.Response{
-			Err: fmt.Sprintf("Unexpected ref: %s", refFullName),
+			UserMsg: fmt.Sprintf("Unexpected ref: %s", refFullName),
 		})
 		return
 	}

routers/private/hook_proc_receive.go

@@ -1,7 +1,6 @@
 // Copyright 2021 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead.
 package private
 
 import (
@@ -30,8 +29,8 @@ func HookProcReceive(ctx *gitea_context.PrivateContext) {
 			ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error())
 		} else {
 			log.Error(err.Error())
-			ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
+			ctx.JSON(http.StatusInternalServerError, private.Response{
-				"Err": err.Error(),
+				Err: err.Error(),
 			})
 		}
 

routers/private/hook_verification.go

@@ -1,7 +1,6 @@
 // Copyright 2021 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead.
 package private
 
 import (
@@ -16,19 +15,6 @@ import (
 	"code.gitea.io/gitea/modules/log"
 )
 
-// _________                        .__  __
-// \_   ___ \  ____   _____   _____ |__|/  |_
-// /    \  \/ /  _ \ /     \ /     \|  \   __\
-// \     \___(  <_> )  Y Y  \  Y Y  \  ||  |
-//  \______  /\____/|__|_|  /__|_|  /__||__|
-//         \/             \/      \/
-// ____   ____           .__  _____.__               __  .__
-// \   \ /   /___________|__|/ ____\__| ____ _____ _/  |_|__| ____   ____
-//  \   Y   // __ \_  __ \  \   __\|  |/ ___\\__  \\   __\  |/  _ \ /    \
-//   \     /\  ___/|  | \/  ||  |  |  \  \___ / __ \|  | |  (  <_> )   |  \
-//    \___/  \___  >__|  |__||__|  |__|\___  >____  /__| |__|\____/|___|  /
-//               \/                        \/     \/                    \/
-//
 // This file contains commit verification functions for refs passed across in hooks
 
 func verifyCommits(oldCommitID, newCommitID string, repo *git.Repository, env []string) error {

routers/private/internal.go

@@ -1,7 +1,7 @@
 // Copyright 2017 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead.
+// Package private contains all internal routes. The package name "internal" isn't usable because Golang reserves it for disabling cross-package usage.
 package private
 
 import (

routers/private/internal_repo.go

@@ -1,7 +1,6 @@
 // Copyright 2021 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead.
 package private
 
 import (
@@ -13,23 +12,10 @@ import (
 	gitea_context "code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/private"
 )
 
-// __________
+// This file contains common functions relating to setting the Repository for the internal routes
-// \______   \ ____ ______   ____
-//  |       _// __ \\____ \ /  _ \
-//  |    |   \  ___/|  |_> >  <_> )
-//  |____|_  /\___  >   __/ \____/
-//         \/     \/|__|
-//    _____                .__                                     __
-//   /  _  \   ______ _____|__| ____   ____   _____   ____   _____/  |_
-//  /  /_\  \ /  ___//  ___/  |/ ___\ /    \ /     \_/ __ \ /    \   __\
-// /    |    \\___ \ \___ \|  / /_/  >   |  \  Y Y  \  ___/|   |  \  |
-// \____|__  /____  >____  >__\___  /|___|  /__|_|  /\___  >___|  /__|
-//         \/     \/     \/  /_____/      \/      \/     \/     \/
-
-// This file contains common functions relating to setting the Repository for the
-// internal routes
 
 // RepoAssignment assigns the repository and gitrepository to the private context
 func RepoAssignment(ctx *gitea_context.PrivateContext) context.CancelFunc {
@@ -45,8 +31,8 @@ func RepoAssignment(ctx *gitea_context.PrivateContext) context.CancelFunc {
 	gitRepo, err := git.OpenRepository(ctx, repo.RepoPath())
 	if err != nil {
 		log.Error("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err)
-		ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
+		ctx.JSON(http.StatusInternalServerError, private.Response{
-			"Err": fmt.Sprintf("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err),
+			Err: fmt.Sprintf("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err),
 		})
 		return nil
 	}
@@ -71,8 +57,8 @@ func loadRepository(ctx *gitea_context.PrivateContext, ownerName, repoName strin
 	repo, err := repo_model.GetRepositoryByOwnerAndName(ctx, ownerName, repoName)
 	if err != nil {
 		log.Error("Failed to get repository: %s/%s Error: %v", ownerName, repoName, err)
-		ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
+		ctx.JSON(http.StatusInternalServerError, private.Response{
-			"Err": fmt.Sprintf("Failed to get repository: %s/%s Error: %v", ownerName, repoName, err),
+			Err: fmt.Sprintf("Failed to get repository: %s/%s Error: %v", ownerName, repoName, err),
 		})
 		return nil
 	}

routers/private/key.go

@@ -1,7 +1,6 @@
 // Copyright 2018 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead.
 package private
 
 import (

routers/private/manager.go

@@ -31,14 +31,14 @@ func FlushQueues(ctx *context.PrivateContext) {
 			}
 		}()
 		ctx.JSON(http.StatusAccepted, private.Response{
-			Err: "Flushing",
+			UserMsg: "Flushing",
 		})
 		return
 	}
 	err := queue.GetManager().FlushAll(ctx, opts.Timeout)
 	if err != nil {
 		ctx.JSON(http.StatusRequestTimeout, private.Response{
-			Err: fmt.Sprintf("%v", err),
+			UserMsg: fmt.Sprintf("%v", err),
 		})
 	}
 	ctx.PlainText(http.StatusOK, "success")

routers/private/manager_windows.go

@@ -16,7 +16,7 @@ import (
 // Restart is not implemented for Windows based servers as they can't fork
 func Restart(ctx *context.PrivateContext) {
 	ctx.JSON(http.StatusNotImplemented, private.Response{
-		Err: "windows servers cannot be gracefully restarted - shutdown and restart manually",
+		UserMsg: "windows servers cannot be gracefully restarted - shutdown and restart manually",
 	})
 }
 

routers/private/serv.go

@@ -1,7 +1,6 @@
 // Copyright 2019 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead.
 package private
 
 import (
@@ -29,7 +28,7 @@ func ServNoCommand(ctx *context.PrivateContext) {
 	keyID := ctx.ParamsInt64(":keyid")
 	if keyID <= 0 {
 		ctx.JSON(http.StatusBadRequest, private.Response{
-			Err: fmt.Sprintf("Bad key id: %d", keyID),
+			UserMsg: fmt.Sprintf("Bad key id: %d", keyID),
 		})
 	}
 	results := private.KeyAndOwner{}
@@ -38,7 +37,7 @@ func ServNoCommand(ctx *context.PrivateContext) {
 	if err != nil {
 		if asymkey_model.IsErrKeyNotExist(err) {
 			ctx.JSON(http.StatusUnauthorized, private.Response{
-				Err: fmt.Sprintf("Cannot find key: %d", keyID),
+				UserMsg: fmt.Sprintf("Cannot find key: %d", keyID),
 			})
 			return
 		}
@@ -55,7 +54,7 @@ func ServNoCommand(ctx *context.PrivateContext) {
 		if err != nil {
 			if user_model.IsErrUserNotExist(err) {
 				ctx.JSON(http.StatusUnauthorized, private.Response{
-					Err: fmt.Sprintf("Cannot find owner with id: %d for key: %d", key.OwnerID, keyID),
+					UserMsg: fmt.Sprintf("Cannot find owner with id: %d for key: %d", key.OwnerID, keyID),
 				})
 				return
 			}
@@ -67,7 +66,7 @@ func ServNoCommand(ctx *context.PrivateContext) {
 		}
 		if !user.IsActive || user.ProhibitLogin {
 			ctx.JSON(http.StatusForbidden, private.Response{
-				Err: "Your account is disabled.",
+				UserMsg: "Your account is disabled.",
 			})
 			return
 		}
@@ -113,23 +112,20 @@ func ServCommand(ctx *context.PrivateContext) {
 		if user_model.IsErrUserNotExist(err) {
 			// User is fetching/cloning a non-existent repository
 			log.Warn("Failed authentication attempt (cannot find repository: %s/%s) from %s", results.OwnerName, results.RepoName, ctx.RemoteAddr())
-			ctx.JSON(http.StatusNotFound, private.ErrServCommand{
+			ctx.JSON(http.StatusNotFound, private.Response{
-				Results: results,
+				UserMsg: fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName),
-				Err:     fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName),
 			})
 			return
 		}
 		log.Error("Unable to get repository owner: %s/%s Error: %v", results.OwnerName, results.RepoName, err)
-		ctx.JSON(http.StatusForbidden, private.ErrServCommand{
+		ctx.JSON(http.StatusForbidden, private.Response{
-			Results: results,
+			UserMsg: fmt.Sprintf("Unable to get repository owner: %s/%s %v", results.OwnerName, results.RepoName, err),
-			Err:     fmt.Sprintf("Unable to get repository owner: %s/%s %v", results.OwnerName, results.RepoName, err),
 		})
 		return
 	}
 	if !owner.IsOrganization() && !owner.IsActive {
-		ctx.JSON(http.StatusForbidden, private.ErrServCommand{
+		ctx.JSON(http.StatusForbidden, private.Response{
-			Results: results,
+			UserMsg: "Repository cannot be accessed, you could retry it later",
-			Err:     "Repository cannot be accessed, you could retry it later",
 		})
 		return
 	}
@@ -144,18 +140,16 @@ func ServCommand(ctx *context.PrivateContext) {
 				if verb == "git-upload-pack" {
 					// User is fetching/cloning a non-existent repository
 					log.Warn("Failed authentication attempt (cannot find repository: %s/%s) from %s", results.OwnerName, results.RepoName, ctx.RemoteAddr())
-					ctx.JSON(http.StatusNotFound, private.ErrServCommand{
+					ctx.JSON(http.StatusNotFound, private.Response{
-						Results: results,
+						UserMsg: fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName),
-						Err:     fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName),
 					})
 					return
 				}
 			}
 		} else {
 			log.Error("Unable to get repository: %s/%s Error: %v", results.OwnerName, results.RepoName, err)
-			ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{
+			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Results: results,
+				Err: fmt.Sprintf("Unable to get repository: %s/%s %v", results.OwnerName, results.RepoName, err),
-				Err:     fmt.Sprintf("Unable to get repository: %s/%s %v", results.OwnerName, results.RepoName, err),
 			})
 			return
 		}
@@ -167,26 +161,23 @@ func ServCommand(ctx *context.PrivateContext) {
 		results.RepoID = repo.ID
 
 		if repo.IsBeingCreated() {
-			ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{
+			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Results: results,
+				Err: "Repository is being created, you could retry after it finished",
-				Err:     "Repository is being created, you could retry after it finished",
 			})
 			return
 		}
 
 		if repo.IsBroken() {
-			ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{
+			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Results: results,
+				Err: "Repository is in a broken state",
-				Err:     "Repository is in a broken state",
 			})
 			return
 		}
 
 		// We can shortcut at this point if the repo is a mirror
 		if mode > perm.AccessModeRead && repo.IsMirror {
-			ctx.JSON(http.StatusForbidden, private.ErrServCommand{
+			ctx.JSON(http.StatusForbidden, private.Response{
-				Results: results,
+				UserMsg: fmt.Sprintf("Mirror Repository %s/%s is read-only", results.OwnerName, results.RepoName),
-				Err:     fmt.Sprintf("Mirror Repository %s/%s is read-only", results.OwnerName, results.RepoName),
 			})
 			return
 		}
@@ -196,16 +187,14 @@ func ServCommand(ctx *context.PrivateContext) {
 	key, err := asymkey_model.GetPublicKeyByID(keyID)
 	if err != nil {
 		if asymkey_model.IsErrKeyNotExist(err) {
-			ctx.JSON(http.StatusNotFound, private.ErrServCommand{
+			ctx.JSON(http.StatusNotFound, private.Response{
-				Results: results,
+				UserMsg: fmt.Sprintf("Cannot find key: %d", keyID),
-				Err:     fmt.Sprintf("Cannot find key: %d", keyID),
 			})
 			return
 		}
 		log.Error("Unable to get public key: %d Error: %v", keyID, err)
-		ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{
+		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Results: results,
+			Err: fmt.Sprintf("Unable to get key: %d  Error: %v", keyID, err),
-			Err:     fmt.Sprintf("Unable to get key: %d  Error: %v", keyID, err),
 		})
 		return
 	}
@@ -215,9 +204,8 @@ func ServCommand(ctx *context.PrivateContext) {
 
 	// If repo doesn't exist, deploy key doesn't make sense
 	if !repoExist && key.Type == asymkey_model.KeyTypeDeploy {
-		ctx.JSON(http.StatusNotFound, private.ErrServCommand{
+		ctx.JSON(http.StatusNotFound, private.Response{
-			Results: results,
+			UserMsg: fmt.Sprintf("Cannot find repository %s/%s", results.OwnerName, results.RepoName),
-			Err:     fmt.Sprintf("Cannot find repository %s/%s", results.OwnerName, results.RepoName),
 		})
 		return
 	}
@@ -232,16 +220,14 @@ func ServCommand(ctx *context.PrivateContext) {
 		deployKey, err = asymkey_model.GetDeployKeyByRepo(ctx, key.ID, repo.ID)
 		if err != nil {
 			if asymkey_model.IsErrDeployKeyNotExist(err) {
-				ctx.JSON(http.StatusNotFound, private.ErrServCommand{
+				ctx.JSON(http.StatusNotFound, private.Response{
-					Results: results,
+					UserMsg: fmt.Sprintf("Public (Deploy) Key: %d:%s is not authorized to %s %s/%s.", key.ID, key.Name, modeString, results.OwnerName, results.RepoName),
-					Err:     fmt.Sprintf("Public (Deploy) Key: %d:%s is not authorized to %s %s/%s.", key.ID, key.Name, modeString, results.OwnerName, results.RepoName),
 				})
 				return
 			}
 			log.Error("Unable to get deploy for public (deploy) key: %d in %-v Error: %v", key.ID, repo, err)
-			ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{
+			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Results: results,
+				Err: fmt.Sprintf("Unable to get Deploy Key for Public Key: %d:%s in %s/%s.", key.ID, key.Name, results.OwnerName, results.RepoName),
-				Err:     fmt.Sprintf("Unable to get Deploy Key for Public Key: %d:%s in %s/%s.", key.ID, key.Name, results.OwnerName, results.RepoName),
 			})
 			return
 		}
@@ -262,23 +248,21 @@ func ServCommand(ctx *context.PrivateContext) {
 		user, err = user_model.GetUserByID(ctx, key.OwnerID)
 		if err != nil {
 			if user_model.IsErrUserNotExist(err) {
-				ctx.JSON(http.StatusUnauthorized, private.ErrServCommand{
+				ctx.JSON(http.StatusUnauthorized, private.Response{
-					Results: results,
+					UserMsg: fmt.Sprintf("Public Key: %d:%s owner %d does not exist.", key.ID, key.Name, key.OwnerID),
-					Err:     fmt.Sprintf("Public Key: %d:%s owner %d does not exist.", key.ID, key.Name, key.OwnerID),
 				})
 				return
 			}
 			log.Error("Unable to get owner: %d for public key: %d:%s Error: %v", key.OwnerID, key.ID, key.Name, err)
-			ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{
+			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Results: results,
+				Err: fmt.Sprintf("Unable to get Owner: %d for Deploy Key: %d:%s in %s/%s.", key.OwnerID, key.ID, key.Name, ownerName, repoName),
-				Err:     fmt.Sprintf("Unable to get Owner: %d for Deploy Key: %d:%s in %s/%s.", key.OwnerID, key.ID, key.Name, ownerName, repoName),
 			})
 			return
 		}
 
 		if !user.IsActive || user.ProhibitLogin {
 			ctx.JSON(http.StatusForbidden, private.Response{
-				Err: "Your account is disabled.",
+				UserMsg: "Your account is disabled.",
 			})
 			return
 		}
@@ -291,9 +275,8 @@ func ServCommand(ctx *context.PrivateContext) {
 
 	// Don't allow pushing if the repo is archived
 	if repoExist && mode > perm.AccessModeRead && repo.IsArchived {
-		ctx.JSON(http.StatusUnauthorized, private.ErrServCommand{
+		ctx.JSON(http.StatusUnauthorized, private.Response{
-			Results: results,
+			UserMsg: fmt.Sprintf("Repo: %s/%s is archived.", results.OwnerName, results.RepoName),
-			Err:     fmt.Sprintf("Repo: %s/%s is archived.", results.OwnerName, results.RepoName),
 		})
 		return
 	}
@@ -307,9 +290,8 @@ func ServCommand(ctx *context.PrivateContext) {
 			setting.Service.RequireSignInView) {
 		if key.Type == asymkey_model.KeyTypeDeploy {
 			if deployKey.Mode < mode {
-				ctx.JSON(http.StatusUnauthorized, private.ErrServCommand{
+				ctx.JSON(http.StatusUnauthorized, private.Response{
-					Results: results,
+					UserMsg: fmt.Sprintf("Deploy Key: %d:%s is not authorized to %s %s/%s.", key.ID, key.Name, modeString, results.OwnerName, results.RepoName),
-					Err:     fmt.Sprintf("Deploy Key: %d:%s is not authorized to %s %s/%s.", key.ID, key.Name, modeString, results.OwnerName, results.RepoName),
 				})
 				return
 			}
@@ -322,9 +304,8 @@ func ServCommand(ctx *context.PrivateContext) {
 			perm, err := access_model.GetUserRepoPermission(ctx, repo, user)
 			if err != nil {
 				log.Error("Unable to get permissions for %-v with key %d in %-v Error: %v", user, key.ID, repo, err)
-				ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{
+				ctx.JSON(http.StatusInternalServerError, private.Response{
-					Results: results,
+					Err: fmt.Sprintf("Unable to get permissions for user %d:%s with key %d in %s/%s Error: %v", user.ID, user.Name, key.ID, results.OwnerName, results.RepoName, err),
-					Err:     fmt.Sprintf("Unable to get permissions for user %d:%s with key %d in %s/%s Error: %v", user.ID, user.Name, key.ID, results.OwnerName, results.RepoName, err),
 				})
 				return
 			}
@@ -333,9 +314,8 @@ func ServCommand(ctx *context.PrivateContext) {
 
 			if userMode < mode {
 				log.Warn("Failed authentication attempt for %s with key %s (not authorized to %s %s/%s) from %s", user.Name, key.Name, modeString, ownerName, repoName, ctx.RemoteAddr())
-				ctx.JSON(http.StatusUnauthorized, private.ErrServCommand{
+				ctx.JSON(http.StatusUnauthorized, private.Response{
-					Results: results,
+					UserMsg: fmt.Sprintf("User: %d:%s with Key: %d:%s is not authorized to %s %s/%s.", user.ID, user.Name, key.ID, key.Name, modeString, ownerName, repoName),
-					Err:     fmt.Sprintf("User: %d:%s with Key: %d:%s is not authorized to %s %s/%s.", user.ID, user.Name, key.ID, key.Name, modeString, ownerName, repoName),
 				})
 				return
 			}
@@ -346,24 +326,21 @@ func ServCommand(ctx *context.PrivateContext) {
 	if !repoExist {
 		owner, err := user_model.GetUserByName(ctx, ownerName)
 		if err != nil {
-			ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{
+			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Results: results,
+				Err: fmt.Sprintf("Unable to get owner: %s %v", results.OwnerName, err),
-				Err:     fmt.Sprintf("Unable to get owner: %s %v", results.OwnerName, err),
 			})
 			return
 		}
 
 		if owner.IsOrganization() && !setting.Repository.EnablePushCreateOrg {
-			ctx.JSON(http.StatusForbidden, private.ErrServCommand{
+			ctx.JSON(http.StatusForbidden, private.Response{
-				Results: results,
+				UserMsg: "Push to create is not enabled for organizations.",
-				Err:     "Push to create is not enabled for organizations.",
 			})
 			return
 		}
 		if !owner.IsOrganization() && !setting.Repository.EnablePushCreateUser {
-			ctx.JSON(http.StatusForbidden, private.ErrServCommand{
+			ctx.JSON(http.StatusForbidden, private.Response{
-				Results: results,
+				UserMsg: "Push to create is not enabled for users.",
-				Err:     "Push to create is not enabled for users.",
 			})
 			return
 		}
@@ -371,9 +348,8 @@ func ServCommand(ctx *context.PrivateContext) {
 		repo, err = repo_service.PushCreateRepo(ctx, user, owner, results.RepoName)
 		if err != nil {
 			log.Error("pushCreateRepo: %v", err)
-			ctx.JSON(http.StatusNotFound, private.ErrServCommand{
+			ctx.JSON(http.StatusNotFound, private.Response{
-				Results: results,
+				UserMsg: fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName),
-				Err:     fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName),
 			})
 			return
 		}
@@ -384,16 +360,14 @@ func ServCommand(ctx *context.PrivateContext) {
 		// Ensure the wiki is enabled before we allow access to it
 		if _, err := repo.GetUnit(ctx, unit.TypeWiki); err != nil {
 			if repo_model.IsErrUnitTypeNotExist(err) {
-				ctx.JSON(http.StatusForbidden, private.ErrServCommand{
+				ctx.JSON(http.StatusForbidden, private.Response{
-					Results: results,
+					UserMsg: "repository wiki is disabled",
-					Err:     "repository wiki is disabled",
 				})
 				return
 			}
 			log.Error("Failed to get the wiki unit in %-v Error: %v", repo, err)
-			ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{
+			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Results: results,
+				Err: fmt.Sprintf("Failed to get the wiki unit in %s/%s Error: %v", ownerName, repoName, err),
-				Err:     fmt.Sprintf("Failed to get the wiki unit in %s/%s Error: %v", ownerName, repoName, err),
 			})
 			return
 		}
@@ -401,9 +375,8 @@ func ServCommand(ctx *context.PrivateContext) {
 		// Finally if we're trying to touch the wiki we should init it
 		if err = wiki_service.InitWiki(ctx, repo); err != nil {
 			log.Error("Failed to initialize the wiki in %-v Error: %v", repo, err)
-			ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{
+			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Results: results,
+				Err: fmt.Sprintf("Failed to initialize the wiki in %s/%s Error: %v", ownerName, repoName, err),
-				Err:     fmt.Sprintf("Failed to initialize the wiki in %s/%s Error: %v", ownerName, repoName, err),
 			})
 			return
 		}

tests/integration/api_private_serv_test.go

@@ -43,8 +43,8 @@ func TestAPIPrivateServ(t *testing.T) {
 		defer cancel()
 
 		// Can push to a repo we own
-		results, err := private.ServCommand(ctx, 1, "user2", "repo1", perm.AccessModeWrite, "git-upload-pack", "")
+		results, extra := private.ServCommand(ctx, 1, "user2", "repo1", perm.AccessModeWrite, "git-upload-pack", "")
-		assert.NoError(t, err)
+		assert.NoError(t, extra.Error)
 		assert.False(t, results.IsWiki)
 		assert.Zero(t, results.DeployKeyID)
 		assert.Equal(t, int64(1), results.KeyID)
@@ -56,18 +56,18 @@ func TestAPIPrivateServ(t *testing.T) {
 		assert.Equal(t, int64(1), results.RepoID)
 
 		// Cannot push to a private repo we're not associated with
-		results, err = private.ServCommand(ctx, 1, "user15", "big_test_private_1", perm.AccessModeWrite, "git-upload-pack", "")
+		results, extra = private.ServCommand(ctx, 1, "user15", "big_test_private_1", perm.AccessModeWrite, "git-upload-pack", "")
-		assert.Error(t, err)
+		assert.Error(t, extra.Error)
 		assert.Empty(t, results)
 
 		// Cannot pull from a private repo we're not associated with
-		results, err = private.ServCommand(ctx, 1, "user15", "big_test_private_1", perm.AccessModeRead, "git-upload-pack", "")
+		results, extra = private.ServCommand(ctx, 1, "user15", "big_test_private_1", perm.AccessModeRead, "git-upload-pack", "")
-		assert.Error(t, err)
+		assert.Error(t, extra.Error)
 		assert.Empty(t, results)
 
 		// Can pull from a public repo we're not associated with
-		results, err = private.ServCommand(ctx, 1, "user15", "big_test_public_1", perm.AccessModeRead, "git-upload-pack", "")
+		results, extra = private.ServCommand(ctx, 1, "user15", "big_test_public_1", perm.AccessModeRead, "git-upload-pack", "")
-		assert.NoError(t, err)
+		assert.NoError(t, extra.Error)
 		assert.False(t, results.IsWiki)
 		assert.Zero(t, results.DeployKeyID)
 		assert.Equal(t, int64(1), results.KeyID)
@@ -79,8 +79,8 @@ func TestAPIPrivateServ(t *testing.T) {
 		assert.Equal(t, int64(17), results.RepoID)
 
 		// Cannot push to a public repo we're not associated with
-		results, err = private.ServCommand(ctx, 1, "user15", "big_test_public_1", perm.AccessModeWrite, "git-upload-pack", "")
+		results, extra = private.ServCommand(ctx, 1, "user15", "big_test_public_1", perm.AccessModeWrite, "git-upload-pack", "")
-		assert.Error(t, err)
+		assert.Error(t, extra.Error)
 		assert.Empty(t, results)
 
 		// Add reading deploy key
@@ -88,8 +88,8 @@ func TestAPIPrivateServ(t *testing.T) {
 		assert.NoError(t, err)
 
 		// Can pull from repo we're a deploy key for
-		results, err = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_1", perm.AccessModeRead, "git-upload-pack", "")
+		results, extra = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_1", perm.AccessModeRead, "git-upload-pack", "")
-		assert.NoError(t, err)
+		assert.NoError(t, extra.Error)
 		assert.False(t, results.IsWiki)
 		assert.NotZero(t, results.DeployKeyID)
 		assert.Equal(t, deployKey.KeyID, results.KeyID)
@@ -101,18 +101,18 @@ func TestAPIPrivateServ(t *testing.T) {
 		assert.Equal(t, int64(19), results.RepoID)
 
 		// Cannot push to a private repo with reading key
-		results, err = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_1", perm.AccessModeWrite, "git-upload-pack", "")
+		results, extra = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_1", perm.AccessModeWrite, "git-upload-pack", "")
-		assert.Error(t, err)
+		assert.Error(t, extra.Error)
 		assert.Empty(t, results)
 
 		// Cannot pull from a private repo we're not associated with
-		results, err = private.ServCommand(ctx, deployKey.ID, "user15", "big_test_private_2", perm.AccessModeRead, "git-upload-pack", "")
+		results, extra = private.ServCommand(ctx, deployKey.ID, "user15", "big_test_private_2", perm.AccessModeRead, "git-upload-pack", "")
-		assert.Error(t, err)
+		assert.Error(t, extra.Error)
 		assert.Empty(t, results)
 
 		// Cannot pull from a public repo we're not associated with
-		results, err = private.ServCommand(ctx, deployKey.ID, "user15", "big_test_public_1", perm.AccessModeRead, "git-upload-pack", "")
+		results, extra = private.ServCommand(ctx, deployKey.ID, "user15", "big_test_public_1", perm.AccessModeRead, "git-upload-pack", "")
-		assert.Error(t, err)
+		assert.Error(t, extra.Error)
 		assert.Empty(t, results)
 
 		// Add writing deploy key
@@ -120,13 +120,13 @@ func TestAPIPrivateServ(t *testing.T) {
 		assert.NoError(t, err)
 
 		// Cannot push to a private repo with reading key
-		results, err = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_1", perm.AccessModeWrite, "git-upload-pack", "")
+		results, extra = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_1", perm.AccessModeWrite, "git-upload-pack", "")
-		assert.Error(t, err)
+		assert.Error(t, extra.Error)
 		assert.Empty(t, results)
 
 		// Can pull from repo we're a writing deploy key for
-		results, err = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_2", perm.AccessModeRead, "git-upload-pack", "")
+		results, extra = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_2", perm.AccessModeRead, "git-upload-pack", "")
-		assert.NoError(t, err)
+		assert.NoError(t, extra.Error)
 		assert.False(t, results.IsWiki)
 		assert.NotZero(t, results.DeployKeyID)
 		assert.Equal(t, deployKey.KeyID, results.KeyID)
@@ -138,8 +138,8 @@ func TestAPIPrivateServ(t *testing.T) {
 		assert.Equal(t, int64(20), results.RepoID)
 
 		// Can push to repo we're a writing deploy key for
-		results, err = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_2", perm.AccessModeWrite, "git-upload-pack", "")
+		results, extra = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_2", perm.AccessModeWrite, "git-upload-pack", "")
-		assert.NoError(t, err)
+		assert.NoError(t, extra.Error)
 		assert.False(t, results.IsWiki)
 		assert.NotZero(t, results.DeployKeyID)
 		assert.Equal(t, deployKey.KeyID, results.KeyID)