refactor: remove AbortSignal support from DataApiService and related components

- Eliminated AbortSignal handling from ApiClient interface and DataApiService to streamline request processing.
- Updated IpcAdapter to remove event sender logic for responses, aligning with the new direct IPC approach.
- Adjusted tests to reflect the removal of cancellation capabilities, emphasizing that direct IPC requests cannot be cancelled.
- Cleaned up related code and comments to enhance clarity and maintainability.
This commit is contained in:
fullex 2025-09-13 13:50:59 +08:00
parent a7d12abd1f
commit bd448b5108
6 changed files with 99 additions and 301 deletions

View File

@ -395,7 +395,6 @@ export interface ApiClient {
options?: {
query?: QueryParamsForPath<TPath>
headers?: Record<string, string>
signal?: AbortSignal
}
): Promise<ResponseForPath<TPath, 'GET'>>
@ -405,7 +404,6 @@ export interface ApiClient {
body?: BodyForPath<TPath, 'POST'>
query?: Record<string, any>
headers?: Record<string, string>
signal?: AbortSignal
}
): Promise<ResponseForPath<TPath, 'POST'>>
@ -415,7 +413,6 @@ export interface ApiClient {
body: BodyForPath<TPath, 'PUT'>
query?: Record<string, any>
headers?: Record<string, string>
signal?: AbortSignal
}
): Promise<ResponseForPath<TPath, 'PUT'>>
@ -424,7 +421,6 @@ export interface ApiClient {
options?: {
query?: Record<string, any>
headers?: Record<string, string>
signal?: AbortSignal
}
): Promise<ResponseForPath<TPath, 'DELETE'>>
@ -434,7 +430,6 @@ export interface ApiClient {
body?: BodyForPath<TPath, 'PATCH'>
query?: Record<string, any>
headers?: Record<string, string>
signal?: AbortSignal
}
): Promise<ResponseForPath<TPath, 'PATCH'>>
}

View File

@ -29,7 +29,7 @@ export class IpcAdapter {
logger.debug('Setting up IPC handlers...')
// Main data request handler
ipcMain.handle(IpcChannel.DataApi_Request, async (event, request: DataRequest): Promise<DataResponse> => {
ipcMain.handle(IpcChannel.DataApi_Request, async (_event, request: DataRequest): Promise<DataResponse> => {
try {
logger.debug(`Handling data request: ${request.method} ${request.path}`, {
id: request.id,
@ -38,9 +38,6 @@ export class IpcAdapter {
const response = await this.apiServer.handleRequest(request)
// Send response back via Data_Response channel for async handling
event.sender.send(IpcChannel.DataApi_Response, response)
return response
} catch (error) {
logger.error(`Data request failed: ${request.method} ${request.path}`, error as Error)
@ -56,9 +53,6 @@ export class IpcAdapter {
}
}
// Send error response
event.sender.send(IpcChannel.DataApi_Response, errorResponse)
return errorResponse
}
})

View File

@ -480,11 +480,6 @@ const api = {
const listener = (_: any, data: any, event: string) => callback(data, event)
ipcRenderer.on(channel, listener)
return () => ipcRenderer.off(channel, listener)
},
onResponse: (callback: (response: any) => void) => {
const listener = (_: any, response: any) => callback(response)
ipcRenderer.on(IpcChannel.DataApi_Response, listener)
return () => ipcRenderer.off(IpcChannel.DataApi_Response, listener)
}
}
}

View File

@ -33,15 +33,6 @@ interface RetryOptions {
export class DataApiService implements ApiClient {
private static instance: DataApiService
private requestId = 0
private pendingRequests = new Map<
string,
{
resolve: (value: DataResponse) => void
reject: (error: Error) => void
timeout?: NodeJS.Timeout
abortController?: AbortController
}
>()
// Subscriptions
private subscriptions = new Map<
@ -72,7 +63,7 @@ export class DataApiService implements ApiClient {
}
private constructor() {
this.setupResponseHandler()
// Initialization completed
}
/**
@ -85,30 +76,6 @@ export class DataApiService implements ApiClient {
return DataApiService.instance
}
/**
* Setup IPC response handler
*/
private setupResponseHandler() {
if (!window.api.dataApi?.onResponse) {
logger.error('Data API not available in preload context')
return
}
window.api.dataApi.onResponse((response: DataResponse) => {
const pending = this.pendingRequests.get(response.id)
if (pending) {
clearTimeout(pending.timeout)
this.pendingRequests.delete(response.id)
if (response.error) {
pending.reject(new Error(response.error.message))
} else {
pending.resolve(response)
}
}
})
}
/**
* Generate unique request ID
*/
@ -116,27 +83,6 @@ export class DataApiService implements ApiClient {
return `req_${Date.now()}_${++this.requestId}`
}
/**
* Cancel request by ID
*/
cancelRequest(requestId: string): void {
const pending = this.pendingRequests.get(requestId)
if (pending) {
pending.abortController?.abort()
clearTimeout(pending.timeout)
this.pendingRequests.delete(requestId)
pending.reject(new Error('Request cancelled'))
}
}
/**
* Cancel all pending requests
*/
cancelAllRequests(): void {
const requestIds = Array.from(this.pendingRequests.keys())
requestIds.forEach((id) => this.cancelRequest(id))
}
/**
* Configure retry options
* @param options Partial retry options to override defaults
@ -158,106 +104,56 @@ export class DataApiService implements ApiClient {
}
/**
* Send request via IPC with AbortController support and retry logic
* Send request via IPC with direct return and retry logic
*/
private async sendRequest<T>(request: DataRequest, retryCount = 0): Promise<T> {
return new Promise((resolve, reject) => {
if (!window.api.dataApi.request) {
reject(new Error('Data API not available'))
return
if (!window.api.dataApi.request) {
throw new Error('Data API not available')
}
try {
logger.debug(`Making ${request.method} request to ${request.path}`, { request })
// Direct IPC call with timeout
const response = await Promise.race([
window.api.dataApi.request(request),
new Promise<never>((_, reject) => setTimeout(() => reject(new Error(`Request timeout: ${request.path}`)), 3000))
])
if (response.error) {
throw new Error(response.error.message)
}
// Create abort controller for cancellation
const abortController = new AbortController()
// Set up timeout
const timeout = setTimeout(() => {
this.pendingRequests.delete(request.id)
const timeoutError = new Error(`Request timeout: ${request.path}`)
// Check if should retry
if (retryCount < this.defaultRetryOptions.maxRetries && this.defaultRetryOptions.retryCondition(timeoutError)) {
logger.debug(
`Request timeout, retrying attempt ${retryCount + 1}/${this.defaultRetryOptions.maxRetries}: ${request.path}`
)
// Calculate delay with exponential backoff
const delay =
this.defaultRetryOptions.retryDelay * Math.pow(this.defaultRetryOptions.backoffMultiplier, retryCount)
setTimeout(() => {
// Create new request with new ID for retry
const retryRequest = { ...request, id: this.generateRequestId() }
this.sendRequest<T>(retryRequest, retryCount + 1)
.then(resolve)
.catch(reject)
}, delay)
} else {
reject(timeoutError)
}
}, 30000) // 30 second timeout
// Store pending request with abort controller
this.pendingRequests.set(request.id, {
resolve: (response: DataResponse) => resolve(response.data),
reject: (error: Error) => {
// Check if should retry on error
if (retryCount < this.defaultRetryOptions.maxRetries && this.defaultRetryOptions.retryCondition(error)) {
logger.debug(
`Request failed, retrying attempt ${retryCount + 1}/${this.defaultRetryOptions.maxRetries}: ${request.path}`,
error
)
const delay =
this.defaultRetryOptions.retryDelay * Math.pow(this.defaultRetryOptions.backoffMultiplier, retryCount)
setTimeout(() => {
const retryRequest = { ...request, id: this.generateRequestId() }
this.sendRequest<T>(retryRequest, retryCount + 1)
.then(resolve)
.catch(reject)
}, delay)
} else {
reject(error)
}
},
timeout,
abortController
logger.debug(`Request succeeded: ${request.method} ${request.path}`, {
status: response.status,
hasData: !!response.data
})
// Handle abort signal
abortController.signal.addEventListener('abort', () => {
clearTimeout(timeout)
this.pendingRequests.delete(request.id)
reject(new Error('Request cancelled'))
})
return response.data as T
} catch (error) {
const errorMessage = error instanceof Error ? error.message : 'Unknown error'
logger.debug(`Request failed: ${request.method} ${request.path}`, error as Error)
// Send request
window.api.dataApi.request(request).catch((error) => {
clearTimeout(timeout)
this.pendingRequests.delete(request.id)
// Check if should retry
if (retryCount < this.defaultRetryOptions.maxRetries && this.defaultRetryOptions.retryCondition(error as Error)) {
logger.debug(
`Retrying request attempt ${retryCount + 1}/${this.defaultRetryOptions.maxRetries}: ${request.path}`,
{ error: errorMessage }
)
// Check if should retry
if (retryCount < this.defaultRetryOptions.maxRetries && this.defaultRetryOptions.retryCondition(error)) {
logger.debug(
`Request failed, retrying attempt ${retryCount + 1}/${this.defaultRetryOptions.maxRetries}: ${request.path}`,
error
)
// Calculate delay with exponential backoff
const delay =
this.defaultRetryOptions.retryDelay * Math.pow(this.defaultRetryOptions.backoffMultiplier, retryCount)
const delay =
this.defaultRetryOptions.retryDelay * Math.pow(this.defaultRetryOptions.backoffMultiplier, retryCount)
await new Promise((resolve) => setTimeout(resolve, delay))
setTimeout(() => {
const retryRequest = { ...request, id: this.generateRequestId() }
this.sendRequest<T>(retryRequest, retryCount + 1)
.then(resolve)
.catch(reject)
}, delay)
} else {
reject(error)
}
})
})
// Create new request with new ID for retry
const retryRequest = { ...request, id: this.generateRequestId() }
return this.sendRequest<T>(retryRequest, retryCount + 1)
}
throw error
}
}
/**
@ -271,15 +167,9 @@ export class DataApiService implements ApiClient {
body?: any
headers?: Record<string, string>
metadata?: Record<string, any>
signal?: AbortSignal
} = {}
): Promise<T> {
const { params, body, headers, metadata, signal } = options
// Check if already aborted
if (signal?.aborted) {
throw new Error('Request cancelled')
}
const { params, body, headers, metadata } = options
// Create request
const request: DataRequest = {
@ -297,23 +187,7 @@ export class DataApiService implements ApiClient {
logger.debug(`Making ${method} request to ${path}`, { request })
// Set up external abort signal handling
const requestPromise = this.sendRequest<T>(request)
if (signal) {
// If external signal is aborted during request, cancel our internal request
const abortListener = () => {
this.cancelRequest(request.id)
}
signal.addEventListener('abort', abortListener)
// Clean up listener when request completes
requestPromise.finally(() => {
signal.removeEventListener('abort', abortListener)
})
}
return requestPromise.catch((error) => {
return this.sendRequest<T>(request).catch((error) => {
logger.error(`Request failed: ${method} ${path}`, error)
throw toDataApiError(error, `${method} ${path}`)
})
@ -327,13 +201,11 @@ export class DataApiService implements ApiClient {
options?: {
query?: any
headers?: Record<string, string>
signal?: AbortSignal
}
): Promise<any> {
return this.makeRequest<any>('GET', path as string, {
params: options?.query,
headers: options?.headers,
signal: options?.signal
headers: options?.headers
})
}
@ -346,14 +218,12 @@ export class DataApiService implements ApiClient {
body?: any
query?: Record<string, any>
headers?: Record<string, string>
signal?: AbortSignal
}
): Promise<any> {
return this.makeRequest<any>('POST', path as string, {
params: options.query,
body: options.body,
headers: options.headers,
signal: options.signal
headers: options.headers
})
}
@ -366,14 +236,12 @@ export class DataApiService implements ApiClient {
body: any
query?: Record<string, any>
headers?: Record<string, string>
signal?: AbortSignal
}
): Promise<any> {
return this.makeRequest<any>('PUT', path as string, {
params: options.query,
body: options.body,
headers: options.headers,
signal: options.signal
headers: options.headers
})
}
@ -385,13 +253,11 @@ export class DataApiService implements ApiClient {
options?: {
query?: Record<string, any>
headers?: Record<string, string>
signal?: AbortSignal
}
): Promise<any> {
return this.makeRequest<any>('DELETE', path as string, {
params: options?.query,
headers: options?.headers,
signal: options?.signal
headers: options?.headers
})
}
@ -404,14 +270,12 @@ export class DataApiService implements ApiClient {
body?: any
query?: Record<string, any>
headers?: Record<string, string>
signal?: AbortSignal
}
): Promise<any> {
return this.makeRequest<any>('PATCH', path as string, {
params: options.query,
body: options.body,
headers: options.headers,
signal: options.signal
headers: options.headers
})
}
@ -471,28 +335,21 @@ export class DataApiService implements ApiClient {
}
/**
* Create an AbortController for request cancellation
* @returns Object with AbortController and convenience methods
*
* @example
* ```typescript
* const { signal, cancel } = requestService.createAbortController()
*
* // Use signal in requests
* const dataPromise = requestService.get('/topics', { signal })
*
* // Cancel if needed
* setTimeout(() => cancel(), 5000) // Cancel after 5 seconds
* ```
* Cancel request by ID
* Note: Direct IPC requests cannot be cancelled once sent
* @deprecated This method has no effect with direct IPC
*/
createAbortController() {
const controller = new AbortController()
cancelRequest(requestId: string): void {
logger.warn('Request cancellation not supported with direct IPC', { requestId })
}
return {
signal: controller.signal,
cancel: () => controller.abort(),
aborted: () => controller.signal.aborted
}
/**
* Cancel all pending requests
* Note: Direct IPC requests cannot be cancelled once sent
* @deprecated This method has no effect with direct IPC
*/
cancelAllRequests(): void {
logger.warn('Request cancellation not supported with direct IPC')
}
/**
@ -500,7 +357,7 @@ export class DataApiService implements ApiClient {
*/
getRequestStats() {
return {
pendingRequests: this.pendingRequests.size,
pendingRequests: 0, // No longer tracked with direct IPC
activeSubscriptions: this.subscriptions.size
}
}
@ -508,10 +365,3 @@ export class DataApiService implements ApiClient {
// Export singleton instance
export const dataApiService = DataApiService.getInstance()
// Clean up on window unload
if (typeof window !== 'undefined') {
window.addEventListener('beforeunload', () => {
dataApiService.cancelAllRequests()
})
}

View File

@ -147,52 +147,28 @@ const DataApiAdvancedTests: React.FC = () => {
setIsRunning(true)
try {
// Test 1: Cancel a slow request
const slowTestPromise = runAdvancedTest('cancel-slow', 'Cancel Slow Request (5s)', 'cancellation', (signal) =>
dataApiService.post('/test/slow', { body: { delay: 5000 }, signal })
)
// Note: Request cancellation is not supported with direct IPC
// These tests demonstrate that cancellation attempts have no effect
// Wait a bit then cancel
setTimeout(() => cancelTest('cancel-slow'), 2000)
// Test 1: Attempt to "cancel" a slow request (will complete normally)
await runAdvancedTest('cancel-slow', 'Slow Request (No Cancel Support)', 'cancellation', async () => {
// Note: This will complete normally since cancellation is not supported
return await dataApiService.post('/test/slow', { body: { delay: 1000 } })
})
try {
await slowTestPromise
} catch {
// Expected to be cancelled
}
// Test 2: Quick request (normal completion)
await runAdvancedTest('cancel-quick', 'Quick Request Test', 'cancellation', async () => {
return await dataApiService.get('/test/items')
})
// Test 2: Cancel a request that should succeed
const quickTestPromise = runAdvancedTest('cancel-quick', 'Cancel Quick Request', 'cancellation', (signal) =>
dataApiService.get('/test/items', { signal })
)
// Test 3: Demonstrate that cancel methods exist but have no effect
await runAdvancedTest('service-cancel', 'Cancel Method Test (No Effect)', 'cancellation', async () => {
// These methods exist but log warnings and have no effect
dataApiService.cancelRequest('dummy-id')
dataApiService.cancelAllRequests()
// Cancel immediately
setTimeout(() => cancelTest('cancel-quick'), 100)
try {
await quickTestPromise
} catch {
// Expected to be cancelled
}
// Test 3: Test using DataApiService's built-in cancellation
await runAdvancedTest('service-cancel', 'DataApiService Built-in Cancellation', 'cancellation', async () => {
const { signal, cancel } = dataApiService.createAbortController()
// Start a slow request
const requestPromise = dataApiService.post('/test/slow', { body: { delay: 3000 }, signal })
// Cancel after 1 second
setTimeout(() => cancel(), 1000)
try {
return await requestPromise
} catch (error) {
if (error instanceof Error && error.message.includes('cancelled')) {
return { cancelled: true, message: 'Successfully cancelled using DataApiService' }
}
throw error
}
// Return successful test data
return { cancelled: false, message: 'Cancel methods called but have no effect with direct IPC' }
})
} finally {
setIsRunning(false)

View File

@ -138,10 +138,7 @@ const DataApiStressTests: React.FC = () => {
}
}, [isRunning, updateRealtimeStats])
const executeStressTest = async (
testName: string,
testFn: (requestId: number, signal: AbortSignal) => Promise<any>
) => {
const executeStressTest = async (testName: string, testFn: (requestId: number) => Promise<any>) => {
setIsRunning(true)
setCurrentTest(testName)
setResults([])
@ -188,7 +185,7 @@ const DataApiStressTests: React.FC = () => {
setResults([...testResults])
try {
const response = await testFn(requestId, signal)
const response = await testFn(requestId)
const endTime = Date.now()
const duration = endTime - startTime
const memoryAfter = getMemoryUsage()
@ -321,37 +318,30 @@ const DataApiStressTests: React.FC = () => {
}
const runConcurrentRequestsTest = async () => {
await executeStressTest('Concurrent Requests Test', async (requestId, signal) => {
await executeStressTest('Concurrent Requests Test', async (requestId) => {
// Alternate between different endpoints to simulate real usage
const endpoints = ['/test/items', '/test/stats'] as const
const endpoint = endpoints[requestId % endpoints.length]
return await dataApiService.get(endpoint, { signal })
return await dataApiService.get(endpoint)
})
}
const runMemoryLeakTest = async () => {
await executeStressTest('Memory Leak Test', async (requestId, signal) => {
// Create and immediately cancel some requests to test cleanup
await executeStressTest('Memory Leak Test', async (requestId) => {
// Test different types of requests to ensure no memory leaks
if (requestId % 5 === 0) {
const { signal: cancelSignal, cancel } = dataApiService.createAbortController()
const requestPromise = dataApiService.post('/test/slow', { body: { delay: 2000 }, signal: cancelSignal })
setTimeout(() => cancel(), 100)
try {
return await requestPromise
} catch (error) {
return { cancelled: true, error: error instanceof Error ? error.message : 'Unknown error' }
}
// Test slower requests
return await dataApiService.post('/test/slow', { body: { delay: 500 } })
} else {
// Normal request
return await dataApiService.get('/test/items', { signal })
return await dataApiService.get('/test/items')
}
})
}
const runErrorHandlingTest = async () => {
await executeStressTest('Error Handling Stress Test', async (requestId, signal) => {
await executeStressTest('Error Handling Stress Test', async (requestId) => {
// Mix of successful and error requests
const errorTypes = ['network', 'server', 'timeout', 'notfound', 'validation']
const shouldError = Math.random() * 100 < config.errorRate
@ -359,24 +349,24 @@ const DataApiStressTests: React.FC = () => {
if (shouldError) {
const errorType = errorTypes[requestId % errorTypes.length]
try {
return await dataApiService.post('/test/error', { body: { errorType: errorType as any }, signal })
return await dataApiService.post('/test/error', { body: { errorType: errorType as any } })
} catch (error) {
return { expectedError: true, error: error instanceof Error ? error.message : 'Unknown error' }
}
} else {
return await dataApiService.get('/test/items', { signal })
return await dataApiService.get('/test/items')
}
})
}
const runMixedOperationsTest = async () => {
await executeStressTest('Mixed Operations Stress Test', async (requestId, signal) => {
await executeStressTest('Mixed Operations Stress Test', async (requestId) => {
const operations = ['GET', 'POST', 'PUT', 'DELETE']
const operation = operations[requestId % operations.length]
switch (operation) {
case 'GET':
return await dataApiService.get('/test/items', { signal })
return await dataApiService.get('/test/items')
case 'POST':
return await dataApiService.post('/test/items', {
@ -384,21 +374,19 @@ const DataApiStressTests: React.FC = () => {
title: `Stress Test Topic ${requestId}`,
description: `Created during stress test #${requestId}`,
type: 'stress-test'
},
signal
}
})
case 'PUT':
// First get an item, then update it
try {
const items = await dataApiService.get('/test/items', { signal })
const items = await dataApiService.get('/test/items')
if ((items as any).items && (items as any).items.length > 0) {
const itemId = (items as any).items[0].id
return await dataApiService.put(`/test/items/${itemId}`, {
body: {
title: `Updated by stress test ${requestId}`
},
signal
}
})
}
} catch (error) {
@ -407,7 +395,7 @@ const DataApiStressTests: React.FC = () => {
return { updateFailed: true, message: 'No items found to update' }
default:
return await dataApiService.get('/test/items', { signal })
return await dataApiService.get('/test/items')
}
})
}