Browse Source

api: add optional hints to errors for troubleshooting

Introduces structured error responses that pair error messages with user-friendly
troubleshooting hints. This improves error handling across the codebase and
provides better guidance to users when things go wrong.

Key changes:
- Add ErrorResponse type with Err and Hint fields
- Update client to handle structured errors in streaming and regular responses
- Add specific error handling for common cases like missing models
- Improve CLI output to clearly show both errors and hints
- Add comprehensive test coverage for new error formats

Maintains backward compatibility with existing error handling while making error
messages more helpful and actionable for users.
Bruce MacDonald 2 months ago
parent
commit
4d9568172d
7 changed files with 167 additions and 57 deletions
  1. 30 11
      api/client.go
  2. 81 17
      api/client_test.go
  3. 10 21
      api/types.go
  4. 23 3
      main.go
  5. 3 3
      openai/openai.go
  6. 11 1
      server/images.go
  7. 9 1
      server/routes.go

+ 30 - 11
api/client.go

@@ -18,7 +18,6 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -30,6 +29,28 @@ import (
 	"github.com/ollama/ollama/version"
 )
 
+// StatusError is an error with an HTTP status code and message,
+// it is parsed on the client-side and not returned from the API
+type StatusError struct {
+	StatusCode int    // e.g. 200
+	Status     string // e.g. "200 OK"
+	ErrorResponse
+}
+
+func (e StatusError) Error() string {
+	switch {
+	case e.Status != "" && e.Err != "":
+		return fmt.Sprintf("%s: %s", e.Status, e.Err)
+	case e.Status != "":
+		return e.Status
+	case e.Err != "":
+		return e.Err
+	default:
+		// this should not happen
+		return "something went wrong, please see the ollama server logs for details"
+	}
+}
+
 // Client encapsulates client state for interacting with the ollama
 // service. Use [ClientFromEnvironment] to create new Clients.
 type Client struct {
@@ -47,7 +68,7 @@ func checkError(resp *http.Response, body []byte) error {
 	err := json.Unmarshal(body, &apiError)
 	if err != nil {
 		// Use the full body as the message if we fail to decode a response.
-		apiError.ErrorMessage = string(body)
+		apiError.Err = string(body)
 	}
 
 	return apiError
@@ -163,24 +184,22 @@ func (c *Client) stream(ctx context.Context, method, path string, data any, fn f
 	scanBuf := make([]byte, 0, maxBufferSize)
 	scanner.Buffer(scanBuf, maxBufferSize)
 	for scanner.Scan() {
-		var errorResponse struct {
-			Error string `json:"error,omitempty"`
-		}
-
 		bts := scanner.Bytes()
+
+		var errorResponse ErrorResponse
 		if err := json.Unmarshal(bts, &errorResponse); err != nil {
 			return fmt.Errorf("unmarshal: %w", err)
 		}
 
-		if errorResponse.Error != "" {
-			return errors.New(errorResponse.Error)
+		if errorResponse.Err != "" {
+			return errorResponse
 		}
 
 		if response.StatusCode >= http.StatusBadRequest {
 			return StatusError{
-				StatusCode:   response.StatusCode,
-				Status:       response.Status,
-				ErrorMessage: errorResponse.Error,
+				StatusCode:    response.StatusCode,
+				Status:        response.Status,
+				ErrorResponse: errorResponse,
 			}
 		}
 

+ 81 - 17
api/client_test.go

@@ -50,10 +50,10 @@ func TestClientFromEnvironment(t *testing.T) {
 	}
 }
 
-// testError represents an internal error type with status code and message
-// this is used since the error response from the server is not a standard error struct
+// testError represents an internal error type for testing different error formats
 type testError struct {
-	message    string
+	message    string         // basic error message
+	structured *ErrorResponse // structured error response, nil for basic format
 	statusCode int
 }
 
@@ -68,7 +68,7 @@ func TestClientStream(t *testing.T) {
 		wantErr   string
 	}{
 		{
-			name: "immediate error response",
+			name: "basic error format",
 			responses: []any{
 				testError{
 					message:    "test error message",
@@ -78,16 +78,46 @@ func TestClientStream(t *testing.T) {
 			wantErr: "test error message",
 		},
 		{
-			name: "error after successful chunks, ok response",
+			name: "structured error format",
 			responses: []any{
-				ChatResponse{Message: Message{Content: "partial response 1"}},
-				ChatResponse{Message: Message{Content: "partial response 2"}},
 				testError{
-					message:    "mid-stream error",
+					message: "test structured error",
+					structured: &ErrorResponse{
+						Err:  "test structured error",
+						Hint: "test hint",
+					},
+					statusCode: http.StatusBadRequest,
+				},
+			},
+			wantErr: "test structured error",
+		},
+		{
+			name: "error after chunks - basic format",
+			responses: []any{
+				ChatResponse{Message: Message{Content: "partial 1"}},
+				ChatResponse{Message: Message{Content: "partial 2"}},
+				testError{
+					message:    "mid-stream basic error",
+					statusCode: http.StatusOK,
+				},
+			},
+			wantErr: "mid-stream basic error",
+		},
+		{
+			name: "error after chunks - structured format",
+			responses: []any{
+				ChatResponse{Message: Message{Content: "partial 1"}},
+				ChatResponse{Message: Message{Content: "partial 2"}},
+				testError{
+					message: "mid-stream structured error",
+					structured: &ErrorResponse{
+						Err:  "mid-stream structured error",
+						Hint: "additional context",
+					},
 					statusCode: http.StatusOK,
 				},
 			},
-			wantErr: "mid-stream error",
+			wantErr: "mid-stream structured error",
 		},
 		{
 			name: "successful stream completion",
@@ -116,9 +146,14 @@ func TestClientStream(t *testing.T) {
 				for _, resp := range tc.responses {
 					if errResp, ok := resp.(testError); ok {
 						w.WriteHeader(errResp.statusCode)
-						err := json.NewEncoder(w).Encode(map[string]string{
-							"error": errResp.message,
-						})
+						var err error
+						if errResp.structured != nil {
+							err = json.NewEncoder(w).Encode(errResp.structured)
+						} else {
+							err = json.NewEncoder(w).Encode(map[string]string{
+								"error": errResp.message,
+							})
+						}
 						if err != nil {
 							t.Fatal("failed to encode error response:", err)
 						}
@@ -168,7 +203,7 @@ func TestClientDo(t *testing.T) {
 		wantErr  string
 	}{
 		{
-			name: "immediate error response",
+			name: "basic error format",
 			response: testError{
 				message:    "test error message",
 				statusCode: http.StatusBadRequest,
@@ -176,13 +211,37 @@ func TestClientDo(t *testing.T) {
 			wantErr: "test error message",
 		},
 		{
-			name: "server error response",
+			name: "structured error format",
+			response: testError{
+				message: "test structured error",
+				structured: &ErrorResponse{
+					Err:  "test structured error",
+					Hint: "test hint",
+				},
+				statusCode: http.StatusBadRequest,
+			},
+			wantErr: "test structured error",
+		},
+		{
+			name: "server error - basic format",
 			response: testError{
 				message:    "internal error",
 				statusCode: http.StatusInternalServerError,
 			},
 			wantErr: "internal error",
 		},
+		{
+			name: "server error - structured format",
+			response: testError{
+				message: "internal server error",
+				structured: &ErrorResponse{
+					Err:  "internal server error",
+					Hint: "please try again later",
+				},
+				statusCode: http.StatusInternalServerError,
+			},
+			wantErr: "internal server error",
+		},
 		{
 			name: "successful response",
 			response: struct {
@@ -200,9 +259,14 @@ func TestClientDo(t *testing.T) {
 			ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				if errResp, ok := tc.response.(testError); ok {
 					w.WriteHeader(errResp.statusCode)
-					err := json.NewEncoder(w).Encode(map[string]string{
-						"error": errResp.message,
-					})
+					var err error
+					if errResp.structured != nil {
+						err = json.NewEncoder(w).Encode(errResp.structured)
+					} else {
+						err = json.NewEncoder(w).Encode(map[string]string{
+							"error": errResp.message,
+						})
+					}
 					if err != nil {
 						t.Fatal("failed to encode error response:", err)
 					}

+ 10 - 21
api/types.go

@@ -12,27 +12,6 @@ import (
 	"time"
 )
 
-// StatusError is an error with an HTTP status code and message.
-type StatusError struct {
-	StatusCode   int
-	Status       string
-	ErrorMessage string `json:"error"`
-}
-
-func (e StatusError) Error() string {
-	switch {
-	case e.Status != "" && e.ErrorMessage != "":
-		return fmt.Sprintf("%s: %s", e.Status, e.ErrorMessage)
-	case e.Status != "":
-		return e.Status
-	case e.ErrorMessage != "":
-		return e.ErrorMessage
-	default:
-		// this should not happen
-		return "something went wrong, please see the ollama server logs for details"
-	}
-}
-
 // ImageData represents the raw binary data of an image file.
 type ImageData []byte
 
@@ -661,6 +640,16 @@ func (d *Duration) UnmarshalJSON(b []byte) (err error) {
 	return nil
 }
 
+// ErrorResponse implements a structured error interface that is returned from the Ollama server
+type ErrorResponse struct {
+	Err  string `json:"error,omitempty"` // The annotated error from the server, helps with debugging the code-path
+	Hint string `json:"hint,omitempty"`  // A user-friendly message about what went wrong, with suggested troubleshooting
+}
+
+func (e ErrorResponse) Error() string {
+	return e.Err
+}
+
 // FormatParams converts specified parameter options to their correct types
 func FormatParams(params map[string][]string) (map[string]interface{}, error) {
 	opts := Options{}

+ 23 - 3
main.go

@@ -2,12 +2,32 @@ package main
 
 import (
 	"context"
+	"fmt"
+	"os"
 
-	"github.com/spf13/cobra"
-
+	"github.com/ollama/ollama/api"
 	"github.com/ollama/ollama/cmd"
 )
 
 func main() {
-	cobra.CheckErr(cmd.NewCLI().ExecuteContext(context.Background()))
+	checkErr(cmd.NewCLI().ExecuteContext(context.Background()))
+}
+
+// checkErr prints the error message and exits the program if the message is not nil.
+func checkErr(msg any) {
+	if msg == nil {
+		return
+	}
+
+	if errorResponse, ok := msg.(api.ErrorResponse); ok {
+		// This error contains some additional information that we want to print
+		fmt.Fprintln(os.Stderr, "Error: ", errorResponse.Err)
+		if errorResponse.Hint != "" {
+			fmt.Fprintf(os.Stderr, "\n%s\n", errorResponse.Hint)
+		}
+		os.Exit(1)
+	}
+
+	fmt.Fprintln(os.Stderr, "Error: ", msg)
+	os.Exit(1)
 }

+ 3 - 3
openai/openai.go

@@ -610,14 +610,14 @@ type EmbedWriter struct {
 }
 
 func (w *BaseWriter) writeError(data []byte) (int, error) {
-	var serr api.StatusError
-	err := json.Unmarshal(data, &serr)
+	var er api.ErrorResponse // error response is used here to parse the error message
+	err := json.Unmarshal(data, &er)
 	if err != nil {
 		return 0, err
 	}
 
 	w.ResponseWriter.Header().Set("Content-Type", "application/json")
-	err = json.NewEncoder(w.ResponseWriter).Encode(NewError(http.StatusInternalServerError, serr.Error()))
+	err = json.NewEncoder(w.ResponseWriter).Encode(NewError(http.StatusInternalServerError, er.Err))
 	if err != nil {
 		return 0, err
 	}

+ 11 - 1
server/images.go

@@ -550,7 +550,7 @@ func PullModel(ctx context.Context, name string, regOpts *registryOptions, fn fu
 
 	manifest, err = pullModelManifest(ctx, mp, regOpts)
 	if err != nil {
-		return fmt.Errorf("pull model manifest: %s", err)
+		return fmt.Errorf("pull model manifest: %w", err)
 	}
 
 	var layers []Layer
@@ -629,6 +629,12 @@ func PullModel(ctx context.Context, name string, regOpts *registryOptions, fn fu
 	return nil
 }
 
+type ErrRemoteModelNotFound struct{}
+
+func (ErrRemoteModelNotFound) Error() string {
+	return "model not found"
+}
+
 func pullModelManifest(ctx context.Context, mp ModelPath, regOpts *registryOptions) (*Manifest, error) {
 	requestURL := mp.BaseURL().JoinPath("v2", mp.GetNamespaceRepository(), "manifests", mp.Tag)
 
@@ -636,6 +642,10 @@ func pullModelManifest(ctx context.Context, mp ModelPath, regOpts *registryOptio
 	headers.Set("Accept", "application/vnd.docker.distribution.manifest.v2+json")
 	resp, err := makeRequestWithRetry(ctx, http.MethodGet, requestURL, headers, nil, regOpts)
 	if err != nil {
+		if errors.Is(err, os.ErrNotExist) {
+			// The model was not found on the remote registry
+			return nil, fmt.Errorf("%w: %s", ErrRemoteModelNotFound{}, err)
+		}
 		return nil, err
 	}
 	defer resp.Body.Close()

+ 9 - 1
server/routes.go

@@ -591,7 +591,15 @@ func (s *Server) PullHandler(c *gin.Context) {
 		defer cancel()
 
 		if err := PullModel(ctx, name.DisplayShortest(), regOpts, fn); err != nil {
-			ch <- gin.H{"error": err.Error()}
+			var e ErrRemoteModelNotFound
+			if errors.As(err, &e) {
+				ch <- api.ErrorResponse{
+					Err:  err.Error(),
+					Hint: fmt.Sprintf("Model %q not found - please check the model name is correct and try again", name.DisplayShortest()),
+				}
+			} else {
+				ch <- gin.H{"error": err.Error()}
+			}
 		}
 	}()