Skip to content

fix(sidebar): prevent null responses from sidebar API endpoints #974

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DHRUVI5674 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(sidebar): prevent null responses from sidebar API endpoints #974

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 40 additions & 23 deletions server/plugin/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,18 +118,23 @@
)

func (p *Plugin) writeJSON(w http.ResponseWriter, v any) {
b, err := json.Marshal(v)
if err != nil {
p.client.Log.Error("Failed to marshal JSON response", "error", err.Error())
w.WriteHeader(http.StatusInternalServerError)
return
}
_, err = w.Write(b)
if err != nil {
p.client.Log.Error("Failed to write JSON response", "error", err.Error())
w.WriteHeader(http.StatusInternalServerError)
return
}
w.Header().Set("Content-Type", "application/json")

if v == nil {
v = map[string]any{}
}

b, err := json.Marshal(v)
if err != nil {
p.client.Log.Error("Failed to marshal JSON response", "error", err.Error())
w.WriteHeader(http.StatusInternalServerError)
return
}

_, err = w.Write(b)
if err != nil {
p.client.Log.Error("Failed to write JSON response", "error", err.Error())
}
}

func (p *Plugin) writeAPIError(w http.ResponseWriter, apiErr *APIErrorResponse) {
Expand Down Expand Up @@ -713,9 +718,9 @@
return nil
})
if cErr != nil {
c.Log.WithError(cErr).Warnf("Failed to list notifications")
return nil
}
c.Log.WithError(cErr).Warnf("Failed to list notifications")
return []*FilteredNotification{}
}

filteredNotifications := []*FilteredNotification{}
for _, n := range notifications {
Expand Down Expand Up @@ -1040,8 +1045,15 @@
func (p *Plugin) getSidebarData(c *UserContext) (*SidebarContent, error) {
reviewResp, assignmentResp, openPRResp, err := p.getLHSData(c)
if err != nil {
return nil, err
}
c.Log.WithError(err).Warn("Sidebar data fetch failed")

Check failure on line 1048 in server/plugin/api.go

View workflow job for this annotation

GitHub Actions / plugin-ci / lint 

c.Log.WithError(err).Warn undefined (type logger.Logger has no field or method Warn)

Check failure on line 1048 in server/plugin/api.go

View workflow job for this annotation

GitHub Actions / plugin-ci / test 

c.Log.WithError(err).Warn undefined (type logger.Logger has no field or method Warn)
Copy link

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Build failure: Warn method does not exist on logger.Logger.

The static analysis indicates that logger.Logger has no method Warn. Use Warnf instead, which is consistent with other logging calls in this file (e.g., line 721).

🐛 Proposed fix 
-    c.Log.WithError(err).Warn("Sidebar data fetch failed")
+    c.Log.WithError(err).Warnf("Sidebar data fetch failed")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
c.Log.WithError(err).Warn("Sidebar data fetch failed")
c.Log.WithError(err).Warnf("Sidebar data fetch failed")
🧰 Tools
🪛 GitHub Check: plugin-ci / test

[failure] 1048-1048:
c.Log.WithError(err).Warn undefined (type logger.Logger has no field or method Warn)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@server/plugin/api.go` at line 1048, Replace the non-existent call to
c.Log.WithError(err).Warn by using the existing Warnf pattern: call
c.Log.WithError(err).Warnf with a formatted message string (e.g., "Sidebar data
fetch failed: %v") and pass the error or relevant args so the logger compiles
and mirrors other calls in this file (see other uses of
c.Log.WithError(...).Warnf).


return &SidebarContent{
PRs: []*graphql.GithubPRDetails{},
Reviews: []*graphql.GithubPRDetails{},
Assignments: []*github.Issue{},
Unreads: []*FilteredNotification{},
}, nil
}

return &SidebarContent{
PRs: openPRResp,
Expand All @@ -1053,13 +1065,18 @@

func (p *Plugin) getSidebarContent(c *UserContext, w http.ResponseWriter, r *http.Request) {
sidebarContent, err := p.getSidebarData(c)
if err != nil {
c.Log.WithError(err).Errorf("Failed to search for the sidebar data")
p.writeAPIError(w, &APIErrorResponse{Message: "failed to search for the sidebar data", StatusCode: http.StatusInternalServerError})
return
}
if err != nil || sidebarContent == nil {
c.Log.Warn("Sidebar content empty, returning defaults")

Check failure on line 1069 in server/plugin/api.go

View workflow job for this annotation

GitHub Actions / plugin-ci / lint 

c.Log.Warn undefined (type logger.Logger has no field or method Warn)

Check failure on line 1069 in server/plugin/api.go

View workflow job for this annotation

GitHub Actions / plugin-ci / test 

c.Log.Warn undefined (type logger.Logger has no field or method Warn)

sidebarContent = &SidebarContent{
PRs: []*graphql.GithubPRDetails{},
Reviews: []*graphql.GithubPRDetails{},
Assignments: []*github.Issue{},
Unreads: []*FilteredNotification{},
}
}
Comment on lines +1068 to +1077
Copy link

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Build failure: Warn method does not exist on logger.Logger.

Same issue as above - use Warnf instead of Warn.

Additionally, note that this defensive block is now effectively unreachable: getSidebarData always returns a non-nil SidebarContent and nil error after your changes. The check is still reasonable as a safeguard, but consider whether this logging path will ever execute.

🐛 Proposed fix 
 if err != nil || sidebarContent == nil {
-    c.Log.Warn("Sidebar content empty, returning defaults")
+    c.Log.Warnf("Sidebar content empty, returning defaults")
 
     sidebarContent = &SidebarContent{
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if err != nil || sidebarContent == nil {
c.Log.Warn("Sidebar content empty, returning defaults")
sidebarContent = &SidebarContent{
PRs: []*graphql.GithubPRDetails{},
Reviews: []*graphql.GithubPRDetails{},
Assignments: []*github.Issue{},
Unreads: []*FilteredNotification{},
}
}
if err != nil || sidebarContent == nil {
c.Log.Warnf("Sidebar content empty, returning defaults")
sidebarContent = &SidebarContent{
PRs: []*graphql.GithubPRDetails{},
Reviews: []*graphql.GithubPRDetails{},
Assignments: []*github.Issue{},
Unreads: []*FilteredNotification{},
}
}
🧰 Tools
🪛 GitHub Check: plugin-ci / test

[failure] 1069-1069:
c.Log.Warn undefined (type logger.Logger has no field or method Warn)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@server/plugin/api.go` around lines 1068 - 1077, The build fails because
logger.Logger has no Warn method; update the defensive logging in the block that
checks "if err != nil || sidebarContent == nil" to call c.Log.Warnf instead of
c.Log.Warn, e.g., use c.Log.Warnf("Sidebar content empty, returning defaults")
so the call matches the logger API; leave the nil-check and defaulting for
sidebarContent (and note getSidebarData still returns non-nil in current code)
as a safeguard.


p.writeJSON(w, sidebarContent)
p.writeJSON(w, sidebarContent)
}

func (p *Plugin) postToDo(c *UserContext, w http.ResponseWriter, r *http.Request) {
Expand Down
Loading