Improve error handling for validator

- Surface error codes for datasource possible errors (not found, unreachabel, auth, timeout)

apps/dashvalidator/pkg/validator/dashboard.go

@@ -106,8 +106,9 @@ func ValidateDashboardCompatibility(ctx context.Context, req DashboardCompatibil
 		// Validate queries
 		validationResult, err := v.ValidateQueries(ctx, dsQueries, ds)
 		if err != nil {
-			// Validation failed for this datasource, skip but could log
-			continue
+			// Validation failed for this datasource - return error to caller
+			// This could be a connection error, auth error, or other critical failure
+			return nil, fmt.Errorf("validation failed for datasource %s: %w", dsMapping.UID, err)
 		}
 
 		// Convert to DatasourceValidationResult

apps/dashvalidator/pkg/validator/errors.go

@@ -0,0 +1,173 @@
+package validator
+
+import (
+	"errors"
+	"fmt"
+	"net/http"
+)
+
+// ErrorCode represents the type of error that occurred
+type ErrorCode string
+
+const (
+	// Datasource-related errors
+	ErrCodeDatasourceNotFound    ErrorCode = "datasource_not_found"
+	ErrCodeDatasourceWrongType   ErrorCode = "datasource_wrong_type"
+	ErrCodeDatasourceUnreachable ErrorCode = "datasource_unreachable"
+	ErrCodeDatasourceAuth        ErrorCode = "datasource_auth_failed"
+	ErrCodeDatasourceConfig      ErrorCode = "datasource_config_error"
+
+	// API-related errors
+	ErrCodeAPIUnavailable     ErrorCode = "api_unavailable"
+	ErrCodeAPIInvalidResponse ErrorCode = "api_invalid_response"
+	ErrCodeAPIRateLimit       ErrorCode = "api_rate_limit"
+	ErrCodeAPITimeout         ErrorCode = "api_timeout"
+
+	// Validation errors
+	ErrCodeInvalidDashboard       ErrorCode = "invalid_dashboard"
+	ErrCodeUnsupportedDashVersion ErrorCode = "unsupported_dashboard_version"
+	ErrCodeInvalidQuery           ErrorCode = "invalid_query"
+
+	// Internal errors
+	ErrCodeInternal ErrorCode = "internal_error"
+)
+
+// ValidationError represents a structured error with context
+type ValidationError struct {
+	Code       ErrorCode
+	Message    string
+	Details    map[string]interface{}
+	StatusCode int
+	Cause      error
+}
+
+// Error implements the error interface
+func (e *ValidationError) Error() string {
+	if e.Cause != nil {
+		return fmt.Sprintf("%s: %s (caused by: %v)", e.Code, e.Message, e.Cause)
+	}
+	return fmt.Sprintf("%s: %s", e.Code, e.Message)
+}
+
+// Unwrap implements error unwrapping
+func (e *ValidationError) Unwrap() error {
+	return e.Cause
+}
+
+// NewValidationError creates a new ValidationError
+func NewValidationError(code ErrorCode, message string, statusCode int) *ValidationError {
+	return &ValidationError{
+		Code:       code,
+		Message:    message,
+		StatusCode: statusCode,
+		Details:    make(map[string]interface{}),
+	}
+}
+
+// WithCause adds the underlying error cause
+func (e *ValidationError) WithCause(err error) *ValidationError {
+	e.Cause = err
+	return e
+}
+
+// WithDetail adds contextual information
+func (e *ValidationError) WithDetail(key string, value interface{}) *ValidationError {
+	e.Details[key] = value
+	return e
+}
+
+// Common error constructors
+
+// NewDatasourceNotFoundError creates an error for datasource not found
+func NewDatasourceNotFoundError(uid string, namespace string) *ValidationError {
+	return NewValidationError(
+		ErrCodeDatasourceNotFound,
+		fmt.Sprintf("datasource not found: %s", uid),
+		http.StatusNotFound,
+	).WithDetail("datasourceUID", uid).WithDetail("namespace", namespace)
+}
+
+// NewDatasourceWrongTypeError creates an error for wrong datasource type
+func NewDatasourceWrongTypeError(uid string, expectedType string, actualType string) *ValidationError {
+	return NewValidationError(
+		ErrCodeDatasourceWrongType,
+		fmt.Sprintf("datasource %s has wrong type: expected %s, got %s", uid, expectedType, actualType),
+		http.StatusBadRequest,
+	).WithDetail("datasourceUID", uid).
+		WithDetail("expectedType", expectedType).
+		WithDetail("actualType", actualType)
+}
+
+// NewDatasourceUnreachableError creates an error for unreachable datasource
+func NewDatasourceUnreachableError(uid string, url string, cause error) *ValidationError {
+	return NewValidationError(
+		ErrCodeDatasourceUnreachable,
+		fmt.Sprintf("datasource %s at %s is unreachable", uid, url),
+		http.StatusServiceUnavailable,
+	).WithDetail("datasourceUID", uid).
+		WithDetail("url", url).
+		WithCause(cause)
+}
+
+// NewAPIUnavailableError creates an error for unavailable API
+func NewAPIUnavailableError(statusCode int, responseBody string, cause error) *ValidationError {
+	return NewValidationError(
+		ErrCodeAPIUnavailable,
+		fmt.Sprintf("Prometheus API returned status %d", statusCode),
+		http.StatusBadGateway,
+	).WithDetail("upstreamStatus", statusCode).
+		WithDetail("responseBody", responseBody).
+		WithCause(cause)
+}
+
+// NewAPIInvalidResponseError creates an error for invalid API response
+func NewAPIInvalidResponseError(message string, cause error) *ValidationError {
+	return NewValidationError(
+		ErrCodeAPIInvalidResponse,
+		fmt.Sprintf("Prometheus API returned invalid response: %s", message),
+		http.StatusBadGateway,
+	).WithCause(cause)
+}
+
+// NewAPITimeoutError creates an error for API timeout
+func NewAPITimeoutError(url string, cause error) *ValidationError {
+	return NewValidationError(
+		ErrCodeAPITimeout,
+		fmt.Sprintf("request to %s timed out", url),
+		http.StatusGatewayTimeout,
+	).WithDetail("url", url).
+		WithCause(cause)
+}
+
+// NewDatasourceAuthError creates an error for authentication failures
+func NewDatasourceAuthError(uid string, statusCode int) *ValidationError {
+	return NewValidationError(
+		ErrCodeDatasourceAuth,
+		fmt.Sprintf("authentication failed for datasource %s (status %d)", uid, statusCode),
+		http.StatusUnauthorized,
+	).WithDetail("datasourceUID", uid).
+		WithDetail("upstreamStatus", statusCode)
+}
+
+// IsValidationError checks if an error is a ValidationError
+func IsValidationError(err error) bool {
+	var validationErr *ValidationError
+	return errors.As(err, &validationErr)
+}
+
+// GetValidationError extracts a ValidationError from an error chain
+func GetValidationError(err error) *ValidationError {
+	var validationErr *ValidationError
+	if errors.As(err, &validationErr) {
+		return validationErr
+	}
+	return nil
+}
+
+// GetHTTPStatusCode returns the appropriate HTTP status code for an error
+func GetHTTPStatusCode(err error) int {
+	if validationErr := GetValidationError(err); validationErr != nil {
+		return validationErr.StatusCode
+	}
+	return http.StatusInternalServerError
+}

apps/dashvalidator/pkg/validator/errors_test.go

@@ -0,0 +1,245 @@
+package validator
+
+import (
+	"errors"
+	"net/http"
+	"testing"
+)
+
+func TestNewDatasourceNotFoundError(t *testing.T) {
+	err := NewDatasourceNotFoundError("test-uid", "org-1")
+
+	if err.Code != ErrCodeDatasourceNotFound {
+		t.Errorf("expected error code %s, got %s", ErrCodeDatasourceNotFound, err.Code)
+	}
+
+	if err.StatusCode != http.StatusNotFound {
+		t.Errorf("expected status code %d, got %d", http.StatusNotFound, err.StatusCode)
+	}
+
+	if err.Details["datasourceUID"] != "test-uid" {
+		t.Errorf("expected datasourceUID detail to be 'test-uid', got %v", err.Details["datasourceUID"])
+	}
+
+	if err.Details["namespace"] != "org-1" {
+		t.Errorf("expected namespace detail to be 'org-1', got %v", err.Details["namespace"])
+	}
+}
+
+func TestNewDatasourceWrongTypeError(t *testing.T) {
+	err := NewDatasourceWrongTypeError("test-uid", "prometheus", "influxdb")
+
+	if err.Code != ErrCodeDatasourceWrongType {
+		t.Errorf("expected error code %s, got %s", ErrCodeDatasourceWrongType, err.Code)
+	}
+
+	if err.StatusCode != http.StatusBadRequest {
+		t.Errorf("expected status code %d, got %d", http.StatusBadRequest, err.StatusCode)
+	}
+
+	if err.Details["expectedType"] != "prometheus" {
+		t.Errorf("expected expectedType detail to be 'prometheus', got %v", err.Details["expectedType"])
+	}
+
+	if err.Details["actualType"] != "influxdb" {
+		t.Errorf("expected actualType detail to be 'influxdb', got %v", err.Details["actualType"])
+	}
+}
+
+func TestNewDatasourceUnreachableError(t *testing.T) {
+	cause := errors.New("connection refused")
+	err := NewDatasourceUnreachableError("test-uid", "http://localhost:9090", cause)
+
+	if err.Code != ErrCodeDatasourceUnreachable {
+		t.Errorf("expected error code %s, got %s", ErrCodeDatasourceUnreachable, err.Code)
+	}
+
+	if err.StatusCode != http.StatusServiceUnavailable {
+		t.Errorf("expected status code %d, got %d", http.StatusServiceUnavailable, err.StatusCode)
+	}
+
+	if err.Cause != cause {
+		t.Errorf("expected cause to be set")
+	}
+
+	if err.Details["url"] != "http://localhost:9090" {
+		t.Errorf("expected url detail to be 'http://localhost:9090', got %v", err.Details["url"])
+	}
+}
+
+func TestNewAPIUnavailableError(t *testing.T) {
+	err := NewAPIUnavailableError(503, "service unavailable", nil)
+
+	if err.Code != ErrCodeAPIUnavailable {
+		t.Errorf("expected error code %s, got %s", ErrCodeAPIUnavailable, err.Code)
+	}
+
+	if err.StatusCode != http.StatusBadGateway {
+		t.Errorf("expected status code %d, got %d", http.StatusBadGateway, err.StatusCode)
+	}
+
+	if err.Details["upstreamStatus"] != 503 {
+		t.Errorf("expected upstreamStatus detail to be 503, got %v", err.Details["upstreamStatus"])
+	}
+}
+
+func TestNewAPIInvalidResponseError(t *testing.T) {
+	cause := errors.New("invalid JSON")
+	err := NewAPIInvalidResponseError("missing data field", cause)
+
+	if err.Code != ErrCodeAPIInvalidResponse {
+		t.Errorf("expected error code %s, got %s", ErrCodeAPIInvalidResponse, err.Code)
+	}
+
+	if err.StatusCode != http.StatusBadGateway {
+		t.Errorf("expected status code %d, got %d", http.StatusBadGateway, err.StatusCode)
+	}
+
+	if err.Cause != cause {
+		t.Errorf("expected cause to be set")
+	}
+}
+
+func TestNewAPITimeoutError(t *testing.T) {
+	cause := errors.New("context deadline exceeded")
+	err := NewAPITimeoutError("http://localhost:9090/api/v1/query", cause)
+
+	if err.Code != ErrCodeAPITimeout {
+		t.Errorf("expected error code %s, got %s", ErrCodeAPITimeout, err.Code)
+	}
+
+	if err.StatusCode != http.StatusGatewayTimeout {
+		t.Errorf("expected status code %d, got %d", http.StatusGatewayTimeout, err.StatusCode)
+	}
+
+	if err.Cause != cause {
+		t.Errorf("expected cause to be set")
+	}
+}
+
+func TestNewDatasourceAuthError(t *testing.T) {
+	err := NewDatasourceAuthError("test-uid", 401)
+
+	if err.Code != ErrCodeDatasourceAuth {
+		t.Errorf("expected error code %s, got %s", ErrCodeDatasourceAuth, err.Code)
+	}
+
+	if err.StatusCode != http.StatusUnauthorized {
+		t.Errorf("expected status code %d, got %d", http.StatusUnauthorized, err.StatusCode)
+	}
+
+	if err.Details["upstreamStatus"] != 401 {
+		t.Errorf("expected upstreamStatus detail to be 401, got %v", err.Details["upstreamStatus"])
+	}
+}
+
+func TestValidationErrorChaining(t *testing.T) {
+	cause := errors.New("network error")
+	err := NewValidationError(ErrCodeInternal, "test error", http.StatusInternalServerError).
+		WithCause(cause).
+		WithDetail("key1", "value1").
+		WithDetail("key2", 123)
+
+	if err.Cause != cause {
+		t.Errorf("expected cause to be set")
+	}
+
+	if err.Details["key1"] != "value1" {
+		t.Errorf("expected detail key1 to be 'value1', got %v", err.Details["key1"])
+	}
+
+	if err.Details["key2"] != 123 {
+		t.Errorf("expected detail key2 to be 123, got %v", err.Details["key2"])
+	}
+}
+
+func TestIsValidationError(t *testing.T) {
+	validationErr := NewDatasourceNotFoundError("test-uid", "org-1")
+	regularErr := errors.New("regular error")
+
+	if !IsValidationError(validationErr) {
+		t.Errorf("expected IsValidationError to return true for ValidationError")
+	}
+
+	if IsValidationError(regularErr) {
+		t.Errorf("expected IsValidationError to return false for regular error")
+	}
+}
+
+func TestGetValidationError(t *testing.T) {
+	validationErr := NewDatasourceNotFoundError("test-uid", "org-1")
+	regularErr := errors.New("regular error")
+
+	retrieved := GetValidationError(validationErr)
+	if retrieved == nil {
+		t.Errorf("expected GetValidationError to return the ValidationError")
+	}
+	if retrieved.Code != ErrCodeDatasourceNotFound {
+		t.Errorf("expected retrieved error to have correct code")
+	}
+
+	retrieved = GetValidationError(regularErr)
+	if retrieved != nil {
+		t.Errorf("expected GetValidationError to return nil for regular error")
+	}
+}
+
+func TestGetHTTPStatusCode(t *testing.T) {
+	validationErr := NewDatasourceNotFoundError("test-uid", "org-1")
+	regularErr := errors.New("regular error")
+
+	statusCode := GetHTTPStatusCode(validationErr)
+	if statusCode != http.StatusNotFound {
+		t.Errorf("expected status code %d, got %d", http.StatusNotFound, statusCode)
+	}
+
+	statusCode = GetHTTPStatusCode(regularErr)
+	if statusCode != http.StatusInternalServerError {
+		t.Errorf("expected default status code %d for regular error, got %d", http.StatusInternalServerError, statusCode)
+	}
+}
+
+func TestErrorUnwrap(t *testing.T) {
+	cause := errors.New("underlying error")
+	err := NewDatasourceUnreachableError("test-uid", "http://localhost:9090", cause)
+
+	unwrapped := errors.Unwrap(err)
+	if unwrapped != cause {
+		t.Errorf("expected Unwrap to return the cause")
+	}
+}
+
+func TestErrorErrorMethod(t *testing.T) {
+	// Test without cause
+	err1 := NewDatasourceNotFoundError("test-uid", "org-1")
+	errMsg1 := err1.Error()
+	if errMsg1 == "" {
+		t.Errorf("expected non-empty error message")
+	}
+
+	// Test with cause
+	cause := errors.New("underlying error")
+	err2 := NewDatasourceUnreachableError("test-uid", "http://localhost:9090", cause)
+	errMsg2 := err2.Error()
+	if errMsg2 == "" {
+		t.Errorf("expected non-empty error message")
+	}
+	// Error message should include the cause
+	if !contains(errMsg2, "underlying error") {
+		t.Errorf("expected error message to include cause, got: %s", errMsg2)
+	}
+}
+
+// Helper function to check if a string contains a substring
+func contains(s, substr string) bool {
+	return len(s) >= len(substr) && (s == substr || len(s) > len(substr) && containsHelper(s, substr))
+}
+
+func containsHelper(s, substr string) bool {
+	for i := 0; i <= len(s)-len(substr); i++ {
+		if s[i:i+len(substr)] == substr {
+			return true
+		}
+	}
+	return false
+}

apps/dashvalidator/pkg/validator/prometheus/fetcher.go

@@ -3,10 +3,14 @@ package prometheus
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
 	"net/url"
+	"strings"
+
+	"github.com/grafana/grafana/apps/dashvalidator/pkg/validator"
 )
 
 // Fetcher fetches available metrics from a Prometheus datasource
@@ -31,13 +35,22 @@ func (f *Fetcher) FetchMetrics(ctx context.Context, datasourceURL string, client
 	// Build the API URL
 	baseURL, err := url.Parse(datasourceURL)
 	if err != nil {
-		return nil, fmt.Errorf("invalid datasource URL: %w", err)
+		return nil, validator.NewValidationError(
+			validator.ErrCodeDatasourceConfig,
+			"invalid datasource URL",
+			http.StatusBadRequest,
+		).WithCause(err).WithDetail("url", datasourceURL)
 	}
 
 	// Prometheus metrics endpoint
 	apiPath, err := url.Parse("/api/v1/label/__name__/values")
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse API path: %w", err)
+		// This should never happen with a hardcoded path, but handle it anyway
+		return nil, validator.NewValidationError(
+			validator.ErrCodeInternal,
+			"failed to parse API path",
+			http.StatusInternalServerError,
+		).WithCause(err)
 	}
 
 	fullURL := baseURL.ResolveReference(apiPath)
@@ -45,31 +58,91 @@ func (f *Fetcher) FetchMetrics(ctx context.Context, datasourceURL string, client
 	// Create the request
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, fullURL.String(), nil)
 	if err != nil {
-		return nil, fmt.Errorf("failed to create request: %w", err)
+		return nil, validator.NewValidationError(
+			validator.ErrCodeInternal,
+			"failed to create HTTP request",
+			http.StatusInternalServerError,
+		).WithCause(err)
 	}
 
 	// Execute the request using the provided authenticated client
 	resp, err := client.Do(req)
 	if err != nil {
-		return nil, fmt.Errorf("failed to fetch metrics from Prometheus: %w", err)
+		// Check if it's a timeout error
+		if errors.Is(err, context.DeadlineExceeded) || strings.Contains(err.Error(), "timeout") {
+			return nil, validator.NewAPITimeoutError(fullURL.String(), err)
+		}
+		// Network or connection error - datasource is unreachable
+		return nil, validator.NewDatasourceUnreachableError("", datasourceURL, err)
 	}
 	defer resp.Body.Close()
 
-	// Check status code
-	if resp.StatusCode != http.StatusOK {
-		body, _ := io.ReadAll(resp.Body)
-		return nil, fmt.Errorf("Prometheus API returned status %d: %s", resp.StatusCode, string(body))
+	// Read response body for error reporting
+	body, readErr := io.ReadAll(resp.Body)
+	if readErr != nil {
+		body = []byte("<unable to read response body>")
 	}
 
-	// Parse the response
+	// Check HTTP status code
+	switch resp.StatusCode {
+	case http.StatusOK:
+		// Success - continue to parse response
+	case http.StatusUnauthorized, http.StatusForbidden:
+		// Authentication or authorization failure
+		return nil, validator.NewDatasourceAuthError("", resp.StatusCode).
+			WithDetail("url", fullURL.String()).
+			WithDetail("responseBody", string(body))
+	case http.StatusNotFound:
+		// Endpoint not found - might not be a valid Prometheus instance
+		return nil, validator.NewAPIUnavailableError(
+			resp.StatusCode,
+			string(body),
+			fmt.Errorf("endpoint not found - this may not be a valid Prometheus datasource"),
+		).WithDetail("url", fullURL.String())
+	case http.StatusTooManyRequests:
+		// Rate limiting
+		return nil, validator.NewValidationError(
+			validator.ErrCodeAPIRateLimit,
+			"Prometheus API rate limit exceeded",
+			http.StatusTooManyRequests,
+		).WithDetail("url", fullURL.String()).WithDetail("responseBody", string(body))
+	case http.StatusServiceUnavailable, http.StatusBadGateway, http.StatusGatewayTimeout:
+		// Upstream service is down or unavailable
+		return nil, validator.NewAPIUnavailableError(resp.StatusCode, string(body), nil).
+			WithDetail("url", fullURL.String())
+	default:
+		// Other error status codes
+		return nil, validator.NewAPIUnavailableError(resp.StatusCode, string(body), nil).
+			WithDetail("url", fullURL.String())
+	}
+
+	// Parse the response JSON
 	var promResp prometheusResponse
-	if err := json.NewDecoder(resp.Body).Decode(&promResp); err != nil {
-		return nil, fmt.Errorf("failed to decode Prometheus response: %w", err)
+	if err := json.Unmarshal(body, &promResp); err != nil {
+		return nil, validator.NewAPIInvalidResponseError(
+			"response is not valid JSON",
+			err,
+		).WithDetail("url", fullURL.String()).WithDetail("responseBody", string(body))
 	}
 
-	// Check Prometheus API status
+	// Check Prometheus API status field
 	if promResp.Status != "success" {
-		return nil, fmt.Errorf("Prometheus API returned error: %s", promResp.Error)
+		errorMsg := promResp.Error
+		if errorMsg == "" {
+			errorMsg = "unknown error"
+		}
+		return nil, validator.NewAPIInvalidResponseError(
+			fmt.Sprintf("Prometheus API returned error status: %s", errorMsg),
+			nil,
+		).WithDetail("url", fullURL.String()).WithDetail("prometheusError", errorMsg)
+	}
+
+	// Validate that we got data
+	if promResp.Data == nil {
+		return nil, validator.NewAPIInvalidResponseError(
+			"response missing 'data' field",
+			nil,
+		).WithDetail("url", fullURL.String()).WithDetail("responseBody", string(body))
 	}
 
 	return promResp.Data, nil