Skip to content

Commit bc6d1ac

Browse files
polazpolaz
authored
feat(fetch): add retry logic for idempotent API operations (#120)
* feat(fetch): add retry logic for idempotent API operations Implement automatic retry with exponential backoff for GET/HEAD/OPTIONS requests: - Add retry configuration env vars (GITLAB_API_RETRY_*) - Reduce default timeout from 20s to 10s for faster failures - Add structured TIMEOUT error code for AI client self-correction - Add idempotent flag to EnhancedToolDefinition type - Improve logging with request context (url, method, duration) Retry behavior: - GET requests retry by default (idempotent) - POST/PUT/DELETE do NOT retry by default (not idempotent) - Retries on: timeouts, network errors, 5xx responses, 429 rate limits - Uses exponential backoff: 1s, 2s, 4s (configurable) - Respects Retry-After header for 429 responses Closes #119 * test(fetch): add resetDispatcherCache unit tests * fix(fetch): cancel response body before retry to prevent socket leaks - Call response.body?.cancel() before sleep in retry loop - Use Jest fake timers in Retry-After test - Pass retry:false in error-handling unit tests * fix(fetch): exclude unreachable code from coverage * fix(fetch): improve retry logic robustness and security - Add URL redaction for logs to prevent secret leakage in upload paths - Make network error detection case-insensitive for reliable retries - Validate numeric env vars with Number.isFinite fallback to defaults - Add download_* to isIdempotentOperation for correct timeout labels - Update docstring to match actual retry behavior (GET/HEAD/OPTIONS) - Use fake timers in all retry tests for deterministic execution - Sync README and .env.example with new retry configuration * test(fetch): add configuration edge case tests Add tests for fetch configuration scenarios: - SOCKS proxy detection and warning logging - HTTP/HTTPS proxy dispatcher creation - TLS verification skip warnings - CA certificate loading success and error paths - OAuth token context warnings - URL redaction for invalid URLs and parse errors * fix(test): expect URL-encoded redaction marker in query params URL.searchParams.set() URL-encodes values, so [REDACTED] becomes %5BREDACTED%5D in the logged URL. * chore: test hook enforcement * chore: test hook debug 2 * chore: test fixed hook * fix(fetch): improve URL redaction and cap Retry-After delay - Redact URL userinfo (username:password) in logs - Fix upload secret regex to match any path segment, not just hex - Cap Retry-After header to API_RETRY_MAX_DELAY_MS to prevent excessive waits - Add validation that Retry-After is positive - Add test for Retry-After cap behavior - Update types.ts docs to include download_* in idempotent patterns * test(handlers): add timeout error handling and idempotency tests - Add tests for TIMEOUT structured error response - Test idempotency detection for browse_*, list_*, get_*, download_* tools - Test retryable flag based on tool type (idempotent vs non-idempotent) - Test direct StructuredToolError passthrough - Fix Retry-After cap test to use correct API_RETRY_MAX_DELAY_MS value (4000ms) * fix: improve URL redaction security and config validation - Fix catch fallback in redactUrlForLogging to not leak userinfo - Add positive value validation for config timeout/retry settings - Update test description to match actual behavior * test: update URL redaction test for secure fallback behavior * docs(fetch): update retry JSDoc and test descriptions - List GET/HEAD/OPTIONS methods in retry option JSDoc - Reference jest.resetModules in test file header - Use mocked config value (400ms) in Retry-After cap test * fix(fetch): preserve error stack trace in failure logs * fix(fetch): add access_token and oauth_token to URL redaction * feat(fetch): improve signal handling, error recovery, and HTTP-date parsing - Wrap response.body?.cancel() in try-catch to handle disturbed body errors - Merge caller-provided AbortSignal with internal timeout controller - Support HTTP-date format in Retry-After header (RFC 7231) - Use AbortSignal.any() on Node.js 20+ with fallback for older versions * fix(fetch): distinguish caller abort from internal timeout in retry logic - Use abort reason to identify internal timeout vs caller-initiated abort - Propagate caller AbortError directly instead of converting to timeout - Return false from isRetryableError for AbortError - Document worst-case timing in enhancedFetch JSDoc * test(fetch): caller abort handling and retry behavior tests * test(fetch): update AbortError test expectation * fix(fetch): make backoff sleep abort-aware and accept Retry-After: 0 - sleep() now accepts optional AbortSignal to cancel during backoff - parseRetryAfter() accepts "0" as valid (immediate retry per RFC 7231) - Added tests for both features * test(config): add edge case tests for empty tool/action/param names * chore(deps): update zod to 4.3.6 and @clack/prompts to 1.0.0-alpha.9 - zod: ^4.3.5 → ^4.3.6 - @clack/prompts: ^0.11.0 → 1.0.0-alpha.9 Breaking changes fixed: - validate() now receives string | undefined, added nullish coalescing - SpinnerResult requires additional methods (cancel, error, clear) - @clack/prompts is now ESM-only, added Jest mock for tests * test(fetch): AbortError and Retry-After edge cases - Test AbortError with retry enabled returns false from isRetryableError - Test pre-aborted signal in retry backoff loop - Test invalid Retry-After header uses default backoff - Test Retry-After: 0 for immediate retry - Package.json: add test:help script with usage docs * fix(fetch): cleanup abort listeners and use Symbol for timeout reason - sleep(): remove abort listener on normal timeout completion - doFetch(): store callerAbortHandler ref and cleanup on completion - Use Symbol instead of string for TIMEOUT_REASON (true uniqueness) * fix(fetch): ensure abort rejects with Error, redact end-of-path tokens - getAbortError() helper wraps non-Error reasons in Error instance - Hex token regex now matches tokens at end of URL path * fix(fetch): accept Retry-After with leading zeros per RFC 7231 - Use regex /^\d+$/ instead of String(parseInt(x)) comparison - RFC 7231 delta-seconds = 1*DIGIT allows "01", "001", etc. * fix(fetch): preserve AbortError semantics in getAbortError() - Set error.name = "AbortError" for non-AbortError instances - Use DOMException for non-Error reasons to maintain abort semantics - Allows downstream code to reliably identify abort errors
1 parent b765cb1 commit bc6d1ac

20 files changed

+2044
-47
lines changed

‎.env.example‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,18 @@ NO_PROXY=localhost,127.0.0.1
177177
# PERFORMANCE TUNING
178178
# ============================================================================
179179

180-
# GitLab API timeout in milliseconds (default: 20000 = 20 seconds)
181-
GITLAB_API_TIMEOUT_MS=20000
180+
# GitLab API timeout in milliseconds (default: 10000 = 10 seconds)
181+
GITLAB_API_TIMEOUT_MS=10000
182+
183+
# Retry configuration for idempotent operations (GET/HEAD/OPTIONS)
184+
# Enable automatic retry (default: true)
185+
GITLAB_API_RETRY_ENABLED=true
186+
# Maximum retry attempts (default: 3)
187+
GITLAB_API_RETRY_MAX_ATTEMPTS=3
188+
# Base delay for exponential backoff in ms (default: 1000)
189+
GITLAB_API_RETRY_BASE_DELAY_MS=1000
190+
# Maximum delay cap in ms (default: 4000)
191+
GITLAB_API_RETRY_MAX_DELAY_MS=4000
182192

183193
# Node.js memory limit and options
184194
NODE_OPTIONS=--max-old-space-size=512

‎README.md‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ env = { "GITLAB_TOKEN" = "mytoken", "GITLAB_API_URL" = "https://gitlab.com" }
4848
"GITLAB_PROJECT_ID": "your_project_id", // Optional: default project
4949
"GITLAB_ALLOWED_PROJECT_IDS": "", // Optional: comma-separated list of allowed project IDs
5050
"GITLAB_READ_ONLY_MODE": "false",
51-
"GITLAB_API_TIMEOUT_MS": "20000", // API timeout in milliseconds (default: 20000)
51+
"GITLAB_API_TIMEOUT_MS": "10000", // API timeout in milliseconds (default: 10000)
5252
"USE_GITLAB_WIKI": "false", // use wiki api?
5353
"USE_MILESTONE": "false", // use milestone api?
5454
"USE_PIPELINE": "false", // use pipeline api?
@@ -517,7 +517,11 @@ When OAuth is enabled, the following endpoints are available:
517517
- Multiple values `123,456,789`: MCP server can access projects 123, 456, and 789 but requires explicit project ID in requests
518518
- `GITLAB_READ_ONLY_MODE`: When set to 'true', restricts the server to only expose read-only operations. Useful for enhanced security or when write access is not needed. Also useful for using with Cursor and it's 40 tool limit.
519519
- `GITLAB_DENIED_TOOLS_REGEX`: When set as a regular expression, it excludes the matching tools.
520-
- `GITLAB_API_TIMEOUT_MS`: API request timeout in milliseconds. (Default: `20000` - 20 seconds). If GitLab API doesn't respond within this time, the request will be aborted and a timeout error will be returned to the MCP client.
520+
- `GITLAB_API_TIMEOUT_MS`: API request timeout in milliseconds. (Default: `10000` - 10 seconds). If GitLab API doesn't respond within this time, the request will be aborted. For idempotent operations (GET/HEAD/OPTIONS requests), automatic retry with exponential backoff is attempted before returning a timeout error.
521+
- `GITLAB_API_RETRY_ENABLED`: Enable automatic retry for idempotent operations (GET/HEAD/OPTIONS). (Default: `true`). When enabled, failed requests due to timeouts, network errors, 5xx server errors, or 429 rate limits are automatically retried with exponential backoff. For 429 responses, the Retry-After header is honored when present.
522+
- `GITLAB_API_RETRY_MAX_ATTEMPTS`: Maximum number of retry attempts for failed requests. (Default: `3`). Set to `0` to disable retries.
523+
- `GITLAB_API_RETRY_BASE_DELAY_MS`: Base delay in milliseconds for exponential backoff. (Default: `1000` - 1 second). Actual delays: 1s, 2s, 4s for attempts 1, 2, 3.
524+
- `GITLAB_API_RETRY_MAX_DELAY_MS`: Maximum delay cap for exponential backoff. (Default: `4000` - 4 seconds). Prevents delays from growing too large.
521525
- `USE_GITLAB_WIKI`: When set to 'true', enables the wiki-related tools (list_wiki_pages, get_wiki_page, create_wiki_page, update_wiki_page, delete_wiki_page). Supports both project-level and group-level wikis. By default, wiki features are disabled.
522526
- `USE_MILESTONE`: When set to 'true', enables the milestone-related tools (list_milestones, get_milestone, create_milestone, edit_milestone, delete_milestone, get_milestone_issue, get_milestone_merge_requests, promote_milestone, get_milestone_burndown_events). By default, milestone features are disabled.
523527
- `USE_PIPELINE`: When set to 'true', enables the pipeline-related tools (list_pipelines, get_pipeline, list_pipeline_jobs, list_pipeline_trigger_jobs, get_pipeline_job, get_pipeline_job_output, create_pipeline, retry_pipeline, cancel_pipeline, play_pipeline_job, retry_pipeline_job, cancel_pipeline_job). By default, pipeline features are disabled.

‎jest.config.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ module.exports = {
3838
moduleNameMapper: {
3939
"^(\\.{1,2}/.*)\\.js$": "$1",
4040
},
41+
// Transform ESM modules from node_modules
42+
transformIgnorePatterns: ["node_modules/(?!(@clack/prompts|@clack/core|sisteransi|picocolors)/)"],
4143
testMatch: integrationTestsEnabled
4244
? ["**/tests/**/*.test.ts", "**/?(*.)+(spec|test).ts"]
4345
: ["**/tests/unit/**/*.test.ts"],

‎package.json‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,12 @@
4444
"dev:sse": "node --env-file=.env.test -r source-map-support/register -r ts-node/register --experimental-specifier-resolution=node --experimental-print-required-tla --watch src/main.ts sse",
4545
"dev": "node --env-file=.env.test -r source-map-support/register -r ts-node/register --experimental-specifier-resolution=node --experimental-print-required-tla --watch src/main.ts",
4646
"watch": "tsc --watch",
47-
"test": "yarn jest --runInBand",
48-
"test:unit": "cross-env JEST_UNIT_ONLY=true yarn jest",
47+
"test": "cross-env JEST_UNIT_ONLY=true yarn jest",
4948
"test:cov": "cross-env JEST_UNIT_ONLY=true yarn jest --coverage",
49+
"test:integration": "yarn jest --testPathPattern=tests/integration --runInBand",
50+
"test:all": "yarn jest --runInBand",
5051
"test:env-gating": "node tests/scripts/test-env-gating.js",
52+
"test:help": "printf '# Test Scripts\\n yarn test - Unit tests only (fast, safe default)\\n yarn test <pattern> - Unit tests matching pattern\\n yarn test:cov - Unit test coverage\\n yarn test:integration - Integration tests (needs .env.test)\\n yarn test:all - Full suite (unit + integration)\\n yarn test:env-gating - Test environment gating\\n'",
5153
"cleanup:test-groups": "node scripts/cleanup-test-groups.js",
5254
"cleanup:test-groups:dry-run": "node scripts/cleanup-test-groups.js --dry-run",
5355
"cleanup:test-groups:force": "node scripts/cleanup-test-groups.js --force",
@@ -59,7 +61,7 @@
5961
"list-tools:dev": "node --env-file=.env.test -r source-map-support/register -r ts-node/register --experimental-specifier-resolution=node src/cli/list-tools.ts"
6062
},
6163
"dependencies": {
62-
"@clack/prompts": "^0.11.0",
64+
"@clack/prompts": "1.0.0-alpha.9",
6365
"@graphql-typed-document-node/core": "^3.2.0",
6466
"@modelcontextprotocol/sdk": "^1.25.3",
6567
"@prisma/client": "^7.3.0",
@@ -77,7 +79,7 @@
7779
"transliteration": "^2.6.1",
7880
"undici": "^7.19.0",
7981
"yaml": "^2.8.2",
80-
"zod": "^4.3.5"
82+
"zod": "^4.3.6"
8183
},
8284
"devDependencies": {
8385
"@eslint/js": "^9.39.2",

‎src/cli/docker/docker-command.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ export async function initDocker(): Promise<void> {
148148
placeholder: "3333",
149149
initialValue: "3333",
150150
validate: value => {
151-
const num = parseInt(value, 10);
151+
const num = parseInt(value ?? "", 10);
152152
if (isNaN(num) || num < 1 || num > 65535) {
153153
return "Port must be a number between 1 and 65535";
154154
}

‎src/cli/init/wizard.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ export async function runWizard(): Promise<void> {
6363
message: "Enter your GitLab instance URL:",
6464
placeholder: "https://gitlab.example.com",
6565
validate: value => {
66-
const result = validateGitLabUrl(value);
66+
const result = validateGitLabUrl(value ?? "");
6767
return result.valid ? undefined : result.error;
6868
},
6969
});

‎src/config.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,26 @@ export const SSL_PASSPHRASE = process.env.SSL_PASSPHRASE;
146146
export const TRUST_PROXY = process.env.TRUST_PROXY;
147147

148148
// API timeout configuration (in milliseconds)
149-
export const API_TIMEOUT_MS = parseInt(process.env.GITLAB_API_TIMEOUT_MS ?? "20000", 10);
149+
// Default 10s allows time for retries within a reasonable total response time
150+
const parsedTimeoutMs = parseInt(process.env.GITLAB_API_TIMEOUT_MS ?? "10000", 10);
151+
export const API_TIMEOUT_MS =
152+
Number.isFinite(parsedTimeoutMs) && parsedTimeoutMs > 0 ? parsedTimeoutMs : 10000;
153+
154+
// Retry configuration for idempotent operations (GET/HEAD/OPTIONS requests by default)
155+
// Retries on: timeouts, network errors, 5xx server errors, 429 rate limits
156+
export const API_RETRY_ENABLED = process.env.GITLAB_API_RETRY_ENABLED !== "false";
157+
158+
const parsedMaxAttempts = parseInt(process.env.GITLAB_API_RETRY_MAX_ATTEMPTS ?? "3", 10);
159+
export const API_RETRY_MAX_ATTEMPTS =
160+
Number.isFinite(parsedMaxAttempts) && parsedMaxAttempts >= 0 ? parsedMaxAttempts : 3;
161+
162+
const parsedBaseDelay = parseInt(process.env.GITLAB_API_RETRY_BASE_DELAY_MS ?? "1000", 10);
163+
export const API_RETRY_BASE_DELAY_MS =
164+
Number.isFinite(parsedBaseDelay) && parsedBaseDelay > 0 ? parsedBaseDelay : 1000;
165+
166+
const parsedMaxDelay = parseInt(process.env.GITLAB_API_RETRY_MAX_DELAY_MS ?? "4000", 10);
167+
export const API_RETRY_MAX_DELAY_MS =
168+
Number.isFinite(parsedMaxDelay) && parsedMaxDelay > 0 ? parsedMaxDelay : 4000;
150169

151170
// Rate limiting configuration
152171
// Per-IP rate limiting (for anonymous requests) - enabled by default

‎src/handlers.ts‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import {
66
handleGitLabError,
77
GitLabStructuredError,
88
isStructuredToolError,
9+
createTimeoutError,
10+
parseTimeoutError,
911
} from "./utils/error-handler";
1012

1113
interface JsonSchemaProperty {
@@ -91,6 +93,20 @@ function extractActionFromError(error: unknown): string | undefined {
9193
return undefined;
9294
}
9395

96+
/**
97+
* Check if a tool operation is idempotent (safe to retry)
98+
* browse_* tools are always idempotent (read-only)
99+
* list_*, get_*, and download_* tools are also idempotent
100+
*/
101+
function isIdempotentOperation(toolName: string): boolean {
102+
return (
103+
toolName.startsWith("browse_") ||
104+
toolName.startsWith("list_") ||
105+
toolName.startsWith("get_") ||
106+
toolName.startsWith("download_")
107+
);
108+
}
109+
94110
/**
95111
* Convert an error to a structured GitLab error response
96112
* Extracts tool name and action from context, parses API errors
@@ -113,17 +129,24 @@ function toStructuredError(
113129

114130
if (!(error instanceof Error)) return null;
115131

116-
// Try to parse GitLab API error from message
117-
const parsed = parseGitLabApiError(error.message);
118-
if (!parsed) return null;
119-
120-
// Extract action: prefer from error cause, then from tool args
132+
// Extract action early - needed for both timeout and API errors
121133
let action = extractActionFromError(error);
122134
if (!action && toolArgs && typeof toolArgs.action === "string") {
123135
action = toolArgs.action;
124136
}
125137
action ??= "unknown";
126138

139+
// Check for timeout error first (before parseGitLabApiError)
140+
const timeoutMs = parseTimeoutError(error.message);
141+
if (timeoutMs !== null) {
142+
const retryable = isIdempotentOperation(toolName);
143+
return createTimeoutError(toolName, action, timeoutMs, retryable);
144+
}
145+
146+
// Try to parse GitLab API error from message
147+
const parsed = parseGitLabApiError(error.message);
148+
if (!parsed) return null;
149+
127150
return handleGitLabError(
128151
{ status: parsed.status, message: parsed.message },
129152
toolName,

‎src/types.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,14 @@ export interface FeatureGate {
3333
export interface EnhancedToolDefinition extends ToolDefinition {
3434
handler: (args: unknown) => Promise<unknown>;
3535
gate?: FeatureGate; // Optional - tools without gate are always enabled
36+
/**
37+
* Mark the tool as idempotent (safe to retry on failure).
38+
* If not specified, idempotency is inferred from tool name:
39+
* - browse_*, list_*, get_*, download_* are considered idempotent (read-only)
40+
* - manage_* are considered non-idempotent (write operations)
41+
* Set explicitly to override the default behavior.
42+
*/
43+
idempotent?: boolean;
3644
}
3745

3846
// Tool registry type for storing enhanced tool definitions

‎src/utils/error-handler.ts‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,17 @@ export interface ApiError extends StructuredError {
133133
gitlab_error?: string;
134134
}
135135

136+
/**
137+
* Timeout error for API requests that exceeded the timeout limit
138+
*/
139+
export interface TimeoutError extends StructuredError {
140+
error_code: "TIMEOUT";
141+
/** Timeout duration in milliseconds */
142+
timeout_ms: number;
143+
/** Whether the request can be retried (idempotent operation) */
144+
retryable: boolean;
145+
}
146+
136147
/**
137148
* Union type of all structured errors
138149
*/
@@ -141,7 +152,8 @@ export type GitLabStructuredError =
141152
| TierRestrictedError
142153
| PermissionDeniedError
143154
| NotFoundError
144-
| ApiError;
155+
| ApiError
156+
| TimeoutError;
145157

146158
// ============================================================================
147159
// Tier Restriction Detection
@@ -849,6 +861,48 @@ export function createValidationError(
849861
};
850862
}
851863

864+
// ============================================================================
865+
// Timeout Error Helper
866+
// ============================================================================
867+
868+
/**
869+
* Create a timeout error response
870+
*
871+
* @param tool - Tool name that triggered the timeout
872+
* @param action - Action that was attempted
873+
* @param timeoutMs - Timeout duration in milliseconds
874+
* @param retryable - Whether the operation is idempotent and can be retried
875+
*/
876+
export function createTimeoutError(
877+
tool: string,
878+
action: string,
879+
timeoutMs: number,
880+
retryable: boolean = false
881+
): TimeoutError {
882+
const retryHint = retryable
883+
? " This is a read-only operation - you can safely retry."
884+
: " This is a write operation - check if it completed before retrying.";
885+
886+
return {
887+
error_code: "TIMEOUT",
888+
tool,
889+
action,
890+
timeout_ms: timeoutMs,
891+
retryable,
892+
message: `Request timed out after ${timeoutMs}ms`,
893+
suggested_fix: `The GitLab server is slow to respond. Try again later or increase GITLAB_API_TIMEOUT_MS.${retryHint}`,
894+
};
895+
}
896+
897+
/**
898+
* Parse timeout error from error message
899+
* Returns timeout value in ms if the error is a timeout error, null otherwise
900+
*/
901+
export function parseTimeoutError(errorMessage: string): number | null {
902+
const match = errorMessage.match(/GitLab API timeout after (\d+)ms/);
903+
return match ? parseInt(match[1], 10) : null;
904+
}
905+
852906
// ============================================================================
853907
// Custom Error Class
854908
// ============================================================================

0 commit comments

Comments
 (0)