Repository: reviewdog/reviewdog (go)

Issue:
github-pr-check fails to complete if there are too many findings
Updating the check run fails with the following error message:

```
UpdateCheckRun failed: {"message":"Invalid request. Only 65535 characters are allowed; 66618 were supplied.","documentation_url":"https://docs.github.com/rest/reference/checks#update-a-check-run"}
```

At some point there was an attempt to fix this in fe3996e1bd87c07ce52a705453e517c9e1153531, but it seems like the change was not sufficient.

Currently, the summary is limited to 150 findings, but if the text for the individual findings is very long, this can still be too much.
https://github.com/reviewdog/reviewdog/blob/abcd1e0f45b73cd4d5c624b5e64870556d8631b0/doghouse/server/doghouse.go#L20-L26
https://github.com/reviewdog/reviewdog/blob/abcd1e0f45b73cd4d5c624b5e64870556d8631b0/doghouse/server/doghouse.go#L216-L231
Maybe a better approach would be to stop appending after the byte threshold is reached:

```golang
func (ch *Checker) summaryFindings(name string, checks []*filter.FilteredDiagnostic) []string {
	var lines []string
	lines = append(lines, "<details>")
	lines = append(lines, fmt.Sprintf("<summary>%s (%d)</summary>", name, len(checks)))
	lines = append(lines, "")
	for _, c := range checks {
		nextLine := githubutils.LinkedMarkdownDiagnostic(ch.req.Owner, ch.req.Repo, ch.req.SHA, c.Diagnostic)
		if len(lines) + len(nextLine) >= 65535 {
			lines = append(lines, "... (Too many findings. Dropped some findings)")
			break
		}
		lines = append(lines, nextLine)
	}
	lines = append(lines, "</details>")
	return lines
}
``` 

I think it's also worth considering, if the `text` field of the output wouldn't be a better place for all the findings. In the example from the GitHub docs[^1], the summary is much shorter.

```json
  "output": {
    "title": "Mighty Readme report",
    "summary": "There are 0 failures, 2 warnings, and 1 notice.",
    "text": "You may have some misspelled words on lines 2 and 4. You also may want to add a section in your README about how to install your app.",
    "annotations_count": 2,
    "annotations_url": "https://api.github.com/repos/github/hello-world/check-runs/4/annotations"
  },
```

[^1]: https://docs.github.com/en/rest/checks/runs#create-a-check-run

Pull request:
Enforce size limit for check-run summary
closes #1207



Patch:
diff --git a/doghouse/server/doghouse.go b/doghouse/server/doghouse.go
index c85479a8..aa43a20f 100644
--- a/doghouse/server/doghouse.go
+++ b/doghouse/server/doghouse.go
@@ -23,7 +23,7 @@ import (
 //  https://api.github.com/repos/easymotion/vim-easymotion/check-runs: 422
 //  Invalid request.
 //  Only 65535 characters are allowed; 250684 were supplied. []
-const maxFilteredFinding = 150
+const maxAllowedSize = 65535
 
 // > The Checks API limits the number of annotations to a maximum of 50 per API
 // > request.
@@ -196,8 +196,9 @@ func (ch *Checker) reqAnnotationLevel() string {
 
 func (ch *Checker) summary(checks []*filter.FilteredDiagnostic) string {
 	var lines []string
+	var usedBytes int
 	lines = append(lines, "reported by [reviewdog](https://github.com/reviewdog/reviewdog) :dog:")
-
+	usedBytes += len(lines[0]) + 1
 	var findings []*filter.FilteredDiagnostic
 	var filteredFindings []*filter.FilteredDiagnostic
 	for _, c := range checks {
@@ -207,27 +208,39 @@ func (ch *Checker) summary(checks []*filter.FilteredDiagnostic) string {
 			filteredFindings = append(filteredFindings, c)
 		}
 	}
-	lines = append(lines, ch.summaryFindings("Findings", findings)...)
-	lines = append(lines, ch.summaryFindings("Filtered Findings", filteredFindings)...)
 
+	findingMsgs, usedBytes := ch.summaryFindings("Findings", usedBytes, findings)
+	lines = append(lines, findingMsgs...)
+	filteredFindingsMsgs, _ := ch.summaryFindings("Filtered Findings", usedBytes, filteredFindings)
+	lines = append(lines, filteredFindingsMsgs...)
 	return strings.Join(lines, "\n")
 }
 
-func (ch *Checker) summaryFindings(name string, checks []*filter.FilteredDiagnostic) []string {
+func (ch *Checker) summaryFindings(name string, usedBytes int, checks []*filter.FilteredDiagnostic) ([]string, int) {
 	var lines []string
-	lines = append(lines, "<details>")
-	lines = append(lines, fmt.Sprintf("<summary>%s (%d)</summary>", name, len(checks)))
-	lines = append(lines, "")
-	for i, c := range checks {
-		if i >= maxFilteredFinding {
-			lines = append(lines, "... (Too many findings. Dropped some findings)")
+	lines = append(lines, fmt.Sprintf("<details>\n<summary>%s (%d)</summary>\n", name, len(checks)))
+	if len(lines[0])+1+usedBytes > maxAllowedSize {
+		// bail out if we're already over the limit
+		return nil, usedBytes
+	}
+	usedBytes += len(lines[0]) + 1
+	for _, c := range checks {
+		nextLine := githubutils.LinkedMarkdownDiagnostic(ch.req.Owner, ch.req.Repo, ch.req.SHA, c.Diagnostic)
+		// existing lines + newline + closing details tag must be smaller than the max allowed size
+		if usedBytes+len(nextLine)+1+10 >= maxAllowedSize {
+			cutoffMsg := "... (Too many findings. Dropped some findings)"
+			if usedBytes+len(cutoffMsg)+1+10 <= maxAllowedSize {
+				lines = append(lines, cutoffMsg)
+				usedBytes += len(cutoffMsg) + 1
+			}
 			break
 		}
-		lines = append(lines, githubutils.LinkedMarkdownDiagnostic(
-			ch.req.Owner, ch.req.Repo, ch.req.SHA, c.Diagnostic))
+		lines = append(lines, nextLine)
+		usedBytes += len(nextLine) + 1
 	}
 	lines = append(lines, "</details>")
-	return lines
+	usedBytes += 10 + 1
+	return lines, usedBytes
 }
 
 func (ch *Checker) toCheckRunAnnotation(c *filter.FilteredDiagnostic) *github.CheckRunAnnotation {
diff --git a/go.mod b/go.mod
index 950d8e2c..873db706 100644
--- a/go.mod
+++ b/go.mod
@@ -25,7 +25,7 @@ require (
 	github.com/vvakame/sdlog v0.0.0-20200409072131-7c0d359efddc
 	github.com/xanzy/go-gitlab v0.73.1
 	go.opencensus.io v0.23.0
-	golang.org/x/build v0.0.0-20220825194821-542f302f8263
+	golang.org/x/build v0.0.0-20220819190107-7cf3efd5d0f9
 	golang.org/x/oauth2 v0.0.0-20220822191816-0ebed06d0094
 	golang.org/x/sync v0.0.0-20220819030929-7fc1605a5dde
 	google.golang.org/protobuf v1.28.1
diff --git a/go.sum b/go.sum
index 0c7053be..20e0b585 100644
--- a/go.sum
+++ b/go.sum
@@ -287,8 +287,8 @@ go.opencensus.io v0.22.5/go.mod h1:5pWMHQbX5EPX2/62yrJeAkowc+lfs/XD7Uxpq3pI6kk=
 go.opencensus.io v0.23.0 h1:gqCw0LfLxScz8irSi8exQc7fyQ0fKQU/qnC/X8+V/1M=
 go.opencensus.io v0.23.0/go.mod h1:XItmlyltB5F7CS4xOC1DcqMoFqwtC6OG2xF7mCv7P7E=
 go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI=
-golang.org/x/build v0.0.0-20220825194821-542f302f8263 h1:BqWeFJtkNMyGOadq+DV5P/SSfgdkEDRr7SWHo0JU9i8=
-golang.org/x/build v0.0.0-20220825194821-542f302f8263/go.mod h1:+glKXzREIk0efZG2AtQ3aBv/8JTl8VM5VCFlLdFUMXU=
+golang.org/x/build v0.0.0-20220819190107-7cf3efd5d0f9 h1:K/v91LAwQMbInqeaLCcVupPQVKYq13COqwyHrdh9B1g=
+golang.org/x/build v0.0.0-20220819190107-7cf3efd5d0f9/go.mod h1:+glKXzREIk0efZG2AtQ3aBv/8JTl8VM5VCFlLdFUMXU=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=


Test patch:
diff --git a/doghouse/server/doghouse_test.go b/doghouse/server/doghouse_test.go
index 279808ed..859670ef 100644
--- a/doghouse/server/doghouse_test.go
+++ b/doghouse/server/doghouse_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"net/http"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -663,3 +664,23 @@ func TestCheck_fail_check_with_403(t *testing.T) {
 		t.Error("resp.CheckedResults should not be nil")
 	}
 }
+func TestCheck_too_many_findings_cut_off_correctly(t *testing.T) {
+	checker := &Checker{req: &doghouse.CheckRequest{}}
+
+	var diagnostics []*filter.FilteredDiagnostic
+	for i := 0; i < 1000; i++ {
+		diagnostics = append(diagnostics, &filter.FilteredDiagnostic{
+			ShouldReport: true,
+			Diagnostic: &rdf.Diagnostic{
+				Message: "this is a pretty long test message that will lead to overshooting the maximum allowed size",
+			},
+		})
+	}
+	summaryText := checker.summary(diagnostics)
+	if len(summaryText) > maxAllowedSize {
+		t.Errorf("summary text is %d bytes long, but the maximum allowed size is %d", len(summaryText), maxAllowedSize)
+	}
+	if !strings.Contains(summaryText, "... (Too many findings. Dropped some findings)\n</details>") {
+		t.Error("summary text was not cut off correctly")
+	}
+}