-
Notifications
You must be signed in to change notification settings - Fork 840
Description
Description
Per the title, the client currently doesn't check response status codes before deserializing in both the non-streaming
extensions/src/Libraries/Microsoft.Extensions.AI.Ollama/OllamaChatClient.cs
Lines 90 to 97 in fc84cf9
| var response = (await httpResponse.Content.ReadFromJsonAsync( | |
| JsonContext.Default.OllamaChatResponse, | |
| cancellationToken).ConfigureAwait(false))!; | |
| if (!string.IsNullOrEmpty(response.Error)) | |
| { | |
| throw new InvalidOperationException($"Ollama error: {response.Error}"); | |
| } |
and streaming implementations
extensions/src/Libraries/Microsoft.Extensions.AI.Ollama/OllamaChatClient.cs
Lines 119 to 126 in fc84cf9
| using var httpResponse = await _httpClient.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false); | |
| using var httpResponseStream = await httpResponse.Content | |
| #if NET | |
| .ReadAsStreamAsync(cancellationToken) | |
| #else | |
| .ReadAsStreamAsync() | |
| #endif | |
| .ConfigureAwait(false); |
In practice, this results in one single empty chat update being returned in cases where, for example, Ollama returns a 404 response. We should fix this and ensure that OllamaSharp doesn't suffer from the same issue.
Reproduction Steps
N/A
Expected behavior
Should throw an exception when non 20x responses are being produced.
Actual behavior
Does not throw an exception when non-20x responses are being produced.
Regression?
No response
Known Workarounds
No response
Configuration
No response
Other information
No response