fix(slurm): parse structured errors from non-2xx Slurm API responses

Replace ErrorResponse with SlurmAPIError that extracts structured errors/warnings from JSON body when Slurm returns non-2xx (e.g. 404 with valid JSON). Add IsNotFound helper for fallback logic.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

internal/slurm/client_test.go

@@ -137,11 +137,11 @@ func TestClient_ErrorHandling(t *testing.T) {
 		t.Fatal("expected error for 500 response")
 	}
 
-	errorResp, ok := err.(*ErrorResponse)
+	errorResp, ok := err.(*SlurmAPIError)
 	if !ok {
-		t.Fatalf("expected *ErrorResponse, got %T", err)
+		t.Fatalf("expected *SlurmAPIError, got %T", err)
 	}
-	if errorResp.Response.StatusCode != 500 {
-		t.Errorf("expected status 500, got %d", errorResp.Response.StatusCode)
+	if errorResp.StatusCode != 500 {
+		t.Errorf("expected status 500, got %d", errorResp.StatusCode)
 	}
 }

internal/slurm/errors.go

@@ -1,38 +1,85 @@
 package slurm
 
 import (
+	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
 )
 
-// ErrorResponse represents an error returned by the Slurm REST API.
-type ErrorResponse struct {
-	Response *http.Response
-	Message  string
+// errorResponseFields is used to parse errors/warnings from a Slurm API error body.
+type errorResponseFields struct {
+	Errors   OpenapiErrors   `json:"errors,omitempty"`
+	Warnings OpenapiWarnings `json:"warnings,omitempty"`
 }
 
-func (r *ErrorResponse) Error() string {
+// SlurmAPIError represents a structured error returned by the Slurm REST API.
+// It captures both the HTTP details and the parsed Slurm error array when available.
+type SlurmAPIError struct {
+	Response   *http.Response
+	StatusCode int
+	Errors     OpenapiErrors
+	Warnings   OpenapiWarnings
+	Message    string // raw body fallback when JSON parsing fails
+}
+
+func (e *SlurmAPIError) Error() string {
+	if len(e.Errors) > 0 {
+		first := e.Errors[0]
+		detail := ""
+		if first.Error != nil {
+			detail = *first.Error
+		} else if first.Description != nil {
+			detail = *first.Description
+		}
+		if detail != "" {
+			return fmt.Sprintf("%v %v: %d %s",
+				e.Response.Request.Method, e.Response.Request.URL,
+				e.StatusCode, detail)
+		}
+	}
 	return fmt.Sprintf("%v %v: %d %s",
-		r.Response.Request.Method, r.Response.Request.URL,
-		r.Response.StatusCode, r.Message)
+		e.Response.Request.Method, e.Response.Request.URL,
+		e.StatusCode, e.Message)
+}
+
+// IsNotFound reports whether err is a SlurmAPIError with HTTP 404 status.
+func IsNotFound(err error) bool {
+	if apiErr, ok := err.(*SlurmAPIError); ok {
+		return apiErr.StatusCode == http.StatusNotFound
+	}
+	return false
 }
 
 // CheckResponse checks the API response for errors. It returns nil if the
 // response is a 2xx status code. For non-2xx codes, it reads the response
-// body and returns an ErrorResponse.
+// body, attempts to parse structured Slurm errors, and returns a SlurmAPIError.
 func CheckResponse(r *http.Response) error {
 	if c := r.StatusCode; c >= 200 && c <= 299 {
 		return nil
 	}
 
-	errorResponse := &ErrorResponse{Response: r}
 	data, err := io.ReadAll(r.Body)
 	if err != nil || len(data) == 0 {
-		errorResponse.Message = r.Status
-		return errorResponse
+		return &SlurmAPIError{
+			Response:   r,
+			StatusCode: r.StatusCode,
+			Message:    r.Status,
+		}
 	}
 
-	errorResponse.Message = string(data)
-	return errorResponse
+	apiErr := &SlurmAPIError{
+		Response:   r,
+		StatusCode: r.StatusCode,
+		Message:    string(data),
+	}
+
+	// Try to extract structured errors/warnings from JSON body.
+	var fields errorResponseFields
+	if json.Unmarshal(data, &fields) == nil {
+		apiErr.Errors = fields.Errors
+		apiErr.Warnings = fields.Warnings
+	}
+
+	return apiErr
 }

internal/slurm/errors_test.go

@@ -0,0 +1,220 @@
+package slurm
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+func TestCheckResponse(t *testing.T) {
+	tests := []struct {
+		name                string
+		statusCode          int
+		body                string
+		wantErr             bool
+		wantStatusCode      int
+		wantErrors          int
+		wantWarnings        int
+		wantMessageContains string
+	}{
+		{
+			name:       "2xx response returns nil",
+			statusCode: http.StatusOK,
+			body:       `{"meta":{}}`,
+			wantErr:    false,
+		},
+		{
+			name:                "404 with valid JSON error body",
+			statusCode:          http.StatusNotFound,
+			body:                `{"errors":[{"description":"Unable to query JobId=198","error_number":2017,"error":"Invalid job id specified","source":"_handle_job_get"}],"warnings":[]}`,
+			wantErr:             true,
+			wantStatusCode:      404,
+			wantErrors:          1,
+			wantWarnings:        0,
+			wantMessageContains: "Invalid job id specified",
+		},
+		{
+			name:                "500 with non-JSON body",
+			statusCode:          http.StatusInternalServerError,
+			body:                "internal server error",
+			wantErr:             true,
+			wantStatusCode:      500,
+			wantErrors:          0,
+			wantWarnings:        0,
+			wantMessageContains: "internal server error",
+		},
+		{
+			name:                "503 with empty body returns http.Status text",
+			statusCode:          http.StatusServiceUnavailable,
+			body:                "",
+			wantErr:             true,
+			wantStatusCode:      503,
+			wantErrors:          0,
+			wantWarnings:        0,
+			wantMessageContains: "503 Service Unavailable",
+		},
+		{
+			name:                "400 with valid JSON but empty errors array",
+			statusCode:          http.StatusBadRequest,
+			body:                `{"errors":[],"warnings":[]}`,
+			wantErr:             true,
+			wantStatusCode:      400,
+			wantErrors:          0,
+			wantWarnings:        0,
+			wantMessageContains: `{"errors":[],"warnings":[]}`,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			rec := httptest.NewRecorder()
+			rec.WriteHeader(tt.statusCode)
+			if tt.body != "" {
+				rec.Body.WriteString(tt.body)
+			}
+
+			err := CheckResponse(rec.Result())
+
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("CheckResponse() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			if !tt.wantErr {
+				return
+			}
+
+			apiErr, ok := err.(*SlurmAPIError)
+			if !ok {
+				t.Fatalf("expected *SlurmAPIError, got %T", err)
+			}
+
+			if apiErr.StatusCode != tt.wantStatusCode {
+				t.Errorf("StatusCode = %d, want %d", apiErr.StatusCode, tt.wantStatusCode)
+			}
+			if len(apiErr.Errors) != tt.wantErrors {
+				t.Errorf("len(Errors) = %d, want %d", len(apiErr.Errors), tt.wantErrors)
+			}
+			if len(apiErr.Warnings) != tt.wantWarnings {
+				t.Errorf("len(Warnings) = %d, want %d", len(apiErr.Warnings), tt.wantWarnings)
+			}
+			if !strings.Contains(apiErr.Message, tt.wantMessageContains) {
+				t.Errorf("Message = %q, want to contain %q", apiErr.Message, tt.wantMessageContains)
+			}
+		})
+	}
+}
+
+func TestIsNotFound(t *testing.T) {
+	tests := []struct {
+		name string
+		err  error
+		want bool
+	}{
+		{
+			name: "404 SlurmAPIError returns true",
+			err:  &SlurmAPIError{StatusCode: http.StatusNotFound},
+			want: true,
+		},
+		{
+			name: "500 SlurmAPIError returns false",
+			err:  &SlurmAPIError{StatusCode: http.StatusInternalServerError},
+			want: false,
+		},
+		{
+			name: "200 SlurmAPIError returns false",
+			err:  &SlurmAPIError{StatusCode: http.StatusOK},
+			want: false,
+		},
+		{
+			name: "plain error returns false",
+			err:  fmt.Errorf("some error"),
+			want: false,
+		},
+		{
+			name: "nil returns false",
+			err:  nil,
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := IsNotFound(tt.err); got != tt.want {
+				t.Errorf("IsNotFound() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestSlurmAPIError_Error(t *testing.T) {
+	fakeReq := httptest.NewRequest("GET", "http://localhost/slurm/v0.0.40/job/123", nil)
+
+	tests := []struct {
+		name         string
+		err          *SlurmAPIError
+		wantContains []string
+	}{
+		{
+			name: "with Error field set",
+			err: &SlurmAPIError{
+				Response:   &http.Response{Request: fakeReq},
+				StatusCode: http.StatusNotFound,
+				Errors:     OpenapiErrors{{Error: Ptr("Job not found")}},
+				Message:    "raw body",
+			},
+			wantContains: []string{"404", "Job not found"},
+		},
+		{
+			name: "with Description field set when Error is nil",
+			err: &SlurmAPIError{
+				Response:   &http.Response{Request: fakeReq},
+				StatusCode: http.StatusBadRequest,
+				Errors:     OpenapiErrors{{Description: Ptr("Unable to query")}},
+				Message:    "raw body",
+			},
+			wantContains: []string{"400", "Unable to query"},
+		},
+		{
+			name: "with both Error and Description nil falls through to Message",
+			err: &SlurmAPIError{
+				Response:   &http.Response{Request: fakeReq},
+				StatusCode: http.StatusInternalServerError,
+				Errors:     OpenapiErrors{{}},
+				Message:    "something went wrong",
+			},
+			wantContains: []string{"500", "something went wrong"},
+		},
+		{
+			name: "with empty Errors slice falls through to Message",
+			err: &SlurmAPIError{
+				Response:   &http.Response{Request: fakeReq},
+				StatusCode: http.StatusServiceUnavailable,
+				Errors:     OpenapiErrors{},
+				Message:    "service unavailable fallback",
+			},
+			wantContains: []string{"503", "service unavailable fallback"},
+		},
+		{
+			name: "with non-empty Errors but empty detail string falls through to Message",
+			err: &SlurmAPIError{
+				Response:   &http.Response{Request: fakeReq},
+				StatusCode: http.StatusBadGateway,
+				Errors:     OpenapiErrors{{ErrorNumber: Ptr(int32(42))}},
+				Message:    "gateway error detail",
+			},
+			wantContains: []string{"502", "gateway error detail"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.err.Error()
+			for _, substr := range tt.wantContains {
+				if !strings.Contains(got, substr) {
+					t.Errorf("Error() = %q, want to contain %q", got, substr)
+				}
+			}
+		})
+	}
+}