Skip to content

Add timeout for remote model list loading#6551

Open
vicksiyi wants to merge 2 commits into
FlowiseAI:mainfrom
vicksiyi:codex/bound-model-list-timeout
Open

Add timeout for remote model list loading#6551
vicksiyi wants to merge 2 commits into
FlowiseAI:mainfrom
vicksiyi:codex/bound-model-list-timeout

Conversation

@vicksiyi

Copy link
Copy Markdown

Summary

  • Add a bounded timeout to remote model-list fetches so slow or blackholed URLs fall back to the bundled models file.
  • Add a focused test covering the timeout option and fallback path.

Tests

  • ./node_modules/.bin/jest src/modelLoader.test.ts --runInBand (from packages/components)
  • git diff --check

Notes

  • pnpm-based typecheck is blocked locally by the repo engine gate: current Node v22.22.2 / pnpm 10.6.5, repo requires Node ^24 / pnpm ^10.26.0.

Fixes #6513

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

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.

Code Review

This pull request introduces a 5-second timeout for fetching remote model configurations and adds a corresponding unit test. The review feedback highlights a potential performance and user experience issue where repeated network requests are made on every lookup, recommending the implementation of an in-memory cache. Additionally, it is suggested to make the fetch timeout configurable via an environment variable.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread packages/components/src/modelLoader.ts Outdated
try {
if (isValidUrl(modelFile)) {
const resp = await axios.get(modelFile)
const resp = await axios.get(modelFile, { timeout: MODEL_LIST_FETCH_TIMEOUT_MS })

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.

high

Performance & UX Issue: Repeated Network Requests and Timeouts

Currently, getRawModelFile is called on every single model configuration lookup (e.g., in getModelConfig, getModelConfigByModelName, getModels, and getRegions).

With the new 5-second timeout:

  1. When Online: Every single lookup will trigger a redundant HTTP request to GitHub, causing unnecessary latency and potentially hitting GitHub's rate limits.
  2. When Offline / Slow Connection: Every single lookup will block for 5 seconds before falling back to the local file, leading to a severely degraded user experience and laggy UI.

Recommendation

Implement an in-memory cache for the loaded models so that the remote file (or local fallback) is only fetched/read once per application lifecycle.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in c992188. The loader now caches the loaded model file per model-list source and supports MODEL_LIST_FETCH_TIMEOUT_MS with a default fallback.

Validation: git diff --check and ./node_modules/.bin/jest src/modelLoader.test.ts --runInBand from packages/components.

Comment thread packages/components/src/modelLoader.ts Outdated
import * as path from 'path'
import { INodeOptionsValue } from './Interface'

const MODEL_LIST_FETCH_TIMEOUT_MS = 5000

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

It is recommended to make the fetch timeout configurable via an environment variable. This allows users in restricted network environments, slow proxies, or air-gapped systems to adjust the timeout value as needed.

const MODEL_LIST_FETCH_TIMEOUT_MS = process.env.MODEL_LIST_FETCH_TIMEOUT_MS
    ? parseInt(process.env.MODEL_LIST_FETCH_TIMEOUT_MS, 10) || 5000
    : 5000

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in c992188. The loader now caches the loaded model file per model-list source and supports MODEL_LIST_FETCH_TIMEOUT_MS with a default fallback.

Validation: git diff --check and ./node_modules/.bin/jest src/modelLoader.test.ts --runInBand from packages/components.

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.

Remote model list loading should use a bounded timeout before fallback

1 participant