Skip to content

Commit d67106b

Browse files
committed
Reject Ollama pull stream error records
Signed-off-by: Kaylee Lubick <klubick@nvidia.com>
1 parent 150f68c commit d67106b

2 files changed

Lines changed: 65 additions & 0 deletions

File tree

‎services/nvpair-engine-manager/pull.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,15 @@ func (e *Executor) PullModelStream(ctx context.Context, engine, model string, pa
126126
if len(line) == 0 || !json.Valid(line) {
127127
continue
128128
}
129+
// Ollama can report a failed pull in an error record after sending HTTP
130+
// 200. Detect it before publishing progress or returning it as success
131+
// so callers use their existing terminal error paths.
132+
var frame struct {
133+
Error string `json:"error"`
134+
}
135+
if err := json.Unmarshal(line, &frame); err == nil && frame.Error != "" {
136+
return nil, fmt.Errorf("pull %q: %s", model, frame.Error)
137+
}
129138
last = append(json.RawMessage(nil), line...)
130139
ev := pullProgressFromLine(engine, line)
131140
if ev.Stage == lastStage && ev.Percent == lastPct {

‎services/nvpair-engine-manager/pull_test.go‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,15 @@ import (
77
"bytes"
88
"context"
99
"encoding/json"
10+
"io"
11+
"net"
12+
"net/http"
1013
"net/http/httptest"
14+
"strconv"
1115
"strings"
1216
"sync"
1317
"testing"
18+
"time"
1419
)
1520

1621
func TestPullProgressFromLine(t *testing.T) {
@@ -229,3 +234,54 @@ func TestActionPullModelFailureEmitsTerminalError(t *testing.T) {
229234
t.Fatalf("expected errors:report on the wire, got %s", reportBuf.String())
230235
}
231236
}
237+
238+
// Ollama can report a failed pull in an NDJSON error record after sending HTTP
239+
// 200. That record must fail the operation even when reading the stream succeeds.
240+
func TestPullModelStreamRejectsOllamaNDJSONError(t *testing.T) {
241+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
242+
if r.Method != http.MethodPost || r.URL.Path != "/api/pull" {
243+
t.Errorf("request = %s %s, want POST /api/pull", r.Method, r.URL.Path)
244+
http.Error(w, "unexpected request", http.StatusBadRequest)
245+
return
246+
}
247+
w.Header().Set("Content-Type", "application/x-ndjson")
248+
w.WriteHeader(http.StatusOK)
249+
if _, err := io.WriteString(w, `{"status":"pulling manifest"}
250+
{"error":"model not found"}
251+
`); err != nil {
252+
t.Errorf("write pull response: %v", err)
253+
}
254+
}))
255+
t.Cleanup(srv.Close)
256+
257+
_, portText, err := net.SplitHostPort(srv.Listener.Addr().String())
258+
if err != nil {
259+
t.Fatalf("parse test server address: %v", err)
260+
}
261+
port, err := strconv.Atoi(portText)
262+
if err != nil {
263+
t.Fatalf("parse test server port: %v", err)
264+
}
265+
266+
m := testEngineManifest(fakeEngineBin)
267+
m.Engine = "ollama"
268+
m.Actions[pullModelAction] = Action{HTTP: &ActionHTTP{Method: http.MethodPost, Path: "/api/pull"}}
269+
ex := newTestExecutor(t, m)
270+
ex.client = srv.Client()
271+
st, err := ex.state(m.Engine)
272+
if err != nil {
273+
t.Fatalf("initialize engine state: %v", err)
274+
}
275+
st.running = true
276+
st.port = port
277+
278+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
279+
t.Cleanup(cancel)
280+
result, err := ex.PullModelStream(ctx, m.Engine, "missing:latest", nil)
281+
if err == nil {
282+
t.Fatalf("pull returned success with result %s for an NDJSON error record", result)
283+
}
284+
if !strings.Contains(err.Error(), "model not found") {
285+
t.Fatalf("pull error = %q, want the engine's model not found message", err)
286+
}
287+
}

0 commit comments

Comments
 (0)