Skip to content

Commit de5fcf9

Browse files
authored
fix(providers): stop reporting an absent Ollama as an error (#6387)
* fix(providers): stop reporting an absent Ollama as an error 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. * fix(providers): keep an unreadable Ollama response out of the not-reachable 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.
1 parent c9aed7a commit de5fcf9

3 files changed

Lines changed: 224 additions & 14 deletions

File tree

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { createMockRequest, resetEnvFlagsMock, setEnvFlags } from '@sim/testing'
5+
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const {
8+
mockFilterBlacklistedModels,
9+
mockIsProviderBlacklisted,
10+
mockFetch,
11+
mockIsOllamaUrlConfigured,
12+
ollamaLogger,
13+
} = vi.hoisted(() => ({
14+
mockFilterBlacklistedModels: vi.fn(),
15+
mockIsProviderBlacklisted: vi.fn(),
16+
mockFetch: vi.fn(),
17+
mockIsOllamaUrlConfigured: vi.fn(),
18+
ollamaLogger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
19+
}))
20+
21+
vi.mock('@sim/logger', () => ({
22+
createLogger: vi.fn(() => ollamaLogger),
23+
logger: ollamaLogger,
24+
runWithRequestContext: vi.fn(<T>(_ctx: unknown, fn: () => T): T => fn()),
25+
getRequestContext: vi.fn(() => undefined),
26+
}))
27+
28+
vi.mock('@/providers/utils', () => ({
29+
filterBlacklistedModels: mockFilterBlacklistedModels,
30+
isProviderBlacklisted: mockIsProviderBlacklisted,
31+
}))
32+
33+
vi.mock('@/lib/core/utils/urls', () => ({
34+
getOllamaUrl: () => 'http://localhost:11434',
35+
isOllamaUrlConfigured: mockIsOllamaUrlConfigured,
36+
}))
37+
38+
import { GET } from '@/app/api/providers/ollama/models/route'
39+
40+
const request = () => createMockRequest('GET')
41+
42+
describe('ollama models route', () => {
43+
beforeEach(() => {
44+
vi.clearAllMocks()
45+
mockIsOllamaUrlConfigured.mockReturnValue(false)
46+
mockIsProviderBlacklisted.mockReturnValue(false)
47+
mockFilterBlacklistedModels.mockImplementation((models: string[]) => models)
48+
vi.stubGlobal('fetch', mockFetch)
49+
setEnvFlags({ isHosted: false })
50+
})
51+
52+
afterAll(() => {
53+
vi.unstubAllGlobals()
54+
resetEnvFlagsMock()
55+
})
56+
57+
it('does not probe a loopback Ollama on the hosted platform', async () => {
58+
setEnvFlags({ isHosted: true })
59+
60+
const response = await GET(request())
61+
62+
await expect(response.json()).resolves.toEqual({ models: [] })
63+
expect(mockFetch).not.toHaveBeenCalled()
64+
})
65+
66+
it('still honours an explicit OLLAMA_URL on the hosted platform', async () => {
67+
setEnvFlags({ isHosted: true })
68+
mockIsOllamaUrlConfigured.mockReturnValue(true)
69+
mockFetch.mockResolvedValue({ ok: true, json: async () => ({ models: [{ name: 'llama3' }] }) })
70+
71+
const response = await GET(request())
72+
73+
await expect(response.json()).resolves.toEqual({ models: ['llama3'] })
74+
expect(mockFetch).toHaveBeenCalled()
75+
})
76+
77+
it('probes the default host when self-hosted, so no configuration is required', async () => {
78+
mockFetch.mockResolvedValue({ ok: true, json: async () => ({ models: [{ name: 'llama3' }] }) })
79+
80+
const response = await GET(request())
81+
82+
await expect(response.json()).resolves.toEqual({ models: ['llama3'] })
83+
expect(mockFetch).toHaveBeenCalledWith(expect.stringContaining('/api/tags'), expect.anything())
84+
})
85+
86+
it('reports an unreachable Ollama as an empty list rather than a failure', async () => {
87+
/**
88+
* A deployment that runs no Ollama refuses this connection on every poll. The
89+
* level is the point: an optional service being absent is not an error.
90+
*/
91+
mockFetch.mockRejectedValue(new Error('Unable to connect. Is the computer able to access it?'))
92+
93+
const response = await GET(request())
94+
95+
expect(response.status).toBe(200)
96+
await expect(response.json()).resolves.toEqual({ models: [] })
97+
expect(ollamaLogger.error).not.toHaveBeenCalled()
98+
expect(ollamaLogger.info).toHaveBeenCalledWith(
99+
'Ollama service is not reachable, returning empty models',
100+
expect.objectContaining({ host: expect.any(String) })
101+
)
102+
})
103+
104+
it('returns nothing when the provider is blacklisted', async () => {
105+
mockIsProviderBlacklisted.mockReturnValue(true)
106+
107+
const response = await GET(request())
108+
109+
await expect(response.json()).resolves.toEqual({ models: [] })
110+
expect(mockFetch).not.toHaveBeenCalled()
111+
})
112+
113+
it('reports an unreadable response as an error, not as absence', async () => {
114+
/**
115+
* Something answered 2xx but did not return a tag listing. Unlike a refused
116+
* connection that is a real fault, and must not be filed under "no Ollama here".
117+
*/
118+
mockFetch.mockResolvedValue({
119+
ok: true,
120+
json: async () => {
121+
throw new SyntaxError('Unexpected token < in JSON at position 0')
122+
},
123+
})
124+
125+
const response = await GET(request())
126+
127+
expect(response.status).toBe(200)
128+
await expect(response.json()).resolves.toEqual({ models: [] })
129+
expect(ollamaLogger.error).toHaveBeenCalledWith(
130+
'Ollama returned a response this route cannot read',
131+
expect.objectContaining({ host: expect.any(String) })
132+
)
133+
})
134+
135+
it('reports a non-2xx response as unavailable', async () => {
136+
mockFetch.mockResolvedValue({ ok: false, status: 503, statusText: 'Service Unavailable' })
137+
138+
const response = await GET(request())
139+
140+
await expect(response.json()).resolves.toEqual({ models: [] })
141+
expect(ollamaLogger.warn).toHaveBeenCalled()
142+
expect(ollamaLogger.error).not.toHaveBeenCalled()
143+
})
144+
145+
it('reports a wrongly-shaped tag listing as an error', async () => {
146+
/** Reachable and 2xx, but the entries are not Ollama models. */
147+
mockFetch.mockResolvedValue({ ok: true, json: async () => ({ models: [{ noName: true }] }) })
148+
149+
const response = await GET(request())
150+
151+
await expect(response.json()).resolves.toEqual({ models: [] })
152+
expect(ollamaLogger.error).toHaveBeenCalled()
153+
})
154+
155+
it('accepts a listing with no models as simply empty', async () => {
156+
/** The schema defaults `models` to [], so an empty answer is not a fault. */
157+
mockFetch.mockResolvedValue({ ok: true, json: async () => ({}) })
158+
159+
const response = await GET(request())
160+
161+
await expect(response.json()).resolves.toEqual({ models: [] })
162+
expect(ollamaLogger.error).not.toHaveBeenCalled()
163+
})
164+
})

apps/sim/app/api/providers/ollama/models/route.ts

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ import {
55
ollamaUpstreamResponseSchema,
66
providerModelsResponseSchema,
77
} from '@/lib/api/contracts/providers'
8-
import { getOllamaUrl } from '@/lib/core/utils/urls'
8+
import { isHosted } from '@/lib/core/config/env-flags'
9+
import { getOllamaUrl, isOllamaUrlConfigured } from '@/lib/core/utils/urls'
910
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1011
import { filterBlacklistedModels, isProviderBlacklisted } from '@/providers/utils'
1112

@@ -21,26 +22,61 @@ export const GET = withRouteHandler(async (_request: NextRequest) => {
2122
return NextResponse.json({ models: [] })
2223
}
2324

24-
try {
25-
logger.info('Fetching Ollama models', {
26-
host: OLLAMA_HOST,
27-
})
25+
/**
26+
* Ollama runs alongside the app it serves, so the hosted platform never has one
27+
* and `OLLAMA_URL`'s loopback default cannot answer there. Skip the probe rather
28+
* than dial an address known to refuse on every poll.
29+
*
30+
* Only the unconfigured default is skipped: an explicit `OLLAMA_URL` states an
31+
* intent to reach a real server and is still honoured. Self-hosted deployments
32+
* are untouched either way, including the localhost default that needs no
33+
* configuration to work.
34+
*/
35+
if (isHosted && !isOllamaUrlConfigured()) {
36+
logger.info('Ollama is not available on the hosted platform, returning empty models')
37+
return NextResponse.json({ models: [] })
38+
}
2839

29-
const response = await fetch(`${OLLAMA_HOST}/api/tags`, {
40+
logger.info('Fetching Ollama models', {
41+
host: OLLAMA_HOST,
42+
})
43+
44+
let response: Response
45+
try {
46+
response = await fetch(`${OLLAMA_HOST}/api/tags`, {
3047
headers: {
3148
'Content-Type': 'application/json',
3249
},
3350
next: { revalidate: 60 },
3451
})
52+
} catch (error) {
53+
/**
54+
* Ollama is optional, so a deployment that does not run one refuses the
55+
* connection on every poll. That is an expected state rather than a failure of
56+
* this route — the same condition its siblings report when `VLLM_BASE_URL` or
57+
* `LITELLM_BASE_URL` is absent — and the response is the same empty list a
58+
* blacklisted provider returns.
59+
*
60+
* Scoped to the connection itself: a server that answers but answers wrongly is
61+
* a real fault and is reported as one below.
62+
*/
63+
logger.info('Ollama service is not reachable, returning empty models', {
64+
error: getErrorMessage(error, 'Unknown error'),
65+
host: OLLAMA_HOST,
66+
})
3567

36-
if (!response.ok) {
37-
logger.warn('Ollama service is not available', {
38-
status: response.status,
39-
statusText: response.statusText,
40-
})
41-
return NextResponse.json({ models: [] })
42-
}
68+
return NextResponse.json({ models: [] })
69+
}
4370

71+
if (!response.ok) {
72+
logger.warn('Ollama service is not available', {
73+
status: response.status,
74+
statusText: response.statusText,
75+
})
76+
return NextResponse.json({ models: [] })
77+
}
78+
79+
try {
4480
const data = ollamaUpstreamResponseSchema.parse(await response.json())
4581
const allModels = data.models.map((model) => model.name)
4682
const models = filterBlacklistedModels(allModels)
@@ -53,7 +89,8 @@ export const GET = withRouteHandler(async (_request: NextRequest) => {
5389

5490
return NextResponse.json(providerModelsResponseSchema.parse({ models }))
5591
} catch (error) {
56-
logger.error('Failed to fetch Ollama models', {
92+
/** Something is listening and returned 2xx, but not an Ollama tag listing. */
93+
logger.error('Ollama returned a response this route cannot read', {
5794
error: getErrorMessage(error, 'Unknown error'),
5895
host: OLLAMA_HOST,
5996
})

apps/sim/lib/core/utils/urls.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,3 +245,12 @@ export function getSocketUrl(): string {
245245
export function getOllamaUrl(): string {
246246
return env.OLLAMA_URL || DEFAULT_OLLAMA_URL
247247
}
248+
249+
/**
250+
* Whether OLLAMA_URL names a server, as opposed to {@link getOllamaUrl} falling
251+
* back to the loopback default. Callers use this to tell "someone pointed us at
252+
* an Ollama" apart from "nobody configured one".
253+
*/
254+
export function isOllamaUrlConfigured(): boolean {
255+
return Boolean(env.OLLAMA_URL)
256+
}

0 commit comments

Comments
 (0)