Skip to content

Commit aa1d2d6

Browse files
committed
fix: enhance API error diagnostics and tool documentation
- Improve error visibility by reading and returning full Flashduty API error messages. - Use io.LimitReader (4KB) when reading response bodies to prevent potential OOM. - Enrich tool descriptions with specific parameter constraints (lengths, ranges, regex). - Provide more actionable error guidance for time range and pagination limits.
1 parent 4bd5564 commit aa1d2d6

8 files changed

Lines changed: 84 additions & 44 deletions

File tree

pkg/flashduty/changes.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,13 @@ that may be correlated with incidents.
2020
**Parameters:**
2121
- change_ids (optional): Comma-separated change IDs for direct lookup
2222
- channel_id (optional): Filter by collaboration space ID
23-
- start_time, end_time (optional): Unix timestamp range
23+
- start_time, end_time (optional): Unix timestamp in seconds.
24+
**Time constraints:**
25+
- start_time must be less than end_time
26+
- Time range (end_time - start_time) must not exceed 31 days (2678400 seconds)
27+
- If not specified, defaults to last 1 hour
2428
- type (optional): Filter by change type
25-
- limit (optional): Max results (default 20)
29+
- limit (optional): Max results (1-100, default 20)
2630
2731
**Returns:**
2832
- Change records with enriched channel and creator names`
@@ -87,7 +91,7 @@ func QueryChanges(getClient GetFlashdutyClientFn, t translations.TranslationHelp
8791
defer func() { _ = resp.Body.Close() }()
8892

8993
if resp.StatusCode != http.StatusOK {
90-
return mcp.NewToolResultError(fmt.Sprintf("API failed with status %d", resp.StatusCode)), nil
94+
return mcp.NewToolResultError(handleAPIError(resp).Error()), nil
9195
}
9296

9397
var result struct {

pkg/flashduty/channels.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,12 @@ import (
1515
const queryChannelsDescription = `Query collaboration spaces (channels).
1616
1717
**Parameters:**
18-
- channel_ids (optional): Comma-separated channel IDs for direct lookup
19-
- name (optional): Search by channel name
18+
- channel_ids (optional): Comma-separated channel IDs for direct lookup (max 1000)
19+
- name (optional): Search by channel name (case-insensitive substring match)
2020
2121
**Returns:**
22-
- Channel list with team information`
22+
- Channel list with team information
23+
- When listing all channels, returns up to 100 results`
2324

2425
// QueryChannels creates a tool to query channels
2526
func QueryChannels(getClient GetFlashdutyClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
@@ -76,7 +77,7 @@ func QueryChannels(getClient GetFlashdutyClientFn, t translations.TranslationHel
7677
defer func() { _ = resp.Body.Close() }()
7778

7879
if resp.StatusCode != http.StatusOK {
79-
return mcp.NewToolResultError(fmt.Sprintf("API request failed with HTTP status %d", resp.StatusCode)), nil
80+
return mcp.NewToolResultError(handleAPIError(resp).Error()), nil
8081
}
8182

8283
var result struct {
@@ -157,7 +158,7 @@ func QueryEscalationRules(getClient GetFlashdutyClientFn, t translations.Transla
157158
defer func() { _ = resp.Body.Close() }()
158159

159160
if resp.StatusCode != http.StatusOK {
160-
return mcp.NewToolResultError(fmt.Sprintf("API request failed with HTTP status %d", resp.StatusCode)), nil
161+
return mcp.NewToolResultError(handleAPIError(resp).Error()), nil
161162
}
162163

163164
var result struct {

pkg/flashduty/client.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,8 @@ func sanitizeError(err error) string {
130130
// parseResponse parses the HTTP response into the given interface.
131131
// Note: caller is responsible for closing resp.Body.
132132
func parseResponse(resp *http.Response, v interface{}) error {
133-
body, err := io.ReadAll(resp.Body)
133+
// Limit reading to 4KB to prevent potential OOM
134+
body, err := io.ReadAll(io.LimitReader(resp.Body, 4096))
134135
if err != nil {
135136
return fmt.Errorf("failed to read response body: %w", err)
136137
}
@@ -148,6 +149,20 @@ func parseResponse(resp *http.Response, v interface{}) error {
148149
return nil
149150
}
150151

152+
// handleAPIError reads the response body and returns a detailed error message.
153+
// This function should be called when resp.StatusCode != http.StatusOK.
154+
// It returns the full response body which contains request_id for debugging.
155+
func handleAPIError(resp *http.Response) error {
156+
// Limit reading to 4KB to prevent potential OOM with unexpected large response bodies
157+
body, err := io.ReadAll(io.LimitReader(resp.Body, 4096))
158+
if err != nil {
159+
return fmt.Errorf("API request failed with HTTP status %d (failed to read response: %v)", resp.StatusCode, err)
160+
}
161+
162+
// Return full response body for better debugging (contains request_id, error code, message)
163+
return fmt.Errorf("API request failed with HTTP status %d: %s", resp.StatusCode, string(body))
164+
}
165+
151166
// FlashdutyResponse represents the standard Flashduty API response structure
152167
type FlashdutyResponse struct {
153168
Error *DutyError `json:"error,omitempty"`

pkg/flashduty/enrichment.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func (c *Client) fetchIncidentTimeline(ctx context.Context, incidentID string) (
3131
defer func() { _ = resp.Body.Close() }()
3232

3333
if resp.StatusCode != http.StatusOK {
34-
return nil, fmt.Errorf("timeline API request failed with HTTP status %d", resp.StatusCode)
34+
return nil, handleAPIError(resp)
3535
}
3636

3737
var result struct {
@@ -69,7 +69,7 @@ func (c *Client) fetchIncidentAlerts(ctx context.Context, incidentID string, lim
6969
defer func() { _ = resp.Body.Close() }()
7070

7171
if resp.StatusCode != http.StatusOK {
72-
return nil, 0, fmt.Errorf("alerts API request failed with HTTP status %d", resp.StatusCode)
72+
return nil, 0, handleAPIError(resp)
7373
}
7474

7575
var result struct {
@@ -144,7 +144,7 @@ func (c *Client) fetchPersonInfos(ctx context.Context, personIDs []int64) (map[i
144144
defer func() { _ = resp.Body.Close() }()
145145

146146
if resp.StatusCode != http.StatusOK {
147-
return nil, fmt.Errorf("person API request failed with HTTP status %d", resp.StatusCode)
147+
return nil, handleAPIError(resp)
148148
}
149149

150150
var result struct {
@@ -214,7 +214,7 @@ func (c *Client) fetchChannelInfos(ctx context.Context, channelIDs []int64) (map
214214
defer func() { _ = resp.Body.Close() }()
215215

216216
if resp.StatusCode != http.StatusOK {
217-
return nil, fmt.Errorf("channel API request failed with HTTP status %d", resp.StatusCode)
217+
return nil, handleAPIError(resp)
218218
}
219219

220220
var result struct {

pkg/flashduty/fields.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,18 @@ Use this tool to discover available fields before updating incidents.
1818
1919
**Parameters:**
2020
- field_ids (optional): Comma-separated field IDs for direct lookup
21-
- field_name (optional): Search by field name
21+
- field_name (optional): Search by exact field name (field_name must match regexp: ^[a-z][a-z0-9_]*$)
2222
2323
**Returns:**
2424
- Field definitions including:
2525
- field_type: checkbox, multi_select, single_select, text
2626
- value_type: string, bool, float
2727
- options: available values for select fields
28-
- default_value: default value if set`
28+
- default_value: default value if set
29+
30+
**Notes:**
31+
- Maximum 15 custom fields allowed per account
32+
- Returns all fields (no pagination needed)`
2933

3034
// QueryFields creates a tool to query custom field definitions
3135
func QueryFields(getClient GetFlashdutyClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
@@ -54,7 +58,7 @@ func QueryFields(getClient GetFlashdutyClientFn, t translations.TranslationHelpe
5458
defer func() { _ = resp.Body.Close() }()
5559

5660
if resp.StatusCode != http.StatusOK {
57-
return mcp.NewToolResultError(fmt.Sprintf("API failed with status %d", resp.StatusCode)), nil
61+
return mcp.NewToolResultError(handleAPIError(resp).Error()), nil
5862
}
5963

6064
var result struct {

pkg/flashduty/incidents.go

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,12 @@ Returns complete incident information including:
3737
- progress (optional): Filter by status - Triggered, Processing, Closed
3838
- severity (optional): Filter by severity - Info, Warning, Critical
3939
- channel_id (optional): Filter by collaboration space ID
40-
- start_time, end_time (optional): Unix timestamp range (required if no incident_ids)
40+
- start_time, end_time (optional): Unix timestamp in seconds. Required if no incident_ids.
41+
**Time constraints:**
42+
- start_time must be less than end_time
43+
- Time range (end_time - start_time) must not exceed 31 days (2678400 seconds)
44+
- end_time must be within the data retention period (account-specific, typically 365 days)
45+
- If you get a time range error, use more recent timestamps (e.g., last 7-30 days from now)
4146
- title (optional): Title keyword search
4247
- limit (optional): Max results (default 20)
4348
- include_alerts (optional): Include alerts preview (default true)
@@ -342,7 +347,7 @@ func (c *Client) fetchIncidentsByIDs(ctx context.Context, incidentIDs []string)
342347
defer func() { _ = resp.Body.Close() }()
343348

344349
if resp.StatusCode != http.StatusOK {
345-
return nil, fmt.Errorf("API request failed with HTTP status %d", resp.StatusCode)
350+
return nil, handleAPIError(resp)
346351
}
347352

348353
var result struct {
@@ -392,7 +397,7 @@ func (c *Client) fetchIncidentsByFilters(ctx context.Context, progress, severity
392397
defer func() { _ = resp.Body.Close() }()
393398

394399
if resp.StatusCode != http.StatusOK {
395-
return nil, fmt.Errorf("API request failed with HTTP status %d", resp.StatusCode)
400+
return nil, handleAPIError(resp)
396401
}
397402

398403
var result struct {
@@ -416,10 +421,10 @@ func (c *Client) fetchIncidentsByFilters(ctx context.Context, progress, severity
416421
const createIncidentDescription = `Create a new incident in Flashduty.
417422
418423
**Parameters:**
419-
- title (required): Incident title
424+
- title (required): Incident title (3-200 characters)
420425
- severity (required): Info, Warning, or Critical
421426
- channel_id (optional): Collaboration space ID
422-
- description (optional): Incident description
427+
- description (optional): Incident description (max 6144 characters)
423428
- assigned_to (optional): Comma-separated person IDs to assign
424429
425430
**Returns:**
@@ -500,14 +505,15 @@ const updateIncidentDescription = `Update an existing incident.
500505
501506
**Parameters:**
502507
- incident_id (required): Incident ID to update
503-
- title (optional): New title
504-
- description (optional): New description
508+
- title (optional): New title (3-200 characters)
509+
- description (optional): New description (max 6144 characters)
505510
- severity (optional): New severity (Info, Warning, Critical)
506511
- custom_fields (optional): JSON object of custom field updates, e.g. {"field_name": "value"}
507512
508513
**Notes:**
509514
- Only provided fields will be updated
510-
- Use query_fields to discover available custom fields`
515+
- Use query_fields to discover available custom fields
516+
- Custom field names must match regexp: ^[a-z][a-z0-9_]*$`
511517

512518
// UpdateIncident creates a tool to update an incident
513519
func UpdateIncident(getClient GetFlashdutyClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
@@ -627,7 +633,7 @@ func (c *Client) updateIncidentField(ctx context.Context, incidentID, endpoint,
627633
defer func() { _ = resp.Body.Close() }()
628634

629635
if resp.StatusCode != http.StatusOK {
630-
return fmt.Errorf("API request failed with HTTP status %d", resp.StatusCode)
636+
return handleAPIError(resp)
631637
}
632638

633639
var result FlashdutyResponse
@@ -655,7 +661,7 @@ func (c *Client) updateCustomField(ctx context.Context, incidentID, fieldName st
655661
defer func() { _ = resp.Body.Close() }()
656662

657663
if resp.StatusCode != http.StatusOK {
658-
return fmt.Errorf("API request failed with HTTP status %d", resp.StatusCode)
664+
return handleAPIError(resp)
659665
}
660666

661667
var result FlashdutyResponse
@@ -713,7 +719,7 @@ func AckIncident(getClient GetFlashdutyClientFn, t translations.TranslationHelpe
713719
defer func() { _ = resp.Body.Close() }()
714720

715721
if resp.StatusCode != http.StatusOK {
716-
return mcp.NewToolResultError(fmt.Sprintf("API request failed with HTTP status %d", resp.StatusCode)), nil
722+
return mcp.NewToolResultError(handleAPIError(resp).Error()), nil
717723
}
718724

719725
var result FlashdutyResponse
@@ -776,7 +782,7 @@ func CloseIncident(getClient GetFlashdutyClientFn, t translations.TranslationHel
776782
defer func() { _ = resp.Body.Close() }()
777783

778784
if resp.StatusCode != http.StatusOK {
779-
return mcp.NewToolResultError(fmt.Sprintf("API request failed with HTTP status %d", resp.StatusCode)), nil
785+
return mcp.NewToolResultError(handleAPIError(resp).Error()), nil
780786
}
781787

782788
var result FlashdutyResponse
@@ -847,7 +853,7 @@ func ListSimilarIncidents(getClient GetFlashdutyClientFn, t translations.Transla
847853
defer func() { _ = resp.Body.Close() }()
848854

849855
if resp.StatusCode != http.StatusOK {
850-
return mcp.NewToolResultError(fmt.Sprintf("API request failed with HTTP status %d", resp.StatusCode)), nil
856+
return mcp.NewToolResultError(handleAPIError(resp).Error()), nil
851857
}
852858

853859
var result struct {

pkg/flashduty/statuspage.go

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func QueryStatusPages(getClient GetFlashdutyClientFn, t translations.Translation
4747
defer func() { _ = resp.Body.Close() }()
4848

4949
if resp.StatusCode != http.StatusOK {
50-
return mcp.NewToolResultError(fmt.Sprintf("API failed with status %d", resp.StatusCode)), nil
50+
return mcp.NewToolResultError(handleAPIError(resp).Error()), nil
5151
}
5252

5353
var result struct {
@@ -141,7 +141,9 @@ const listStatusChangesDescription = `List active change events (incidents/maint
141141
- type (required): Change type - "incident" or "maintenance"
142142
143143
**Returns:**
144-
- List of active change events (non-resolved/completed)`
144+
- List of active change events (non-resolved/completed)
145+
- For incidents: returns those with status investigating/identified/monitoring (not resolved)
146+
- For maintenances: returns those with status scheduled/ongoing (not completed)`
145147

146148
// ListStatusChanges creates a tool to list status page changes
147149
func ListStatusChanges(getClient GetFlashdutyClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
@@ -181,7 +183,7 @@ func ListStatusChanges(getClient GetFlashdutyClientFn, t translations.Translatio
181183
defer func() { _ = resp.Body.Close() }()
182184

183185
if resp.StatusCode != http.StatusOK {
184-
return mcp.NewToolResultError(fmt.Sprintf("API failed with status %d", resp.StatusCode)), nil
186+
return mcp.NewToolResultError(handleAPIError(resp).Error()), nil
185187
}
186188

187189
var result struct {
@@ -216,15 +218,19 @@ const createStatusIncidentDescription = `Create a new incident on a status page.
216218
217219
**Parameters:**
218220
- page_id (required): Status page ID
219-
- title (required): Incident title
220-
- message (optional): Initial update message describing the incident
221+
- title (required): Incident title (max 255 characters)
222+
- message (optional): Initial update message describing the incident (required for the incident description)
221223
- status (optional): Status - investigating, identified, monitoring, resolved (default: investigating)
222224
- affected_components (optional): Comma-separated component IDs with status, format: "id1:degraded,id2:partial_outage"
223-
- Component statuses: operational, degraded, partial_outage, full_outage
225+
- Component statuses for incidents: degraded, partial_outage, full_outage
226+
- At least one component change is required
224227
- notify_subscribers (optional): Whether to notify subscribers (default: true)
225228
226229
**Returns:**
227-
- Created change event ID`
230+
- Created change event ID
231+
232+
**Notes:**
233+
- The message/description field is required for creating an incident`
228234

229235
// CreateStatusIncident creates a tool to create status page incident
230236
func CreateStatusIncident(getClient GetFlashdutyClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
@@ -340,17 +346,19 @@ const createChangeTimelineDescription = `Add a timeline update to a status page
340346
- page_id (required): Status page ID
341347
- change_id (required): Change event ID to update
342348
- message (required): Update message describing the change
343-
- at (optional): Timestamp for the update (Unix timestamp, default: now)
344-
- status (optional): New status - investigating, identified, monitoring, resolved
349+
- at (optional): Timestamp for the update (Unix timestamp in seconds, default: now)
350+
- status (optional): New status for incidents - investigating, identified, monitoring, resolved
351+
- For maintenances use: scheduled, ongoing, completed
345352
- component_changes (optional): JSON array of component status changes, e.g. [{"component_id":"xxx","status":"degraded"}]
353+
- Component statuses: operational, degraded, partial_outage, full_outage
346354
347355
**Use cases:**
348356
- Post investigation updates
349357
- Mark incident as resolved
350358
- Update affected components
351359
352360
**Returns:**
353-
- Created timeline entry ID`
361+
- Success confirmation`
354362

355363
// CreateChangeTimeline creates a tool to add timeline entry to status change
356364
func CreateChangeTimeline(getClient GetFlashdutyClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {

pkg/flashduty/users.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ const queryMembersDescription = `Query members (users) in the account.
2121
- email (optional): Search by email
2222
2323
**Returns:**
24-
- Member list with ID, name, email, and team memberships`
24+
- Member list with ID, name, email, and team memberships
25+
- When listing members, returns up to 100 results per page (default limit)`
2526

2627
// QueryMembers creates a tool to query members
2728
func QueryMembers(getClient GetFlashdutyClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
@@ -91,7 +92,7 @@ func QueryMembers(getClient GetFlashdutyClientFn, t translations.TranslationHelp
9192
defer func() { _ = resp.Body.Close() }()
9293

9394
if resp.StatusCode != http.StatusOK {
94-
return mcp.NewToolResultError(fmt.Sprintf("API request failed with HTTP status %d", resp.StatusCode)), nil
95+
return mcp.NewToolResultError(handleAPIError(resp).Error()), nil
9596
}
9697

9798
var result MemberListResponse
@@ -123,7 +124,8 @@ const queryTeamsDescription = `Query teams in the account.
123124
- name (optional): Search by team name
124125
125126
**Returns:**
126-
- Team list with members (names and emails)`
127+
- Team list with members (names and emails)
128+
- When listing teams, returns up to 20 results per page (default limit, max 100)`
127129

128130
// QueryTeams creates a tool to query teams
129131
func QueryTeams(getClient GetFlashdutyClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
@@ -162,7 +164,7 @@ func QueryTeams(getClient GetFlashdutyClientFn, t translations.TranslationHelper
162164
defer func() { _ = resp.Body.Close() }()
163165

164166
if resp.StatusCode != http.StatusOK {
165-
return mcp.NewToolResultError(fmt.Sprintf("API request failed with HTTP status %d", resp.StatusCode)), nil
167+
return mcp.NewToolResultError(handleAPIError(resp).Error()), nil
166168
}
167169

168170
var result FlashdutyResponse
@@ -192,7 +194,7 @@ func QueryTeams(getClient GetFlashdutyClientFn, t translations.TranslationHelper
192194
defer func() { _ = resp.Body.Close() }()
193195

194196
if resp.StatusCode != http.StatusOK {
195-
return mcp.NewToolResultError(fmt.Sprintf("API request failed with HTTP status %d", resp.StatusCode)), nil
197+
return mcp.NewToolResultError(handleAPIError(resp).Error()), nil
196198
}
197199

198200
var result struct {

0 commit comments

Comments
 (0)