Skip to content

Fix: Abort fetch requests on timeout in SimStudioClien #656

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix: Abort fetch requests on timeout in SimStudioClien #656

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 19 additions & 19 deletions packages/ts-sdk/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,21 +63,23 @@ export class SimStudioClient {
const { input, timeout = 30000 } = options

try {
// Create a timeout promise
const timeoutPromise = new Promise<never>((_, reject) => {
setTimeout(() => reject(new Error('TIMEOUT')), timeout)
})
const abortController = new AbortController()
const timeoutId = setTimeout(() => abortController.abort(), timeout)

const fetchPromise = fetch(url, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'X-API-Key': this.apiKey,
},
body: JSON.stringify(input || {}),
})
try {
const fetchPromise = fetch(url, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'X-API-Key': this.apiKey,
},
body: JSON.stringify(input || {}),
signal: abortController.signal, // Attach the abort signal
})

Comment on lines +69 to 79
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Remove nested try block - already inside a try block. Move catch handler up to avoid nested error handling.

Suggested change
try {
const fetchPromise = fetch(url, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'X-API-Key': this.apiKey,
},
body: JSON.stringify(input || {}),
signal: abortController.signal, // Attach the abort signal
})
const fetchPromise = fetch(url, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'X-API-Key': this.apiKey,
},
body: JSON.stringify(input || {}),
signal: abortController.signal, // Attach the abort signal
})

const response = await Promise.race([fetchPromise, timeoutPromise])
const response = await fetchPromise // No need for Promise.race here anymore

clearTimeout(timeoutId) // Clear the timeout if fetch completes first

if (!response.ok) {
const errorData = (await response.json().catch(() => ({}))) as unknown as any
Expand All @@ -91,13 +93,11 @@ export class SimStudioClient {
const result = await response.json()
return result as WorkflowExecutionResult
} catch (error: any) {
if (error instanceof SimStudioError) {
throw error
}
clearTimeout(timeoutId) // Ensure timeout is cleared on error too

Comment on lines 95 to 97
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: clearTimeout(timeoutId) is indented incorrectly, making it unreachable for some error paths. Move to same level as catch block.

Suggested change
} catch (error: any) {
if (error instanceof SimStudioError) {
throw error
}
clearTimeout(timeoutId) // Ensure timeout is cleared on error too
} catch (error: any) {
clearTimeout(timeoutId) // Ensure timeout is cleared on error too

if (error.message === 'TIMEOUT') {
throw new SimStudioError(`Workflow execution timed out after ${timeout}ms`, 'TIMEOUT')
}
if (error.name === 'AbortError') {
throw new SimStudioError(`Workflow execution timed out after ${timeout}ms`, 'TIMEOUT')
}

throw new SimStudioError(error?.message || 'Failed to execute workflow', 'EXECUTION_ERROR')
}
Expand Down