Skip to content

Commit 9a85129

Browse files
committed
handle errors better, add debug logs support
1 parent 98b139a commit 9a85129

File tree

6 files changed

+261
-22
lines changed

6 files changed

+261
-22
lines changed

client/client.go

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,6 @@ type Client struct {
4646
// TODO: We can abstract out the auth provider
4747
APIKey string
4848

49-
Debug bool
50-
5149
// Services used for talking to different Harness entities
5250
Connectors *ConnectorService
5351
PullRequests *PullRequestService
@@ -79,13 +77,6 @@ func NewWithToken(uri, apiKey string) (*Client, error) {
7977
return c, nil
8078
}
8179

82-
// SetDebug sets the debug flag. When the debug flag is
83-
// true, the http.Resposne body to stdout which can be
84-
// helpful when debugging.
85-
func (c *Client) SetDebug(debug bool) {
86-
c.Debug = debug
87-
}
88-
8980
func (c *Client) initialize() error {
9081
if c.client == nil {
9182
c.client = defaultHTTPClient()
@@ -144,15 +135,18 @@ func (c *Client) Get(
144135
return err
145136
}
146137

147-
var respBody string
148-
if unmarshalErr := unmarshalResponse(resp, response); unmarshalErr != nil {
149-
body, _ := io.ReadAll(resp.Body)
150-
respBody = string(body)
151-
return fmt.Errorf("response body unmarshal error: %w - original response: %s", unmarshalErr, respBody)
138+
// Try to unmarshal the response
139+
if err := unmarshalResponse(resp, response); err != nil {
140+
// If we already have a status code error, wrap it with the unmarshal error
141+
if statusErr := mapStatusCodeToError(resp.StatusCode); statusErr != nil {
142+
return fmt.Errorf("%w: %v", statusErr, err)
143+
}
144+
return err
152145
}
153146

147+
// Return any status code error if present
154148
if err != nil {
155-
return fmt.Errorf("%w - response body: %s", err, respBody)
149+
return err
156150
}
157151

158152
return err
@@ -281,9 +275,18 @@ func unmarshalResponse(resp *http.Response, data interface{}) error {
281275
return fmt.Errorf("error reading response body : %w", err)
282276
}
283277

278+
// For non-success status codes, try to unmarshal as an error response first
279+
if resp.StatusCode >= 400 {
280+
var errResp dto.ErrorResponse
281+
if jsonErr := json.Unmarshal(body, &errResp); jsonErr == nil && (errResp.Code != "" || errResp.Message != "") {
282+
return fmt.Errorf("API error: %s", errResp.Message)
283+
}
284+
// If we couldn't parse it as an error response, continue with normal unmarshaling
285+
}
286+
284287
err = json.Unmarshal(body, data)
285288
if err != nil {
286-
return fmt.Errorf("error deserializing response body : %w", err)
289+
return fmt.Errorf("error deserializing response body : %w - original response: %s", err, string(body))
287290
}
288291

289292
return nil

client/dto/error.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package dto
2+
3+
// ErrorResponse represents the standard error response format
4+
// returned by the API when an error occurs
5+
type ErrorResponse struct {
6+
Code string `json:"code,omitempty"`
7+
Message string `json:"message,omitempty"`
8+
}

cmd/harness-mcp-server/config/config.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,5 @@ type Config struct {
1010
ReadOnly bool
1111
Toolsets []string
1212
LogFilePath string
13+
Debug bool
1314
}

cmd/harness-mcp-server/main.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ var (
5757
ReadOnly: viper.GetBool("read_only"),
5858
Toolsets: toolsets,
5959
LogFilePath: viper.GetString("log_file"),
60+
Debug: viper.GetBool("debug"),
6061
}
6162

6263
if err := runStdioServer(cfg); err != nil {
@@ -76,7 +77,7 @@ func init() {
7677
rootCmd.PersistentFlags().StringSlice("toolsets", harness.DefaultTools, "An optional comma separated list of groups of tools to allow, defaults to enabling all")
7778
rootCmd.PersistentFlags().Bool("read-only", false, "Restrict the server to read-only operations")
7879
rootCmd.PersistentFlags().String("log-file", "", "Path to log file")
79-
rootCmd.PersistentFlags().Bool("enable-command-logging", true, "When enabled, the server will log all command requests and responses to the log file")
80+
rootCmd.PersistentFlags().Bool("debug", false, "Enable debug logging")
8081
rootCmd.PersistentFlags().String("base-url", "https://app.harness.io", "Base URL for Harness")
8182
rootCmd.PersistentFlags().String("api-key", "", "API key for authentication")
8283
rootCmd.PersistentFlags().String("account-id", "", "Account ID to use")
@@ -87,7 +88,7 @@ func init() {
8788
_ = viper.BindPFlag("toolsets", rootCmd.PersistentFlags().Lookup("toolsets"))
8889
_ = viper.BindPFlag("read_only", rootCmd.PersistentFlags().Lookup("read-only"))
8990
_ = viper.BindPFlag("log_file", rootCmd.PersistentFlags().Lookup("log-file"))
90-
_ = viper.BindPFlag("enable_command_logging", rootCmd.PersistentFlags().Lookup("enable-command-logging"))
91+
_ = viper.BindPFlag("debug", rootCmd.PersistentFlags().Lookup("debug"))
9192
_ = viper.BindPFlag("base_url", rootCmd.PersistentFlags().Lookup("base-url"))
9293
_ = viper.BindPFlag("api_key", rootCmd.PersistentFlags().Lookup("api-key"))
9394
_ = viper.BindPFlag("account_id", rootCmd.PersistentFlags().Lookup("account-id"))
@@ -104,7 +105,7 @@ func initConfig() {
104105
viper.AutomaticEnv()
105106
}
106107

107-
func initLogger(outPath string) error {
108+
func initLogger(outPath string, debug bool) error {
108109
if outPath == "" {
109110
return nil
110111
}
@@ -114,7 +115,12 @@ func initLogger(outPath string) error {
114115
return fmt.Errorf("failed to open log file: %w", err)
115116
}
116117

117-
logger := slog.New(slog.NewTextHandler(file, nil))
118+
handlerOpts := &slog.HandlerOptions{}
119+
if debug {
120+
handlerOpts.Level = slog.LevelDebug
121+
}
122+
123+
logger := slog.New(slog.NewTextHandler(file, handlerOpts))
118124
slog.SetDefault(logger)
119125
return nil
120126
}
@@ -131,7 +137,7 @@ func runStdioServer(config config.Config) error {
131137
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
132138
defer stop()
133139

134-
err := initLogger(config.LogFilePath)
140+
err := initLogger(config.LogFilePath, config.Debug)
135141
if err != nil {
136142
return fmt.Errorf("failed to initialize logger: %w", err)
137143
}

pkg/harness/pullreq.go

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
package harness
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"fmt"
7+
"strconv"
8+
"strings"
9+
10+
"github.com/harness/harness-mcp/client"
11+
"github.com/harness/harness-mcp/client/dto"
12+
"github.com/harness/harness-mcp/cmd/harness-mcp-server/config"
13+
"github.com/mark3labs/mcp-go/mcp"
14+
"github.com/mark3labs/mcp-go/server"
15+
)
16+
17+
// GetPullRequestTool creates a tool for getting a specific pull request
18+
func GetPullRequestTool(config *config.Config, client *client.Client) (tool mcp.Tool, handler server.ToolHandlerFunc) {
19+
return mcp.NewTool("get_pull_request",
20+
mcp.WithDescription("Get details of a specific pull request in a Harness repository."),
21+
mcp.WithString("repo_id",
22+
mcp.Required(),
23+
mcp.Description("The ID of the repository"),
24+
),
25+
mcp.WithNumber("pr_number",
26+
mcp.Required(),
27+
mcp.Description("The number of the pull request"),
28+
),
29+
WithScope(config, true),
30+
),
31+
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
32+
repoID, err := requiredParam[string](request, "repo_id")
33+
if err != nil {
34+
return mcp.NewToolResultError(err.Error()), nil
35+
}
36+
37+
prNumberFloat, err := requiredParam[float64](request, "pr_number")
38+
if err != nil {
39+
return mcp.NewToolResultError(err.Error()), nil
40+
}
41+
prNumber := int(prNumberFloat)
42+
43+
scope, err := fetchScope(config, request, true)
44+
if err != nil {
45+
return mcp.NewToolResultError(err.Error()), nil
46+
}
47+
48+
data, err := client.PullRequests.Get(ctx, scope, repoID, prNumber)
49+
if err != nil {
50+
return nil, fmt.Errorf("failed to get pull request: %w", err)
51+
}
52+
53+
r, err := json.Marshal(data)
54+
if err != nil {
55+
return nil, fmt.Errorf("failed to marshal pull request: %w", err)
56+
}
57+
58+
return mcp.NewToolResultText(string(r)), nil
59+
}
60+
}
61+
62+
// ListPullRequestsTool creates a tool for listing pull requests
63+
// TODO: more options can be added (sort, order, timestamps, etc)
64+
func ListPullRequestsTool(config *config.Config, client *client.Client) (tool mcp.Tool, handler server.ToolHandlerFunc) {
65+
return mcp.NewTool("list_pull_requests",
66+
mcp.WithDescription("List pull requests in a Harness repository."),
67+
mcp.WithString("repo_id",
68+
mcp.Required(),
69+
mcp.Description("The ID of the repository"),
70+
),
71+
mcp.WithString("state",
72+
mcp.Description("Optional comma-separated states to filter pull requests (possible values: open,closed,merged)"),
73+
),
74+
mcp.WithString("source_branch",
75+
mcp.Description("Optional source branch to filter pull requests"),
76+
),
77+
mcp.WithString("target_branch",
78+
mcp.Description("Optional target branch to filter pull requests"),
79+
),
80+
mcp.WithString("query",
81+
mcp.Description("Optional search query to filter pull requests"),
82+
),
83+
mcp.WithBoolean("include_checks",
84+
mcp.Description("Optional flag to include CI check information for builds ran in the PR"),
85+
),
86+
mcp.WithNumber("page",
87+
mcp.Description("Page number for pagination"),
88+
),
89+
mcp.WithNumber("limit",
90+
mcp.Description("Number of items per page"),
91+
),
92+
WithScope(config, true),
93+
),
94+
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
95+
repoID, err := requiredParam[string](request, "repo_id")
96+
if err != nil {
97+
return mcp.NewToolResultError(err.Error()), nil
98+
}
99+
100+
scope, err := fetchScope(config, request, true)
101+
if err != nil {
102+
return mcp.NewToolResultError(err.Error()), nil
103+
}
104+
105+
opts := &dto.PullRequestOptions{}
106+
107+
// Handle pagination
108+
page, err := OptionalParam[float64](request, "page")
109+
if err != nil {
110+
return mcp.NewToolResultError(err.Error()), nil
111+
}
112+
if page > 0 {
113+
opts.Page = int(page)
114+
}
115+
116+
limit, err := OptionalParam[float64](request, "limit")
117+
if err != nil {
118+
return mcp.NewToolResultError(err.Error()), nil
119+
}
120+
if limit > 0 {
121+
opts.Limit = int(limit)
122+
}
123+
124+
// Handle other optional parameters
125+
stateStr, err := OptionalParam[string](request, "state")
126+
if err != nil {
127+
return mcp.NewToolResultError(err.Error()), nil
128+
}
129+
if stateStr != "" {
130+
opts.State = parseCommaSeparatedList(stateStr)
131+
}
132+
133+
sourceBranch, err := OptionalParam[string](request, "source_branch")
134+
if err != nil {
135+
return mcp.NewToolResultError(err.Error()), nil
136+
}
137+
if sourceBranch != "" {
138+
opts.SourceBranch = sourceBranch
139+
}
140+
141+
targetBranch, err := OptionalParam[string](request, "target_branch")
142+
if err != nil {
143+
return mcp.NewToolResultError(err.Error()), nil
144+
}
145+
if targetBranch != "" {
146+
opts.TargetBranch = targetBranch
147+
}
148+
149+
query, err := OptionalParam[string](request, "query")
150+
if err != nil {
151+
return mcp.NewToolResultError(err.Error()), nil
152+
}
153+
if query != "" {
154+
opts.Query = query
155+
}
156+
157+
authorIDStr, err := OptionalParam[string](request, "author_id")
158+
if err != nil {
159+
return mcp.NewToolResultError(err.Error()), nil
160+
}
161+
if authorIDStr != "" {
162+
authorID, err := strconv.Atoi(authorIDStr)
163+
if err != nil {
164+
return mcp.NewToolResultError(fmt.Sprintf("invalid author_id: %s", authorIDStr)), nil
165+
}
166+
opts.AuthorID = authorID
167+
}
168+
169+
includeChecks, err := OptionalParam[bool](request, "include_checks")
170+
if err != nil {
171+
return mcp.NewToolResultError(err.Error()), nil
172+
}
173+
opts.IncludeChecks = includeChecks
174+
175+
data, err := client.PullRequests.List(ctx, scope, repoID, opts)
176+
if err != nil {
177+
return nil, fmt.Errorf("failed to list pull requests: %w", err)
178+
}
179+
180+
r, err := json.Marshal(data)
181+
if err != nil {
182+
return nil, fmt.Errorf("failed to marshal pull request list: %w", err)
183+
}
184+
185+
return mcp.NewToolResultText(string(r)), nil
186+
}
187+
}
188+
189+
// Helper function to parse comma-separated list
190+
func parseCommaSeparatedList(input string) []string {
191+
if input == "" {
192+
return nil
193+
}
194+
return splitAndTrim(input, ",")
195+
}
196+
197+
// splitAndTrim splits a string by the given separator and trims spaces from each element
198+
func splitAndTrim(s, sep string) []string {
199+
if s == "" {
200+
return nil
201+
}
202+
203+
parts := strings.Split(s, sep)
204+
result := make([]string, 0, len(parts))
205+
206+
for _, part := range parts {
207+
trimmed := strings.TrimSpace(part)
208+
if trimmed != "" {
209+
result = append(result, trimmed)
210+
}
211+
}
212+
213+
return result
214+
}

pkg/harness/tools.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,15 @@ func InitToolsets(client *client.Client, config *config.Config) (*toolsets.Tools
2727
toolsets.NewServerTool(ListExecutionsTool(config, client)),
2828
)
2929

30+
// Create the pull requests toolset
31+
pullrequests := toolsets.NewToolset("pullrequests", "Harness Pull Request related tools").
32+
AddReadTools(
33+
toolsets.NewServerTool(GetPullRequestTool(config, client)),
34+
toolsets.NewServerTool(ListPullRequestsTool(config, client)),
35+
)
36+
3037
// Add toolsets to the group
31-
// tsg.AddToolset(pullrequests)
38+
tsg.AddToolset(pullrequests)
3239
tsg.AddToolset(pipelines)
3340

3441
// Enable requested toolsets

0 commit comments

Comments
 (0)