feat: proper caching and api retry#31
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive HTTP infrastructure layer with retry logic, TTL-based caching, and network monitoring to improve API reliability and performance.
Key Changes:
- Added retryable HTTP client with exponential backoff and rate limit handling
- Implemented TTL-based caching system with LRU eviction
- Created network status monitor with VS Code status bar integration
- Integrated caching and retry logic into Linear and Jira API clients
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared/http/index.ts | Exports for the new HTTP infrastructure modules |
| src/shared/http/TTLCache.ts | TTL-based cache implementation with LRU eviction and pattern-based invalidation |
| src/shared/http/RetryableHttpClient.ts | HTTP client with retry logic, rate limiting, and user-friendly error handling |
| src/shared/http/NetworkMonitor.ts | Network status tracking with VS Code status bar integration |
| src/shared/views/UniversalTicketsProvider.ts | Updated error handling to use user-friendly error messages |
| src/providers/linear/LinearClient.ts | Integrated caching and retry logic, replaced console logs with logger |
| src/providers/jira/server/JiraServerClient.ts | Added caching with appropriate TTLs and cache invalidation after mutations |
| src/providers/jira/common/BaseJiraClient.ts | Refactored to use retryable HTTP client with caching support |
| src/providers/jira/cloud/JiraCloudClient.ts | Added caching with appropriate TTLs and cache invalidation after mutations |
| src/extension.ts | Added cleanup for HTTP infrastructure on deactivation |
| src/activation/treeView.ts | Debounced tree view visibility refresh to prevent rapid-fire updates |
| src/activation/initialization.ts | Initialize network monitor during extension activation |
| docs/planning/ROADMAP_1.0.0.md | Updated roadmap to mark API infrastructure tasks as complete |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| delay = Math.min(retryAfter, retryConfig.maxDelay); | ||
| } else { | ||
| // Default delay for rate limiting without Retry-After | ||
| delay = Math.max(delay, 30000); // At least 30 seconds |
There was a problem hiding this comment.
The magic number 30000 (30 seconds) for rate limit delay lacks explanation. Consider extracting this to a named constant like DEFAULT_RATE_LIMIT_DELAY or adding a more detailed comment explaining why 30 seconds is the minimum for rate limiting without a Retry-After header.
| const debouncedVisibilityRefresh = debounce(() => { | ||
| // Only refresh on visibility if we haven't loaded data yet | ||
| // Once we have data, the cache will keep it fresh | ||
| if (!hasInitialData) { | ||
| logger.debug("Universal tree view became visible, loading initial data"); | ||
| ticketsProvider?.refresh(); | ||
| hasInitialData = true; | ||
| } |
There was a problem hiding this comment.
The magic number 500 (ms) for the debounce delay should be extracted to a named constant like TREE_VIEW_REFRESH_DEBOUNCE_MS to improve maintainability and make the delay configurable.
There was a problem hiding this comment.
lets abstract this magic number
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
@angelo-hub I've opened a new pull request, #32, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: angelo-hub <12058178+angelo-hub@users.noreply.github.com>
Co-authored-by: angelo-hub <12058178+angelo-hub@users.noreply.github.com>
Extract magic numbers to named constants
No description provided.