Skip to content

Commit ee8aa7f

Browse files
committed
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 6bd167e commit ee8aa7f

2 files changed

Lines changed: 86 additions & 20 deletions

File tree

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,4 +109,56 @@ describe('ollama models route', () => {
109109
await expect(response.json()).resolves.toEqual({ models: [] })
110110
expect(mockFetch).not.toHaveBeenCalled()
111111
})
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+
})
112164
})

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

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,26 +37,46 @@ export const GET = withRouteHandler(async (_request: NextRequest) => {
3737
return NextResponse.json({ models: [] })
3838
}
3939

40-
try {
41-
logger.info('Fetching Ollama models', {
42-
host: OLLAMA_HOST,
43-
})
40+
logger.info('Fetching Ollama models', {
41+
host: OLLAMA_HOST,
42+
})
4443

45-
const response = await fetch(`${OLLAMA_HOST}/api/tags`, {
44+
let response: Response
45+
try {
46+
response = await fetch(`${OLLAMA_HOST}/api/tags`, {
4647
headers: {
4748
'Content-Type': 'application/json',
4849
},
4950
next: { revalidate: 60 },
5051
})
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+
})
5167

52-
if (!response.ok) {
53-
logger.warn('Ollama service is not available', {
54-
status: response.status,
55-
statusText: response.statusText,
56-
})
57-
return NextResponse.json({ models: [] })
58-
}
68+
return NextResponse.json({ models: [] })
69+
}
70+
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+
}
5978

79+
try {
6080
const data = ollamaUpstreamResponseSchema.parse(await response.json())
6181
const allModels = data.models.map((model) => model.name)
6282
const models = filterBlacklistedModels(allModels)
@@ -69,14 +89,8 @@ export const GET = withRouteHandler(async (_request: NextRequest) => {
6989

7090
return NextResponse.json(providerModelsResponseSchema.parse({ models }))
7191
} catch (error) {
72-
/**
73-
* Ollama is optional, so a deployment that does not run one refuses the
74-
* connection on every poll. That is an expected state rather than a failure of
75-
* this route — the same condition its siblings report when `VLLM_BASE_URL` or
76-
* `LITELLM_BASE_URL` is absent — and the response is the same empty list a
77-
* blacklisted provider returns.
78-
*/
79-
logger.info('Ollama service is not reachable, returning empty 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', {
8094
error: getErrorMessage(error, 'Unknown error'),
8195
host: OLLAMA_HOST,
8296
})

0 commit comments

Comments
 (0)