rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 6 Wiki Activity

fix(gitea): handle single-object response in ListContents #74

Merged
aweiker merged 2 commits from issue-73 into main 2026-05-11 15:38:59 +00:00
Conversation 10 Commits 2 Files Changed 2
+4
1 changed files with 4 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit c27dfd0f08 - Show all commits
gitea/client.go
+4
View File
2
 @@ -458,6 +458,10 @@ func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]
if err2 := json.Unmarshal(body, &single); err2 != nil { if err2 := json.Unmarshal(body, &single); err2 != nil {
sonnet-review-bot commented 2026-05-11 14:22:36 +00:00
Review
Copy Link
Copy Source

[MINOR] The fallback tries to unmarshal into a zero-value ContentEntry. If Gitea returns an empty JSON object {}, the unmarshal will succeed and return an entry with all zero/empty fields (Name: "", Path: "", Type: ""), which would be silently added to the results. This is an edge case but could cause confusing downstream behavior. A guard like if single.Name == "" && single.Path == "" before wrapping would be more defensive, though arguably over-engineering for this specific Gitea API.

**[MINOR]** The fallback tries to unmarshal into a zero-value `ContentEntry`. If Gitea returns an empty JSON object `{}`, the unmarshal will succeed and return an entry with all zero/empty fields (`Name: "", Path: "", Type: ""`), which would be silently added to the results. This is an edge case but could cause confusing downstream behavior. A guard like `if single.Name == "" && single.Path == ""` before wrapping would be more defensive, though arguably over-engineering for this specific Gitea API.
sonnet-review-bot commented 2026-05-11 14:22:36 +00:00
Review
Copy Link
Copy Source

[NIT] The variable err2 shadows the outer err in a way that's slightly awkward — err (from the array unmarshal) is preserved for the outer error return while err2 is used only for the single-object attempt. The naming is fine, but a comment noting why err (not err2) is returned in the failure case would aid future readers: the original error is preferred because it describes the primary expected shape.

**[NIT]** The variable `err2` shadows the outer `err` in a way that's slightly awkward — `err` (from the array unmarshal) is preserved for the outer error return while `err2` is used only for the single-object attempt. The naming is fine, but a comment noting why `err` (not `err2`) is returned in the failure case would aid future readers: the original error is preferred because it describes the primary expected shape.
return nil, fmt.Errorf("parse contents JSON: %w", err) return nil, fmt.Errorf("parse contents JSON: %w", err)
} }
sonnet-review-bot commented 2026-05-11 14:47:57 +00:00
Review
Copy Link
Copy Source

[NIT] The guard condition single.Name == "" && single.Path == "" uses AND — a response where Name is populated but Path is empty (or vice versa) would pass the guard and produce a partially-valid entry. Using OR (single.Name == "" || single.Path == "") would be more defensive, though in practice Gitea always returns both fields for a valid file entry.

**[NIT]** The guard condition `single.Name == "" && single.Path == ""` uses AND — a response where Name is populated but Path is empty (or vice versa) would pass the guard and produce a partially-valid entry. Using OR (`single.Name == "" || single.Path == ""`) would be more defensive, though in practice Gitea always returns both fields for a valid file entry.
// Guard against empty/malformed responses
if single.Name == "" && single.Path == "" {
return nil, fmt.Errorf("parse contents JSON: empty response for path %q", path)
}
entries = []ContentEntry{single} entries = []ContentEntry{single}
} }
return entries, nil return entries, nil