Skip to content

Commit d973246

Browse files
committed
Fix gitlab crash on bogus response
1 parent cd806eb commit d973246

File tree

3 files changed

+21
-1
lines changed

3 files changed

+21
-1
lines changed

internal/reporter/comments.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,10 @@ func updateDestination(ctx context.Context, s Summary, c Commenter, dst any, sho
335335
if !c.CanDelete(existing) {
336336
goto NEXTDelete
337337
}
338+
slog.Info("Trying to delete a stale existing comment",
339+
slog.String("path", existing.path),
340+
slog.Int("line", existing.line),
341+
)
338342
if err := c.Delete(ctx, dst, existing); err != nil {
339343
slog.Error("Failed to delete a stale comment",
340344
slog.String("reporter", c.Describe()),

internal/reporter/gitlab.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,9 @@ func (gl GitLabReporter) List(_ context.Context, dst any) ([]ExistingComment, er
191191
c = gl.noteToExisting(disc.ID, note)
192192
break
193193
}
194-
comments = append(comments, c)
194+
if c.path != "" && c.line > 0 && c.meta != nil {
195+
comments = append(comments, c)
196+
}
195197
NEXT:
196198
}
197199
return comments, nil

internal/reporter/gitlab_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"net/http/httptest"
99
"os"
1010
"path/filepath"
11+
"regexp"
1112
"strconv"
1213
"strings"
1314
"testing"
@@ -234,6 +235,19 @@ func TestGitLabReporter(t *testing.T) {
234235
maxComments int
235236
showDuplicates bool
236237
}{
238+
{
239+
description: "bogus JSON responses",
240+
timeout: time.Minute,
241+
maxComments: 1,
242+
summary: reporter.NewSummary([]reporter.Report{fooReport}),
243+
mock: httpmock.New(func(s *httpmock.Server) {
244+
s.ExpectGet(apiUser).ReturnJSON(gitlab.User{ID: 123})
245+
s.ExpectGet(regexp.MustCompile(".+")).Times(4).Return(`[{"iid": 333}]`)
246+
}),
247+
errorHandler: func(err error) error {
248+
return err
249+
},
250+
},
237251
{
238252
description: "empty list of merge requests",
239253
timeout: time.Minute,

0 commit comments

Comments
 (0)