Skip to content

fix: improve follow-up prompt timeout and abort handling#6430

Open
Abhiiishek44 wants to merge 2 commits into
FlowiseAI:mainfrom
Abhiiishek44:fix/followup-timeout-retry
Open

fix: improve follow-up prompt timeout and abort handling#6430
Abhiiishek44 wants to merge 2 commits into
FlowiseAI:mainfrom
Abhiiishek44:fix/followup-timeout-retry

Conversation

@Abhiiishek44
Copy link
Copy Markdown

Summary

This PR improves follow-up prompt reliability for Ollama-based requests by making the request flow abort-aware and ensuring cleanup across retry attempts.

Changes included:

  • wrap the Ollama follow-up prompt request in an abort-aware race so configured timeouts and retries are respected consistently
  • ensure AbortSignal listeners are always removed after execution to avoid listener accumulation across retries
  • add a small runtime validation guard for the parsed follow-up prompt response before returning the result

Notes

  • changes are intentionally scoped only to packages/components/src/followUpPrompts.ts
  • no behavior changes for unrelated providers
  • verified under the repository's Node 20 environment
  • local repository-wide TypeScript issues observed during checks appear unrelated to this change

Related: #6421

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a robust retry mechanism with exponential backoff and timeout handling for generating follow-up prompts across various LLM providers. It adds a new executeWithRetry utility and updates provider-specific logic to utilize this wrapper. Feedback highlights unreachable code at the end of the retry function and redundant timeout logic within the Ollama provider implementation, suggesting a cleaner approach using the existing wrapper's capabilities.

}
}

throw new Error('Follow-up prompt retry attempts exhausted')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This line is unreachable. The retry loop already throws the last encountered error when attempt >= maxRetries is reached (line 92).

Comment on lines +281 to +332
// Ollama client does not accept AbortSignal directly in this SDK call.
// Wrap the chat promise in a race so the caller-provided signal can
// abort the request and trigger our retry/timeout behavior.
const chatPromise = ollamaClient.chat({
model: providerConfig.modelName,
messages: [
{
role: 'user',
content: followUpPromptsPrompt
}
],
format: {
type: 'object',
properties: {
questions: {
type: 'array',
items: {
type: 'string'
},
minItems: 3,
maxItems: 3,
description: 'Three follow-up questions based on the conversation history'
}
},
minItems: 3,
maxItems: 3,
description: 'Three follow-up questions based on the conversation history'
required: ['questions'],
additionalProperties: false
},
options: {
temperature: parseFloat(`${providerConfig.temperature}`)
}
},
required: ['questions'],
additionalProperties: false
})

let abortHandler: (() => void) | undefined
const abortPromise = new Promise<never>((_, reject) => {
abortHandler = () => {
const timeoutError = new Error('Follow-up prompt request timed out')
timeoutError.name = 'TimeoutError'
reject(timeoutError)
}
signal.addEventListener('abort', abortHandler, { once: true })
})

try {
const response = await Promise.race([chatPromise, abortPromise])
const result = FollowUpPromptType.parse(JSON.parse((response as any).message.content))
if (!result.questions) {
throw new Error('Follow-up prompt response missing questions')
}
return { questions: result.questions }
} finally {
if (abortHandler) signal.removeEventListener('abort', abortHandler)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The manual Promise.race and AbortSignal listener inside the Ollama provider are redundant because executeWithRetry already wraps the execution in a race against a timeout-driven abort signal. Additionally, the check for result.questions is unnecessary as FollowUpPromptType.parse already validates the presence and type of the field, ensuring fail-fast behavior for external data.

                        // Ollama client does not accept AbortSignal directly in this SDK call.
                        // However, executeWithRetry already handles the timeout and abort logic
                        // via Promise.race at the outer level.
                        const response = await ollamaClient.chat({
                            model: providerConfig.modelName,
                            messages: [
                                {
                                    role: 'user',
                                    content: followUpPromptsPrompt
                                }
                            ],
                            format: {
                                type: 'object',
                                properties: {
                                    questions: {
                                        type: 'array',
                                        items: {
                                            type: 'string'
                                        },
                                        minItems: 3,
                                        maxItems: 3,
                                        description: 'Three follow-up questions based on the conversation history'
                                    }
                                },
                                required: ['questions'],
                                additionalProperties: false
                            },
                            options: {
                                temperature: parseFloat(`${providerConfig.temperature}`)
                            }
                        })

                        const result = FollowUpPromptType.parse(JSON.parse((response as any).message.content))
                        return { questions: result.questions }
References
  1. Use a default (fallback) implementation unless the specific implementation has meaningfully different behavior or provides better error messages.
  2. When handling potentially invalid data from external sources, prefer throwing an error for invalid input types rather than silently returning a default or empty value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant