fix(providers): stop reporting an absent Ollama as an error - #6387
Conversation
Ollama is optional and its URL falls back to a loopback default, so a deployment that runs none refuses the probe on every poll — 10,068 of these in 14 days, the single largest error stream in the app, all of them the same expected condition. Report it the way the vLLM and LiteLLM routes already report an unconfigured base URL, and skip the probe entirely on the hosted platform, which has no local runtime to reach. An explicit OLLAMA_URL is still honoured everywhere, so a self-hosted deployment behaves exactly as before — including the localhost default that requires no configuration.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview Hosted: When Self-hosted: Behavior unchanged for successful probes; connection failures now log at info ( Adds Reviewed by Cursor Bugbot for commit ee8aa7f. Configure here. |
Greptile SummaryThis follow-up cleanly separates an absent Ollama service from malformed successful responses while avoiding unnecessary hosted-platform probes.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/app/api/providers/ollama/models/route.ts | Splits connection, HTTP-status, and response-decoding outcomes so only an absent optional Ollama service is treated as informational. |
| apps/sim/app/api/providers/ollama/models/route.test.ts | Adds focused tests for deployment modes, explicit configuration, logging levels, response validation, and provider blacklisting. |
| apps/sim/lib/core/utils/urls.ts | Adds a centralized helper distinguishing an explicit Ollama URL from the loopback fallback. |
Reviews (2): Last reviewed commit: "fix(providers): keep an unreadable Ollam..." | Re-trigger Greptile
…chable path The single catch covered the connection, the JSON read, and the schema parse, so a server that answered but answered wrongly was filed as 'no Ollama here'. Scope the quiet path to the connection itself and report an unusable response as the fault it is.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ee8aa7f. Configure here.
Summary
OllamaModelsAPIis the single largest error stream in the app: 10,068 in 14 days, and every one is the same line —Unable to connecttohttp://localhost:11434.OLLAMA_URLis unset in production, so the route falls back to a hardcoded loopback default and dials it on every poll.Its siblings already handle this correctly, and produce zero errors over the same window:
logger.info('VLLM_BASE_URL not configured')logger.info('LITELLM_BASE_URL not configured')Changes
info, noterror. It is optional; a deployment that runs none refuses the connection on every poll, which is an expected state, not a failure of this route. This is the half that helps self-hostersOLLAMA_URLis unset. An explicit URL states intent and is still honoured, even on hostedisOllamaUrlConfigured()sits besidegetOllamaUrl()soOLLAMA_URLkeeps a single owner, rather than the route reaching around it to read the same variableThe response is
{ models: [] }on every non-success path, exactly as before — the same value a blacklisted provider already returns.Nothing changes for self-hosters
All four permutations are exercised as tests:
OLLAMA_URLOLLAMA_URLVerified preconditions rather than assumed them:
OLLAMA_URLis declared in the env schema (z.string().url().optional()), so the configured-check reads a real valueisHostedisforceHosted || hostname === 'sim.ai' || *.sim.ai, andforceHostedrequires!isProd— a self-hosted production deployment can never satisfy itproviders/utilsalready importsenv-flags, so the new import adds no dependency weightproviders/ollama/index.ts, the execution path) is untouched; this only affects the model-listing routeType of Change
Testing
5 tests. Both halves verified red on their own revert — removing the gate fails the hosted case, restoring
logger.errorfails the unreachable case. The first version of the level test passed either way (it only asserted the response body), so it now asserts the level itself.app/api/providers50 passing / 5 files; wider sweep 1,593 passing. Typecheck, biome, andcheck:api-validationclean. One unrelated suite fails in the worktree from a tailwind v4/v3 resolution artifact — it fails with my changes stashed and passes on the main checkout (13/13).Checklist