Build production-grade TypeScript API client framework with retry logic and error handling#2
Build production-grade TypeScript API client framework with retry logic and error handling#2Copilot wants to merge 9 commits into
Conversation
Co-authored-by: buttered-spuds <234431829+buttered-spuds@users.noreply.github.com>
Co-authored-by: buttered-spuds <234431829+buttered-spuds@users.noreply.github.com>
Co-authored-by: buttered-spuds <234431829+buttered-spuds@users.noreply.github.com>
Co-authored-by: buttered-spuds <234431829+buttered-spuds@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a production-grade, reusable API client framework for Playwright-based automation, providing enterprise-level features including automatic retry logic with exponential backoff, structured error handling, type-safe responses, and extensibility for domain-specific implementations.
Changes:
- Adds core API framework components (ApiClient, BaseApiRequests, utilities, and interfaces)
- Implements domain-specific example with UserApiRequests, UserHelper, and type-safe models
- Adds comprehensive test suites for both core framework and domain-specific implementations
- Provides extensive documentation including README, Quick Start guide, and usage examples
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds @api/* path alias for clean imports of API framework components |
| api/core/api-client.ts | APIRequestContext wrapper managing lifecycle with proper disposal pattern |
| api/core/base-api-requests.ts | Abstract base class providing HTTP methods, retry logic, and error handling |
| api/core/api-config.interface.ts | TypeScript interfaces for configuration, requests, responses, and errors |
| api/core/api-utils.ts | Utility functions for sanitization, error classification, backoff calculation |
| api/core/index.ts | Core exports for clean import paths |
| api/domain-specific/requests/user-requests.ts | Example domain-specific API requests implementation |
| api/domain-specific/models/user.model.ts | TypeScript type definitions for User domain |
| api/domain-specific/helpers/user-helper.ts | High-level test helper methods with auto-cleanup utilities |
| api/domain-specific/index.ts | Domain-specific exports |
| api/index.ts | Main entry point re-exporting core and domain-specific components |
| tests/api-framework/core/api-framework.spec.ts | Comprehensive tests for core framework utilities and HTTP methods |
| tests/api-framework/domain-specific/user-api.spec.ts | Tests for domain-specific UserApiRequests and UserHelper implementations |
| api/README.md | Comprehensive framework documentation with examples and API reference |
| API_FRAMEWORK_QUICKSTART.md | Quick start guide with basic usage patterns |
| README.md | Updates to main README documenting new API framework feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: buttered-spuds <234431829+buttered-spuds@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot apply changes based on the comments in this thread |
…rallel user creation Co-authored-by: buttered-spuds <234431829+buttered-spuds@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot apply changes based on the comments in this thread |
…and endpoint paths Co-authored-by: buttered-spuds <234431829+buttered-spuds@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: buttered-spuds <234431829+buttered-spuds@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async getUsersPaginated(params?: PaginationParams): Promise<ApiResponse<PaginatedResponse<User>>> { | ||
| const queryParams: Record<string, string | number> = { | ||
| page: params?.page || 1, | ||
| limit: params?.limit || 10, | ||
| }; | ||
|
|
||
| if (params?.sort) queryParams.sort = params.sort; | ||
| if (params?.order) queryParams.order = params.order; | ||
|
|
||
| return this.get<PaginatedResponse<User>>('/users', queryParams); | ||
| } |
There was a problem hiding this comment.
getUsersPaginated() calls the same '/users' endpoint as getUsers() but types the response as PaginatedResponse without any transformation. This is inconsistent with getUsers() returning User[] and will mis-type the runtime value. Either change the return type to User[] or implement mapping from a User[] response into a PaginatedResponse shape (if that’s the intended contract).
| async getUserByEmail(email: string): Promise<ApiResponse<User>> { | ||
| return this.get<User>('/users', { email }); |
There was a problem hiding this comment.
getUserByEmail() performs GET '/users' with an email query parameter but returns ApiResponse. For list endpoints, the response is typically an array; returning a single User here is ambiguous and can mask bugs in callers (e.g., UserHelper.findUserByEmail). Consider returning ApiResponse<User[]> or explicitly selecting the first match and returning User | null with clear semantics.
| async getUserByEmail(email: string): Promise<ApiResponse<User>> { | |
| return this.get<User>('/users', { email }); | |
| async getUserByEmail(email: string): Promise<ApiResponse<User[]>> { | |
| return this.get<User[]>('/users', { email }); |
| async dispose(): Promise<void> { | ||
| if (this.apiContext) { | ||
| await this.apiContext.dispose(); | ||
| this.apiContext = null; | ||
| } |
There was a problem hiding this comment.
ApiClient.dispose() doesn’t account for an in-progress getContext() initialization. If dispose() is called while contextInitializing is pending, apiContext may be set after dispose returns, leaking the context and leaving the client “initialized” again. Consider tracking a disposed flag and/or awaiting contextInitializing in dispose() (and preventing assignment if disposed).
| export interface ApiResponse<T = any> { | ||
| data: T; | ||
| status: number; | ||
| statusText: string; | ||
| headers: Record<string, string>; | ||
| success: boolean; | ||
| } |
There was a problem hiding this comment.
ApiResponse requires data: T, but BaseApiRequests.parseResponse can return undefined for 204 or empty bodies. This makes the public type inaccurate for non-void T and forces unsafe casts. Consider changing ApiResponse to data?: T (or data: T | undefined) and updating parseResponse/makeRequest signatures accordingly.
| * NOTE: Tests that make actual HTTP requests use jsonplaceholder.typicode.com. | ||
| * These may fail if the external service is unavailable or in sandboxed environments. | ||
| * For production CI/CD, consider mocking APIRequestContext or using a local mock server. | ||
| */ |
There was a problem hiding this comment.
These tests hardcode an external dependency (jsonplaceholder.typicode.com). In CI or restricted networks, this will be flaky or fail entirely. Consider gating them behind an env flag (e.g., RUN_EXTERNAL_API_TESTS), skipping by default, or switching to a local mock server/route interception so the default test run stays deterministic.
| protected async makeRequest<T = any>(config: ApiRequestConfig): Promise<ApiResponse<T>> { | ||
| const { endpoint, method, data, params, headers, timeout, retries } = config; | ||
| const maxAttempts = retries !== undefined ? retries : this.retryConfig.maxAttempts; | ||
|
|
||
| let lastError: ApiError | null = null; | ||
| let attempt = 0; | ||
|
|
||
| while (attempt < maxAttempts) { | ||
| try { |
There was a problem hiding this comment.
makeRequest() allows retries to be 0, which makes maxAttempts=0 and the while-loop never runs; the function then throws lastError! (currently null), resulting in a non-ApiError throw. Consider clamping maxAttempts to at least 1 (and/or treating retries as “additional retries” rather than total attempts) so callers can disable retry without disabling the initial request.
| import { BaseApiRequests } from '@api/core'; | ||
| import { ApiResponse } from '@api/core'; |
There was a problem hiding this comment.
In the Quick Start code sample, BaseApiRequests and ApiResponse are imported from '@api/core' in two separate import statements. Consolidating into a single import makes the example cleaner and avoids implying there are different entrypoints.
| import { BaseApiRequests } from '@api/core'; | |
| import { ApiResponse } from '@api/core'; | |
| import { BaseApiRequests, ApiResponse } from '@api/core'; |
| // Validate status code | ||
| const expectedStatusCodes = getExpectedStatusCodes(method); | ||
| const status = response.status(); | ||
|
|
||
| if (!expectedStatusCodes.includes(status)) { | ||
| const errorBody = await this.parseResponse(response); | ||
| throw this.createApiError( | ||
| method, | ||
| endpoint, | ||
| status, | ||
| extractErrorMessage(errorBody), | ||
| errorBody | ||
| ); |
There was a problem hiding this comment.
When building the request URL with query params, the error created on non-2xx uses endpoint rather than the computed url. This drops query parameters from ApiError.endpoint, which makes debugging/log correlation harder. Pass the same URL string you actually requested into createApiError (and keep endpoint vs full URL naming consistent).
| try { | ||
| const text = await response.text(); | ||
|
|
||
| // Handle empty response | ||
| if (!text || text.trim() === '') { | ||
| return undefined as any; | ||
| } | ||
|
|
||
| const parsed = JSON.parse(text); | ||
|
|
||
| // Distinguish between empty objects {}, empty arrays [], and undefined | ||
| if (parsed === null) { | ||
| return null as any; | ||
| } | ||
|
|
||
| return parsed; | ||
| } catch (error) { | ||
| console.warn('Failed to parse JSON response:', error); | ||
| // Return raw text instead of undefined for debugging | ||
| const text = await response.text(); |
There was a problem hiding this comment.
parseResponse() reads the response body text, attempts JSON.parse, and on failure calls response.text() again. Prefer reusing the initially-read text to avoid double-reading/caching assumptions and to keep the warning/error context tied to the same payload.
| try { | |
| const text = await response.text(); | |
| // Handle empty response | |
| if (!text || text.trim() === '') { | |
| return undefined as any; | |
| } | |
| const parsed = JSON.parse(text); | |
| // Distinguish between empty objects {}, empty arrays [], and undefined | |
| if (parsed === null) { | |
| return null as any; | |
| } | |
| return parsed; | |
| } catch (error) { | |
| console.warn('Failed to parse JSON response:', error); | |
| // Return raw text instead of undefined for debugging | |
| const text = await response.text(); | |
| const text = await response.text(); | |
| try { | |
| // Handle empty response | |
| if (!text || text.trim() === '') { | |
| return undefined as any; | |
| } | |
| const parsed = JSON.parse(text); | |
| // Distinguish between empty objects {}, empty arrays [], and undefined | |
| if (parsed === null) { | |
| return null as any; | |
| } | |
| return parsed; | |
| } catch (error) { | |
| console.warn('Failed to parse JSON response:', error); | |
| // Return raw text instead of undefined for debugging |
| timestamp: new Date(), | ||
| retryable, | ||
| originalError, | ||
| stackTrace: new Error().stack, |
There was a problem hiding this comment.
createApiError() sets stackTrace to new Error().stack, which loses the original stack if the caught error already has one. Prefer using originalError?.stack when available (falling back to new Error().stack) so consumers can trace the real failure location.
| stackTrace: new Error().stack, | |
| stackTrace: (originalError && (originalError as any).stack) || new Error().stack, |
API Client Framework Implementation - COMPLETE ✅
Implementation Complete ✅
Successfully built a production-grade TypeScript API client framework for Playwright automation.
Recent Changes (Latest Commit)
Previous Changes
retryConfig.retryableStatusCodesinstead of hardcoded list, making configuration actually effective/api/usersto/usersto match jsonplaceholder.typicode.comCore Features ✅
Test Results ✅
Documentation ✅
Code Quality ✅
Original prompt
This section details on the original issue you should resolve
<issue_title>Build a Robust TypeScript API Client for Playwright Automation with Advanced Error Handling & Retry Logic</issue_title>
<issue_description>Overview
Create a production-grade, reusable API client framework in TypeScript for Playwright-based automation projects. The framework should provide a clean abstraction over Playwright's APIRequestContext with enterprise-level features including automatic retry logic, exponential backoff, structured error handling, request/response logging, and extensibility for domain-specific API implementations.
Core Requirements
Implement a layered architecture similar to C#'s HttpClient pattern:
api/
├── core/
│ ├── api-client.ts # Playwright APIRequestContext wrapper
│ ├── base-api-requests.ts # Abstract base class with core functionality
│ ├── api-config.interface.ts # TypeScript interfaces for type safety
│ └── api-utils.ts # Utility functions (validation, sanitization)
├── domain-specific/
│ ├── requests/
│ │ └── [domain]-requests.ts # Extends BaseApiRequests with domain logic
│ ├── helpers/
│ │ └── [feature]-helper.ts # High-level test helper methods
│ └── models/
│ └── [feature].model.ts # TypeScript types for request/response
A. ApiClient (Wrapper for Playwright's APIRequestContext)
Manages Playwright APIRequestContext lifecycle
Handles context creation with configurable options
Proper disposal pattern to prevent memory leaks
Support for ignoreHTTPSErrors and custom request configurations
Key Features:
class ApiClient {
private apiContext: APIRequestContext | null;
constructor(apiURL: string);
async getContext(): Promise;
async dispose(): Promise;
}
B. BaseApiRequests (Abstract Base Class)
The heart of the framework - provides all core HTTP functionality.
Must Include:
Generic Request Method with Retry Logic
Single makeRequest() method handling all HTTP verbs (GET, POST, PUT, DELETE)
Configurable retry attempts (default: 3)
Exponential backoff with jitter to prevent thundering herd
Automatic determination of retryable vs non-retryable errors
Error Handling
Structured ApiError interface with:
Status code, message, endpoint, timestamp
retryable boolean flag for intelligent retry logic
Stack trace preservation
Parse and format error responses (JSON and plain text)
Clean up special characters in error messages (e.g., \r\n)
Sanitize sensitive data in logs (tokens, passwords, auth headers)
Response Parsing
Automatic JSON parsing with fallback handling
Handle empty responses (204 No Content)
Distinguish between empty objects {}, empty arrays [], and undefined
Content-type aware parsing
HTTP Status Validation
Method-specific success status codes:
GET: 200
POST: 200, 201, 202
PUT: 200, 204
DELETE: 200, 204
Configurable per endpoint if needed
Convenience Methods
Protected get(), post(), put(), delete() wrappers
Type-safe generic responses with ApiResponse
Abstract Methods for Subclasses
protected abstract baseUrl: string;
protected abstract getHeaders(): Promise<Record<string, string>>;
Performance & Observability
Request duration tracking
Structured logging with emoji indicators (🚀 start, ✅ success, ❌ failure, ⏳ retry)
Sanitized endpoint logging (hide sensitive query params)
Optional detailed timing breakdowns
C. TypeScript Int...
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.